Here is an updated patch version.
I have changed the backend call from localtime() to gmtime() but then
also to gmtime_r().
I moved the _POSIX_C_SOURCE definition for MinGW from the header file to
a command-line option (-D_POSIX_C_SOURCE). This matches the treatment
of _GNU_SOURCE and similar.
I think this is about as good as it's going to get, and we need it to
be, so I propose to commit this version if there are no further concerns.
From 80e25d03404a6d13ce726e07548270bf66de2792 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Fri, 16 Aug 2024 16:50:19 +0200
Subject: [PATCH v2] 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.)
MinGW exposes neither *_r() nor *_s() by default. You can get at the
POSIX-style *_r() functions by defining _POSIX_C_SOURCE appropriately
before including <time.h>. (There is probably also a way to get at
the Windows-style *_s() functions by supplying some additional options
or defines. But we might as well just use the POSIX ones.)
Discussion:
https://www.postgresql.org/message-id/flat/eba1dc75-298e-4c46-8869-48ba8aad7...@eisentraut.org
---
meson.build | 4 ++++
src/backend/utils/adt/pg_locale.c | 3 ++-
src/include/port/win32_port.h | 9 +++++++++
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, 29 insertions(+), 7 deletions(-)
diff --git a/meson.build b/meson.build
index cd711c6d018..ea07126f78e 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 <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 cd3661e7279..0c3d0c7b5b8 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 7ffe5891c69..27ce90ecc3d 100644
--- a/src/include/port/win32_port.h
+++ b/src/include/port/win32_port.h
@@ -420,6 +420,15 @@ 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 already */
+#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 ed08088bfe1..48074fd3ad1 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_t *) &time, &tmbuf);
else
- tx = gmtime((time_t *) &time);
+ tx = gmtime_r((time_t *) &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 402fae6da67..7cf433266f4 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 3527b9f0f5d..ab8a5ab81f3 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 1895f067a88..4f8b0923fe0 100644
--- 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=""
--
2.46.0
