thread-safety: gmtime_r(), localtime_r()

Use gmtime_r() and localtime_r() instead of gmtime() and localtime(),
for thread-safety.

There are a few affected calls in libpq and ecpg's libpgtypes, which
are probably effectively bugs, because those libraries already claim
to be thread-safe.

There is one affected call in the backend.  Most of the backend
otherwise uses the custom functions pg_gmtime() and pg_localtime(),
which are implemented differently.

While we're here, change the call in the backend to gmtime*() instead
of localtime*(), since for that use time zone behavior is irrelevant,
and this side-steps any questions about when time zones are
initialized by localtime_r() vs localtime().

Portability: gmtime_r() and localtime_r() are in POSIX but are not
available on Windows.  Windows has functions gmtime_s() and
localtime_s() that can fulfill the same purpose, so we add some small
wrappers around them.  (Note that these *_s() functions are also
different from the *_s() functions in the bounds-checking extension of
C11.  We are not using those here.)

On MinGW, you can get the POSIX-style *_r() functions by defining
_POSIX_C_SOURCE appropriately before including <time.h>.  This leads
to a conflict at least in plpython because apparently _POSIX_C_SOURCE
gets defined in some header there, and then our replacement
definitions conflict with the system definitions.  To avoid that sort
of thing, we now always define _POSIX_C_SOURCE on MinGW and use the
POSIX-style functions here.

Reviewed-by: Stepan Neretin <sncfmgg@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Reviewed-by: Thomas Munro <thomas.munro@gmail.com>
Discussion: https://www.postgresql.org/message-id/flat/eba1dc75-298e-4c46-8869-48ba8aad7d70@eisentraut.org

diff --git a/meson.build b/meson.build
index cd711c6d0188a7cc4a8b7e1815823d186f8926a0..ea07126f78e9ce2ee471c909a7503abc51e5dae0 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -268,6 +268,10 @@ elif host_system == 'windows'
   exesuffix = '.exe'
   dlsuffix = '.dll'
   library_path_var = ''
+  if cc.get_id() != 'msvc'
+    # define before including <time.h> for getting localtime_r() etc. on MinGW
+    cppflags += '-D_POSIX_C_SOURCE'
+  endif
 
   export_file_format = 'win'
   export_file_suffix = 'def'

diff --git a/src/backend/utils/adt/pg_locale.c b/src/backend/utils/adt/pg_locale.c
index 48b7e16d81bceacf1639ddca3ce159297c484cdb..643cca05d387800f3fcc2ef6ea824ba2038fb138 100644 (file)
--- a/src/backend/utils/adt/pg_locale.c
+++ b/src/backend/utils/adt/pg_locale.c
@@ -826,6 +826,7 @@ cache_locale_time(void)
        char       *bufptr;
        time_t          timenow;
        struct tm  *timeinfo;
+       struct tm       timeinfobuf;
        bool            strftimefail = false;
        int                     encoding;
        int                     i;
@@ -876,7 +877,7 @@ cache_locale_time(void)
 
        /* We use times close to current time as data for strftime(). */
        timenow = time(NULL);
-       timeinfo = localtime(&timenow);
+       timeinfo = gmtime_r(&timenow, &timeinfobuf);
 
        /* Store the strftime results in MAX_L10N_DATA-sized portions of buf[] */
        bufptr = buf;

diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h
index 7ffe5891c69ac9b76b0f697ad75796b777140958..7789e0431aaa6599a0af6ec6b5dd4c84b5914ee8 100644 (file)
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -420,6 +420,19 @@ extern int _pglstat64(const char *name, struct stat *buf);
  */
 #define strtok_r strtok_s
 
