From a2bbc58f743489784de797d81be37ea309cb0773 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Fri, 23 Aug 2024 07:07:53 +0200 Subject: [PATCH] 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 . 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 Reviewed-by: Heikki Linnakangas Reviewed-by: Thomas Munro Discussion: https://www.postgresql.org/message-id/flat/eba1dc75-298e-4c46-8869-48ba8aad7d70@eisentraut.org --- meson.build | 4 ++++ src/backend/utils/adt/pg_locale.c | 3 ++- src/include/port/win32_port.h | 13 +++++++++++++ src/interfaces/ecpg/pgtypeslib/dt_common.c | 11 +++++++---- src/interfaces/ecpg/pgtypeslib/timestamp.c | 3 ++- src/interfaces/libpq/fe-trace.c | 3 ++- src/template/win32 | 3 +++ 7 files changed, 33 insertions(+), 7 deletions(-) diff --git a/meson.build b/meson.build index cd711c6d01..ea07126f78 100644 --- 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 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 48b7e16d81..643cca05d3 100644 --- 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 7ffe5891c6..7789e0431a 100644 --- 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 . + */ +#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 ed08088bfe..0d63fe52c5 100644 --- 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 402fae6da6..7cf433266f 100644 --- 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 bff7d919d5..19c5b8a890 100644 --- 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 1895f067a8..4f8b0923fe 100644 --- a/src/template/win32 +++ b/src/template/win32 @@ -1,5 +1,8 @@ # src/template/win32 +# define before including 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=""