+/*
+ * Supplement to <time.h>.
+ */
+#ifdef _MSC_VER
+/*
+ * MinGW has these functions if _POSIX_C_SOURCE is defined.  Third-party
+ * libraries might do that, so to avoid clashes we get ahead of it and define
+ * it ourselves and use the system functions provided by MinGW.
+ */
+#define gmtime_r(clock, result) (gmtime_s(result, clock) ? NULL : (result))
+#define localtime_r(clock, result) (localtime_s(result, clock) ? NULL : (result))
+#endif
+
 /*
  * Locale stuff.
  *

diff --git a/src/interfaces/ecpg/pgtypeslib/dt_common.c b/src/interfaces/ecpg/pgtypeslib/dt_common.c
index ed08088bfe1320052aa87bc78f351a2e733aa245..0d63fe52c5bb382103b03a6f38d39d1b21d275f4 100644 (file)
--- a/src/interfaces/ecpg/pgtypeslib/dt_common.c
+++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c
@@ -949,9 +949,10 @@ int
 GetEpochTime(struct tm *tm)
 {
        struct tm  *t0;
+       struct tm       tmbuf;
        time_t          epoch = 0;
 
-       t0 = gmtime(&epoch);
+       t0 = gmtime_r(&epoch, &tmbuf);
 
        if (t0)
        {
@@ -973,12 +974,13 @@ abstime2tm(AbsoluteTime _time, int *tzp, struct tm *tm, char **tzn)
 {
        time_t          time = (time_t) _time;
        struct tm  *tx;
+       struct tm       tmbuf;
 
        errno = 0;
        if (tzp != NULL)
-               tx = localtime((time_t *) &time);
+               tx = localtime_r(&time, &tmbuf);
        else
-               tx = gmtime((time_t *) &time);
+               tx = gmtime_r(&time, &tmbuf);
 
        if (!tx)
        {
@@ -2810,9 +2812,10 @@ PGTYPEStimestamp_defmt_scan(char **str, char *fmt, timestamp * d,
                                /* number of seconds in scan_val.luint_val */
                                {
                                        struct tm  *tms;
+                                       struct tm       tmbuf;
                                        time_t          et = (time_t) scan_val.luint_val;
 
-                                       tms = gmtime(&et);
+                                       tms = gmtime_r(&et, &tmbuf);
 
                                        if (tms)
                                        {

diff --git a/src/interfaces/ecpg/pgtypeslib/timestamp.c b/src/interfaces/ecpg/pgtypeslib/timestamp.c
index 402fae6da679113bdd1cc91aa66b09d91033f608..7cf433266f45855bf5b491e4fe970658579f51d6 100644 (file)
--- a/src/interfaces/ecpg/pgtypeslib/timestamp.c
+++ b/src/interfaces/ecpg/pgtypeslib/timestamp.c
@@ -129,11 +129,12 @@ timestamp2tm(timestamp dt, int *tzp, struct tm *tm, fsec_t *fsec, const char **t
                if (IS_VALID_UTIME(tm->tm_year, tm->tm_mon, tm->tm_mday))
                {
 #if defined(HAVE_STRUCT_TM_TM_ZONE) || defined(HAVE_INT_TIMEZONE)
+                       struct tm       tmbuf;
 
                        utime = dt / USECS_PER_SEC +
                                ((date0 - date2j(1970, 1, 1)) * INT64CONST(86400));
 
-                       tx = localtime(&utime);
+                       tx = localtime_r(&utime, &tmbuf);
                        tm->tm_year = tx->tm_year + 1900;
                        tm->tm_mon = tx->tm_mon + 1;
                        tm->tm_mday = tx->tm_mday;

diff --git a/src/interfaces/libpq/fe-trace.c b/src/interfaces/libpq/fe-trace.c
index bff7d919d570774af16e3f001acb6043391adf3b..19c5b8a89000488aef01247e1e9d21a2e62d1e40 100644 (file)
--- a/src/interfaces/libpq/fe-trace.c
+++ b/src/interfaces/libpq/fe-trace.c
@@ -81,6 +81,7 @@ pqTraceFormatTimestamp(char *timestr, size_t ts_len)
 {
        struct timeval tval;
        time_t          now;
+       struct tm       tmbuf;
 
        gettimeofday(&tval, NULL);
 
@@ -93,7 +94,7 @@ pqTraceFormatTimestamp(char *timestr, size_t ts_len)
        now = tval.tv_sec;
        strftime(timestr, ts_len,
                         "%Y-%m-%d %H:%M:%S",
-                        localtime(&now));
+                        localtime_r(&now, &tmbuf));
        /* append microseconds */
        snprintf(timestr + strlen(timestr), ts_len - strlen(timestr),
                         ".%06u", (unsigned int) (tval.tv_usec));

diff --git a/src/template/win32 b/src/template/win32
index 1895f067a889b52ac9b7fd9c23b548774b56dc72..4f8b0923fe0418290dcd8b0487e136fcd86660b1 100644 (file)
--- a/src/template/win32
+++ b/src/template/win32
@@ -1,5 +1,8 @@
 # src/template/win32
 
+# define before including <time.h> for getting localtime_r() etc. on MinGW
+CPPFLAGS="$CPPFLAGS -D_POSIX_C_SOURCE"
+
 # Extra CFLAGS for code that will go into a shared library
 CFLAGS_SL=""