MT-unsafe time modules

MT-unsafe time modules

[Bruno Haible]

When a module is under LGPL, it is implicitly advertised as suitable for
libraries. And libraries should be MT-safe (for 20 years already). So,
such modules should be MT-safe.

Looking at the LGPLed time-related modules:

* This one is already MT-safe:

timespec

* These need to be made MT-safe:

strftime-fixes
wcsftime
ctime
localtime

mktime, mktime-internal
timegm

time_r

strptime

time_rz
c-nstrftime, nstrftime
parse-datetime, parse-datetime2


I will try to tackle them, in roughly the given order.

Bruno

Re: MT-unsafe time modules

[Bruno Haible]

This patch makes ctime, localtime, tzset, wcsftime multithread-safe.


2024-02-09  Bruno Haible  <bruno@clisp.org>

	ctime, localtime, tzset, wcsftime: Make multithread-safe.
	* lib/ctime.c: Include <wchar.h>.
	(rpl_ctime): Modify _environ and _wenviron without using _putenv.
	* lib/localtime.c: Include <wchar.h>.
	(rpl_localtime): Modify _environ and _wenviron without using _putenv.
	* lib/tzset.c: Include <wchar.h>.
	(rpl_tzset): Modify _environ and _wenviron without using _putenv.
	* lib/wcsftime.c (rpl_wcsftime): Likewise.

diff --git a/lib/ctime.c b/lib/ctime.c
index a11adc5c74..8c54ef463c 100644
--- a/lib/ctime.c
+++ b/lib/ctime.c
@@ -21,6 +21,9 @@
 
 #include <stdlib.h>
 #include <string.h>
+#if defined _WIN32 && ! defined __CYGWIN__
+# include <wchar.h>
+#endif
 
 #undef ctime
 
@@ -52,7 +55,22 @@ rpl_ctime (const time_t *tp)
      responsibility.  */
   const char *tz = getenv ("TZ");
   if (tz != NULL && strchr (tz, '/') != NULL)
-    _putenv ("TZ=");
+    {
+      /* Neutralize it, in a way that is multithread-safe.
+         (If we were to use _putenv ("TZ="), it would free the memory allocated
+         for the environment variable "TZ", and thus other threads that are
+         using the previously fetched value of getenv ("TZ") could crash.)  */
+      char **env = _environ;
+      wchar_t **wenv = _wenviron;
+      if (env != NULL)
+        for (char *s = env; *s != NULL; s++)
+          if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
+            s[0] = '$';
+      if (wenv != NULL)
+        for (wchar_t *ws = wenv; *ws != NULL; ws++)
+          if (ws[0] == L'T' && ws[1] == L'Z' && ws[2] == L'=')
+            ws[0] = L'$';
+    }
 #endif
 
   return ctime (tp);
diff --git a/lib/localtime.c b/lib/localtime.c
index bdea1cab10..f0e91ac647 100644
--- a/lib/localtime.c
+++ b/lib/localtime.c
@@ -21,6 +21,9 @@
 
 #include <stdlib.h>
 #include <string.h>
+#if defined _WIN32 && ! defined __CYGWIN__
+# include <wchar.h>
+#endif
 
 #undef localtime
 
@@ -52,7 +55,22 @@ rpl_localtime (const time_t *tp)
      responsibility.  */
   const char *tz = getenv ("TZ");
   if (tz != NULL && strchr (tz, '/') != NULL)
-    _putenv ("TZ=");
+    {
+      /* Neutralize it, in a way that is multithread-safe.
+         (If we were to use _putenv ("TZ="), it would free the memory allocated
+         for the environment variable "TZ", and thus other threads that are
+         using the previously fetched value of getenv ("TZ") could crash.)  */
+      char **env = _environ;
+      wchar_t **wenv = _wenviron;
+      if (env != NULL)
+        for (char *s = env; *s != NULL; s++)
+          if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
+            s[0] = '$';
+      if (wenv != NULL)
+        for (wchar_t *ws = wenv; *ws != NULL; ws++)
+          if (ws[0] == L'T' && ws[1] == L'Z' && ws[2] == L'=')
+            ws[0] = L'$';
+    }
 #endif
 
   return localtime (tp);
diff --git a/lib/tzset.c b/lib/tzset.c
index 0eb8c161f4..f307f0c3d1 100644
--- a/lib/tzset.c
+++ b/lib/tzset.c
@@ -24,6 +24,9 @@
 
 #include <stdlib.h>
 #include <string.h>
+#if defined _WIN32 && ! defined __CYGWIN__
+# include <wchar.h>
+#endif
 
 void
 rpl_tzset (void)
@@ -54,7 +57,22 @@ rpl_tzset (void)
      responsibility.  */
   const char *tz = getenv ("TZ");
   if (tz != NULL && strchr (tz, '/') != NULL)
-    _putenv ("TZ=");
+    {
+      /* Neutralize it, in a way that is multithread-safe.
+         (If we were to use _putenv ("TZ="), it would free the memory allocated
+         for the environment variable "TZ", and thus other threads that are
+         using the previously fetched value of getenv ("TZ") could crash.)  */
+      char **env = _environ;
+      wchar_t **wenv = _wenviron;
+      if (env != NULL)
+        for (char *s = env; *s != NULL; s++)
+          if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
+            s[0] = '$';
+      if (wenv != NULL)
+        for (wchar_t *ws = wenv; *ws != NULL; ws++)
+          if (ws[0] == L'T' && ws[1] == L'Z' && ws[2] == L'=')
+            ws[0] = L'$';
+    }
 
   /* On native Windows, tzset() is deprecated.  Use _tzset() instead.  See
      <https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/posix-tzset>
diff --git a/lib/wcsftime.c b/lib/wcsftime.c
index 93e0470dd3..d8b471ab57 100644
--- a/lib/wcsftime.c
+++ b/lib/wcsftime.c
@@ -53,7 +53,22 @@ rpl_wcsftime (wchar_t *buf, size_t bufsize, const wchar_t *format, const struct
      responsibility.  */
   const char *tz = getenv ("TZ");
   if (tz != NULL && strchr (tz, '/') != NULL)
-    _putenv ("TZ=");
+    {
+      /* Neutralize it, in a way that is multithread-safe.
+         (If we were to use _putenv ("TZ="), it would free the memory allocated
+         for the environment variable "TZ", and thus other threads that are
+         using the previously fetched value of getenv ("TZ") could crash.)  */
+      char **env = _environ;
+      wchar_t **wenv = _wenviron;
+      if (env != NULL)
+        for (char *s = env; *s != NULL; s++)
+          if (s[0] == 'T' && s[1] == 'Z' && s[2] == '=')
+            s[0] = '$';
+      if (wenv != NULL)
+        for (wchar_t *ws = wenv; *ws != NULL; ws++)
+          if (ws[0] == L'T' && ws[1] == L'Z' && ws[2] == L'=')
+            ws[0] = L'$';
+    }
 #endif
 
   return wcsftime (buf, bufsize, format, tp);

Re: MT-unsafe time modules

[Bruno Haible]

> * These need to be made MT-safe:
> 
> mktime, mktime-internal
> timegm
> 
> time_r
> 
> time_rz
> c-nstrftime, nstrftime
> parse-datetime, parse-datetime2

For these modules, the next function to provide in an MT-safe way is
localtime_r. On native Windows, when the 'localtime_s' function [1][2]
is not available, such as on the older Windows versions that Emacs cares
about, the solution is to use GetTimeZoneInformation [3].

The next function, then, is a localtime_r variant with time zone argument.
It's needed
  * for implementing 'localtime_rz', in the 'time_rz' module.
  * as a 'convert' subroutine for the 'mktime_z' function, in the 'time_rz'
    module.
  * indirectly by nstrftime, c-nstrftime, fprintftime, parse-datetime,
    that all rely on 'time_rz'.

Such a localtime_r variant with time zone argument needs to use the
time zone database that the system has access to. But the only public
API that the system offers (localtime{,_r}) relies on the environment "TZ"
variable "TZ" and some global variables (tzname, timezone, daylight)
derived from it, and changing an environment variable is not MT-safe.

So, there are only two approaches that I can see:

  (a) Read the time zone database information into memory from a file
      shipped together with Gnulib. This is how the libstdc++ implementation
      of the C++ time zone functionality [4] does it [5][6].

  (b) Read the time zone database information into memory from the
      system locations, in a platform dependent way.

The code for doing this exists in glibc and libstdc++ and would need to be
adapted.

BUT this is a major project: it would take several weeks.

And it is overkill for
  - the proposed safer_ctime function,
  - most practical uses of nstrftime, c_nstrftime.
Namely, for these cases, a general timezone_t is not needed, only a boolean
that covers the case of local time and GMT/UTC. And these cases *can* be
done in a multithread-safe way:
  - The value of "TZ" does not need to be changed on the fly.
  - The values of the global variables (tzname, timezone, daylight) may
    be set through tzset(), in a thread, but these values are OK for the
    other threads as well, since getenv ("TZ") is the same in all threads.
  - The GMT/UTC case can be implemented by not looking at "TZ" nor the
    global variables.

I therefore intend to do this latter part:
  1) Make localtime_r on native Windows MT-safe.
  2) Document which LGPLed interfaces are not MT-safe.
  3) Provide an MT-safe and bug-fixed 'strftime' function (that assumes
     local time, no time zone argument).
  4) Provide an analogous 'c_strftime' function that does not even access
     "TZ".
  5) The proposed safer_ctime, based on localtime_r and c_strftime.

Bruno

[1] https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/localtime-s-localtime32-s-localtime64-s
[2] https://learn.microsoft.com/en-us/previous-versions/a442x3ye
[3] https://learn.microsoft.com/en-us/windows/win32/api/timezoneapi/nf-timezoneapi-gettimezoneinformation
[4] https://en.cppreference.com/w/cpp/chrono
[5] https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/std/chrono
[6] https://gcc.gnu.org/git/?p=gcc.git;a=tree;f=libstdc%2B%2B-v3/src/c%2B%2B20

Re: MT-unsafe time modules

[Bruno Haible]

[-- Attachment #1: Type: text/plain, Size: 452 bytes --]

This patch adds a unit test that verifies that gmtime_r and localtime_r
are MT-safe.


2024-02-11  Bruno Haible  <bruno@clisp.org>

	time_r: Add tests.
	* lib/time_r.c: Add comment.
	* tests/test-gmtime_r.c: New file.
	* tests/test-gmtime_r-mt.c: New file, based on
	tests/test-nl_langinfo-mt.c.
	* tests/test-localtime_r.c: New file.
	* tests/test-localtime_r-mt.c: New file, based on
	tests/test-nl_langinfo-mt.c.
	* modules/time_r-tests: New file.


[-- Attachment #2: 0001-time_r-Add-tests.patch --]
[-- Type: text/x-patch, Size: 18997 bytes --]

From 10321db3cd2d0883f085f10eb25f99aa80a97075 Mon Sep 17 00:00:00 2001
From: Bruno Haible <bruno@clisp.org>
Date: Sun, 11 Feb 2024 11:23:14 +0100
Subject: [PATCH] time_r: Add tests.

* lib/time_r.c: Add comment.
* tests/test-gmtime_r.c: New file.
* tests/test-gmtime_r-mt.c: New file, based on
tests/test-nl_langinfo-mt.c.
* tests/test-localtime_r.c: New file.
* tests/test-localtime_r-mt.c: New file, based on
tests/test-nl_langinfo-mt.c.
* modules/time_r-tests: New file.
---
 ChangeLog                   |  12 +++
 lib/time_r.c                |   5 ++
 modules/time_r-tests        |  27 ++++++
 tests/test-gmtime_r-mt.c    | 123 ++++++++++++++++++++++++++
 tests/test-gmtime_r.c       |  56 ++++++++++++
 tests/test-localtime_r-mt.c | 139 ++++++++++++++++++++++++++++++
 tests/test-localtime_r.c    | 167 ++++++++++++++++++++++++++++++++++++
 7 files changed, 529 insertions(+)
 create mode 100644 modules/time_r-tests
 create mode 100644 tests/test-gmtime_r-mt.c
 create mode 100644 tests/test-gmtime_r.c
 create mode 100644 tests/test-localtime_r-mt.c
 create mode 100644 tests/test-localtime_r.c

diff --git a/ChangeLog b/ChangeLog
index 8b6c726087..a25e9f8706 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,15 @@
+2024-02-11  Bruno Haible  <bruno@clisp.org>
+
+	time_r: Add tests.
+	* lib/time_r.c: Add comment.
+	* tests/test-gmtime_r.c: New file.
+	* tests/test-gmtime_r-mt.c: New file, based on
+	tests/test-nl_langinfo-mt.c.
+	* tests/test-localtime_r.c: New file.
+	* tests/test-localtime_r-mt.c: New file, based on
+	tests/test-nl_langinfo-mt.c.
+	* modules/time_r-tests: New file.
+
 2024-02-10  Paul Eggert  <eggert@cs.ucla.edu>
 
 	doc: improve warnings about ctime etc.
diff --git a/lib/time_r.c b/lib/time_r.c
index 3ef0b36802..b724f3b38d 100644
--- a/lib/time_r.c
+++ b/lib/time_r.c
@@ -21,6 +21,11 @@
 
 #include <time.h>
 
+/* The replacement functions in this file are only used on native Windows.
+   They are multithread-safe, because the gmtime() and localtime() functions
+   on native Windows — both in the ucrt and in the older MSVCRT — return a
+   pointer to a 'struct tm' in thread-local memory.  */
+
 static struct tm *
 copy_tm_result (struct tm *dest, struct tm const *src)
 {
diff --git a/modules/time_r-tests b/modules/time_r-tests
new file mode 100644
index 0000000000..56346c0f7c
--- /dev/null
+++ b/modules/time_r-tests
@@ -0,0 +1,27 @@
+Files:
+tests/test-gmtime_r.c
+tests/test-gmtime_r-mt.c
+tests/test-localtime_r.c
+tests/test-localtime_r-mt.c
+tests/macros.h
+
+Depends-on:
+setenv
+thread
+nanosleep
+
+configure.ac:
+dnl Possibly define HAVE_STRUCT_TM_TM_ZONE.
+AC_REQUIRE([AC_STRUCT_TIMEZONE])
+dnl Possibly define HAVE_STRUCT_TM_TM_GMTOFF.
+AC_CHECK_MEMBERS([struct tm.tm_gmtoff],,,[[#include <time.h>]])
+
+Makefile.am:
+TESTS += \
+  test-gmtime_r test-gmtime_r-mt \
+  test-localtime_r test-localtime_r-mt
+check_PROGRAMS += \
+  test-gmtime_r test-gmtime_r-mt \
+  test-localtime_r test-localtime_r-mt
+test_gmtime_r_mt_LDADD = $(LDADD) $(LIBMULTITHREAD) $(NANOSLEEP_LIB)
+test_localtime_r_mt_LDADD = $(LDADD) $(LIBMULTITHREAD) $(NANOSLEEP_LIB)
diff --git a/tests/test-gmtime_r-mt.c b/tests/test-gmtime_r-mt.c
new file mode 100644
index 0000000000..4d38bad1a3
--- /dev/null
+++ b/tests/test-gmtime_r-mt.c
@@ -0,0 +1,123 @@
+/* Multithread-safety test for gmtime_r().
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2024.  */
+
+#include <config.h>
+
+/* Work around GCC bug 44511.  */
+#if 4 < __GNUC__ + (3 <= __GNUC_MINOR__)
+# pragma GCC diagnostic ignored "-Wreturn-type"
+#endif
+
+#if USE_ISOC_THREADS || USE_POSIX_THREADS || USE_ISOC_AND_POSIX_THREADS || USE_WINDOWS_THREADS
+
+/* Specification.  */
+#include <time.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "glthread/thread.h"
+#include "macros.h"
+
+static void *
+thread1_func (void *arg)
+{
+  for (;;)
+    {
+      time_t t = 1509000003; /* 2017-10-26 06:40:03 */
+      struct tm tm;
+      struct tm *result = gmtime_r (&t, &tm);
+      ASSERT (result == &tm);
+      if (!(result->tm_sec == 3
+            && result->tm_min == 40
+            && result->tm_hour == 6
+            && result->tm_mday == 26
+            && result->tm_mon == 10 - 1
+            && result->tm_year == 2017 - 1900
+            && result->tm_wday == 4
+            && result->tm_yday == 298
+            && result->tm_isdst == 0))
+        {
+          fprintf (stderr, "thread1 disturbed by thread2!\n"); fflush (stderr);
+          abort ();
+        }
+    }
+
+  /*NOTREACHED*/
+}
+
+static void *
+thread2_func (void *arg)
+{
+  for (;;)
+    {
+      time_t t = 2000050005; /* 2033-05-18 17:26:45 */
+      struct tm tm;
+      struct tm *result = gmtime_r (&t, &tm);
+      ASSERT (result == &tm);
+      if (!(result->tm_sec == 45
+            && result->tm_min == 26
+            && result->tm_hour == 17
+            && result->tm_mday == 18
+            && result->tm_mon == 5 - 1
+            && result->tm_year == 2033 - 1900
+            && result->tm_wday == 3
+            && result->tm_yday == 137
+            && result->tm_isdst == 0))
+        {
+          fprintf (stderr, "thread2 disturbed by thread1!\n"); fflush (stderr);
+          abort ();
+        }
+    }
+
+  /*NOTREACHED*/
+}
+
+int
+main (int argc, char *argv[])
+{
+  /* Create the threads.  */
+  gl_thread_create (thread1_func, NULL);
+  gl_thread_create (thread2_func, NULL);
+
+  /* Let them run for 1 second.  */
+  {
+    struct timespec duration;
+    duration.tv_sec = (argc > 1 ? atoi (argv[1]) : 1);
+    duration.tv_nsec = 0;
+
+    nanosleep (&duration, NULL);
+  }
+
+  return 0;
+}
+
+#else
+
+/* No multithreading available.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  fputs ("Skipping test: multithreading not enabled\n", stderr);
+  return 77;
+}
+
+#endif
diff --git a/tests/test-gmtime_r.c b/tests/test-gmtime_r.c
new file mode 100644
index 0000000000..01eebf3c29
--- /dev/null
+++ b/tests/test-gmtime_r.c
@@ -0,0 +1,56 @@
+/* Test gmtime_r().
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2024.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include <time.h>
+
+#include <string.h>
+
+#include "macros.h"
+
+int
+main (void)
+{
+  {
+    time_t t = 1509000003; /* 2017-10-26 06:40:03 */
+    struct tm tm;
+    struct tm *result = gmtime_r (&t, &tm);
+    ASSERT (result == &tm);
+    ASSERT (result->tm_sec == 3);
+    ASSERT (result->tm_min == 40);
+    ASSERT (result->tm_hour == 6);
+    ASSERT (result->tm_mday == 26);
+    ASSERT (result->tm_mon == 10 - 1);
+    ASSERT (result->tm_year == 2017 - 1900);
+    ASSERT (result->tm_wday == 4);
+    ASSERT (result->tm_yday == 298);
+    ASSERT (result->tm_isdst == 0);
+#if HAVE_STRUCT_TM_TM_GMTOFF /* glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, Minix, Cygwin, Android */
+    ASSERT (result->tm_gmtoff == 0);
+#endif
+#if HAVE_STRUCT_TM_TM_ZONE /* glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, Minix, Cygwin, Android */
+    printf ("tm_zone = %s\n", result->tm_zone == NULL ? "(null)" : result->tm_zone);
+    ASSERT (strcmp (result->tm_zone, "GMT") == 0 /* glibc, NetBSD, OpenBSD, Minix, Cygwin, Android */
+            || strcmp (result->tm_zone, "UTC") == 0 /* musl, macOS, FreeBSD */);
+#endif
+  }
+
+  return 0;
+}
diff --git a/tests/test-localtime_r-mt.c b/tests/test-localtime_r-mt.c
new file mode 100644
index 0000000000..ae371a8946
--- /dev/null
+++ b/tests/test-localtime_r-mt.c
@@ -0,0 +1,139 @@
+/* Multithread-safety test for localtime_r().
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2024.  */
+
+#include <config.h>
+
+/* Work around GCC bug 44511.  */
+#if 4 < __GNUC__ + (3 <= __GNUC_MINOR__)
+# pragma GCC diagnostic ignored "-Wreturn-type"
+#endif
+
+#if USE_ISOC_THREADS || USE_POSIX_THREADS || USE_ISOC_AND_POSIX_THREADS || USE_WINDOWS_THREADS
+
+/* Specification.  */
+#include <time.h>
+
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "glthread/thread.h"
+#include "macros.h"
+
+
+/* Some common time zone name.  */
+
+#if defined _WIN32 && !defined __CYGWIN__
+/* Cf. <https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/default-time-zones>
+   or <https://ss64.com/timezones.html>  */
+# define FRENCH_TZ  "Romance Standard Time"
+#else
+/* Cf. <https://en.wikipedia.org/wiki/List_of_tz_database_time_zones>  */
+# define FRENCH_TZ  "Europe/Paris"
+#endif
+
+
+static void *
+thread1_func (void *arg)
+{
+  for (;;)
+    {
+
+      time_t t = 1178467200; /* 2007-05-06 18:00:00 */
+      struct tm tm;
+      struct tm *result = localtime_r (&t, &tm);
+      ASSERT (result == &tm);
+      if (!(result->tm_sec == 0
+            && result->tm_min == 0
+            && result->tm_hour == 18
+            && result->tm_mday == 6
+            && result->tm_mon == 5 - 1
+            && result->tm_year == 2007 - 1900
+            && result->tm_wday == 0
+            && result->tm_yday == 125
+            && result->tm_isdst == 1))
+        {
+          fprintf (stderr, "thread1 disturbed by thread2!\n"); fflush (stderr);
+          abort ();
+        }
+    }
+
+  /*NOTREACHED*/
+}
+
+static void *
+thread2_func (void *arg)
+{
+  for (;;)
+    {
+      time_t t = 1336320000; /* 2012-05-06 18:00:00 */
+      struct tm tm;
+      struct tm *result = localtime_r (&t, &tm);
+      ASSERT (result == &tm);
+      if (!(result->tm_sec == 0
+            && result->tm_min == 0
+            && result->tm_hour == 18
+            && result->tm_mday == 6
+            && result->tm_mon == 5 - 1
+            && result->tm_year == 2012 - 1900
+            && result->tm_wday == 0
+            && result->tm_yday == 126
+            && result->tm_isdst == 1))
+        {
+          fprintf (stderr, "thread2 disturbed by thread1!\n"); fflush (stderr);
+          abort ();
+        }
+    }
+
+  /*NOTREACHED*/
+}
+
+int
+main (int argc, char *argv[])
+{
+  setenv ("TZ", FRENCH_TZ, 1);
+
+  /* Create the threads.  */
+  gl_thread_create (thread1_func, NULL);
+  gl_thread_create (thread2_func, NULL);
+
+  /* Let them run for 1 second.  */
+  {
+    struct timespec duration;
+    duration.tv_sec = (argc > 1 ? atoi (argv[1]) : 1);
+    duration.tv_nsec = 0;
+
+    nanosleep (&duration, NULL);
+  }
+
+  return 0;
+}
+
+#else
+
+/* No multithreading available.  */
+
+#include <stdio.h>
+
+int
+main ()
+{
+  fputs ("Skipping test: multithreading not enabled\n", stderr);
+  return 77;
+}
+
+#endif
diff --git a/tests/test-localtime_r.c b/tests/test-localtime_r.c
new file mode 100644
index 0000000000..70ec3b5d4f
--- /dev/null
+++ b/tests/test-localtime_r.c
@@ -0,0 +1,167 @@
+/* Test localtime_r().
+   Copyright (C) 2024 Free Software Foundation, Inc.
+
+   This program is free software: you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation, either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <https://www.gnu.org/licenses/>.  */
+
+/* Written by Bruno Haible <bruno@clisp.org>, 2024.  */
+
+#include <config.h>
+
+/* Specification.  */
+#include <time.h>
+
+#include <string.h>
+
+#include "macros.h"
+
+
+/* Some common time zone name.  */
+
+#if defined _WIN32 && !defined __CYGWIN__
+/* Cf. <https://learn.microsoft.com/en-us/windows-hardware/manufacture/desktop/default-time-zones>
+   or <https://ss64.com/timezones.html>  */
+# define FRENCH_TZ  "Romance Standard Time"
+#else
+/* Cf. <https://en.wikipedia.org/wiki/List_of_tz_database_time_zones>  */
+# define FRENCH_TZ  "Europe/Paris"
+#endif
+
+
+int
+main (void)
+{
+  setenv ("TZ", FRENCH_TZ, 1);
+
+  /* Note: The result->tm_gmtoff values and the result->tm_zone values are the
+     same (3600, "CET" or 7200, "CEST") across all tested platforms:
+     glibc, musl, macOS, FreeBSD, NetBSD, OpenBSD, Minix, Cygwin, Android.  */
+
+  /* A time point when DST was in effect.  */
+  {
+    time_t t = 1178467200; /* 2007-05-06 18:00:00 */
+    struct tm tm;
+    struct tm *result = localtime_r (&t, &tm);
+    ASSERT (result == &tm);
+    ASSERT (result->tm_sec == 0);
+    ASSERT (result->tm_min == 0);
+    ASSERT (result->tm_hour == 18);
+    ASSERT (result->tm_mday == 6);
+    ASSERT (result->tm_mon == 5 - 1);
+    ASSERT (result->tm_year == 2007 - 1900);
+    ASSERT (result->tm_wday == 0);
+    ASSERT (result->tm_yday == 125);
+    ASSERT (result->tm_isdst == 1);
+#if HAVE_STRUCT_TM_TM_GMTOFF
+    ASSERT (result->tm_gmtoff == (1 + result->tm_isdst) * 3600);
+#endif
+#if HAVE_STRUCT_TM_TM_ZONE
+    printf ("tm_zone = %s\n", result->tm_zone == NULL ? "(null)" : result->tm_zone);
+    ASSERT (strcmp (result->tm_zone, "CEST") == 0);
+#endif
+  }
+
+  /* 1 second before and 1 second after the DST interval started.  */
+  {
+    time_t t = 1174784399; /* 2007-03-25 01:59:59 */
+    struct tm tm;
+    struct tm *result = localtime_r (&t, &tm);
+    ASSERT (result == &tm);
+    ASSERT (result->tm_sec == 59);
+    ASSERT (result->tm_min == 59);
+    ASSERT (result->tm_hour == 1);
+    ASSERT (result->tm_mday == 25);
+    ASSERT (result->tm_mon == 3 - 1);
+    ASSERT (result->tm_year == 2007 - 1900);
+    ASSERT (result->tm_wday == 0);
+    ASSERT (result->tm_yday == 83);
+    ASSERT (result->tm_isdst == 0);
+#if HAVE_STRUCT_TM_TM_GMTOFF
+    ASSERT (result->tm_gmtoff == (1 + result->tm_isdst) * 3600);
+#endif
+#if HAVE_STRUCT_TM_TM_ZONE
+    printf ("tm_zone = %s\n", result->tm_zone == NULL ? "(null)" : result->tm_zone);
+    ASSERT (strcmp (result->tm_zone, "CET") == 0);
+#endif
+  }
+  {
+    time_t t = 1174784401; /* 2007-03-25 03:00:01 */
+    struct tm tm;
+    struct tm *result = localtime_r (&t, &tm);
+    ASSERT (result == &tm);
+    ASSERT (result->tm_sec == 1);
+    ASSERT (result->tm_min == 0);
+    ASSERT (result->tm_hour == 3);
+    ASSERT (result->tm_mday == 25);
+    ASSERT (result->tm_mon == 3 - 1);
+    ASSERT (result->tm_year == 2007 - 1900);
+    ASSERT (result->tm_wday == 0);
+    ASSERT (result->tm_yday == 83);
+    ASSERT (result->tm_isdst == 1);
+#if HAVE_STRUCT_TM_TM_GMTOFF
+    ASSERT (result->tm_gmtoff == (1 + result->tm_isdst) * 3600);
+#endif
+#if HAVE_STRUCT_TM_TM_ZONE
+    printf ("tm_zone = %s\n", result->tm_zone == NULL ? "(null)" : result->tm_zone);
+    ASSERT (strcmp (result->tm_zone, "CEST") == 0);
+#endif
+  }
+
+  /* 1 second before and 1 second after the DST interval ended.  */
+  {
+    time_t t = 1193533199; /* 2007-10-28 02:59:59 */
+    struct tm tm;
+    struct tm *result = localtime_r (&t, &tm);
+    ASSERT (result == &tm);
+    ASSERT (result->tm_sec == 59);
+    ASSERT (result->tm_min == 59);
+    ASSERT (result->tm_hour == 2);
+    ASSERT (result->tm_mday == 28);
+    ASSERT (result->tm_mon == 10 - 1);
+    ASSERT (result->tm_year == 2007 - 1900);
+    ASSERT (result->tm_wday == 0);
+    ASSERT (result->tm_yday == 300);
+    ASSERT (result->tm_isdst == 1);
+#if HAVE_STRUCT_TM_TM_GMTOFF
+    ASSERT (result->tm_gmtoff == (1 + result->tm_isdst) * 3600);
+#endif
+#if HAVE_STRUCT_TM_TM_ZONE
+    printf ("tm_zone = %s\n", result->tm_zone == NULL ? "(null)" : result->tm_zone);
+    ASSERT (strcmp (result->tm_zone, "CEST") == 0);
+#endif
+  }
+  {
+    time_t t = 1193533201; /* 2007-10-28 02:00:01 */
+    struct tm tm;
+    struct tm *result = localtime_r (&t, &tm);
+    ASSERT (result == &tm);
+    ASSERT (result->tm_sec == 1);
+    ASSERT (result->tm_min == 0);
+    ASSERT (result->tm_hour == 2);
+    ASSERT (result->tm_mday == 28);
+    ASSERT (result->tm_mon == 10 - 1);
+    ASSERT (result->tm_year == 2007 - 1900);
+    ASSERT (result->tm_wday == 0);
+    ASSERT (result->tm_yday == 300);
+    ASSERT (result->tm_isdst == 0);
+#if HAVE_STRUCT_TM_TM_GMTOFF
+    ASSERT (result->tm_gmtoff == (1 + result->tm_isdst) * 3600);
+#endif
+#if HAVE_STRUCT_TM_TM_ZONE
+    printf ("tm_zone = %s\n", result->tm_zone == NULL ? "(null)" : result->tm_zone);
+    ASSERT (strcmp (result->tm_zone, "CET") == 0);
+#endif
+  }
+
+  return 0;
+}
-- 
2.34.1

localtime on native Windows

[Bruno Haible]

[-- Attachment #1: Type: text/plain, Size: 1802 bytes --]

I wrote:
> For these modules, the next function to provide in an MT-safe way is
> localtime_r.

Our gmtime_r and localtime_r are MT-safe on native Windows. I ran the
test-gmtime_r-mt and test-localtime_r-mt tests for 2000 seconds each, and
they did not crash.

But the problem is that localtime() and localtime_r() on native Windows
produce nonsensical results:
  - They pretend that in France, in 2007, DST began on 2007-03-11. When in
    fact, it started on 2007-03-25.
  - The hour is wrong.
Witness: The attached program loc.c.

> On native Windows, when the 'localtime_s' function [1][2]
> is not available, such as on the older Windows versions that Emacs cares
> about, the solution is to use GetTimeZoneInformation [3].

None of the GetTimeZoneInformation APIs from the Windows DLLs works either.
They pretend that in German and French time zones, DST starts on March 5,
in all years. Witness: The attached program tzi.c.

So, there is no way around implementing a correct localtime_r, based on
tzdata, in Gnulib.

It will be useful
  - for localtime_r on native Windows,
  - for nstrftime, c_nstrftime, parse-datetime, which all take a timezone_t
    argument.

For reading tzdata: The first question is how to include tzdata in gnulib.
  - AFAICS, the main data file (without comments) is tzdata.zi and is about
    100 KB large. It can be upgraded simply by copying the newest tzdata.zi
    from a newer tzdata distribution. Including such a file in gnulib would
    be OK (re copyright, number of files, total size), right?
  - Whereas including all files from /usr/share/zoneinfo is probably not
    acceptable (> 1300 files, ca. 6 MB total size).
  - Access pattern: In a running program, very few among the time zones will
    be used. Therefore, caching in memory is essential.

Bruno

[-- Attachment #2: loc.c --]
[-- Type: text/x-csrc, Size: 1519 bytes --]

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <time.h>


/* Some common time zone name.  */

#if defined _WIN32 && !defined __CYGWIN__
/* Cf. <https://ss64.com/timezones.html>  */
# define FRENCH_TZ  "Romance Standard Time"
#else
/* Cf. <https://en.wikipedia.org/wiki/List_of_tz_database_time_zones>  */
//# define FRENCH_TZ  "Europe/Paris"
# define FRENCH_TZ  "Europe/Berlin"
#endif


static void
show_localtime_result (time_t t)
{
  struct tm *result = localtime (&t);
  printf ("%ld -> %04d-%02d-%02d %02d:%02d:%02d DST=%d\n",
          (long) t,
          result->tm_year + 1900, result->tm_mon + 1, result->tm_mday,
          result->tm_hour, result->tm_min, result->tm_sec,
          result->tm_isdst);
}

int
main (int argc, char *argv[])
{
#if defined _WIN32 && !defined __CYGWIN__
  _putenv ("TZ" "=" FRENCH_TZ);
#else
  setenv ("TZ", FRENCH_TZ, 1);
#endif

  show_localtime_result (1173578399); /* 2007-03-11 02:59:59 */
  show_localtime_result (1173578401); /* 2007-03-11 03:00:01 */

  show_localtime_result (1174784399); /* 2007-03-25 01:59:59 */
  show_localtime_result (1174784401); /* 2007-03-25 03:00:01 */

  return 0;
}
/*
glibc, Cygwin:
1173578399 -> 2007-03-11 02:59:59 DST=0
1173578401 -> 2007-03-11 03:00:01 DST=0
1174784399 -> 2007-03-25 01:59:59 DST=0
1174784401 -> 2007-03-25 03:00:01 DST=1

Native Windows:
1173578399 -> 2007-03-11 01:59:59 DST=0
1173578401 -> 2007-03-11 03:00:01 DST=1
1174784399 -> 2007-03-25 01:59:59 DST=1
1174784401 -> 2007-03-25 02:00:01 DST=1
*/

[-- Attachment #3: tzi.c --]
[-- Type: text/x-csrc, Size: 4372 bytes --]

#include <stdio.h>
#include <windows.h>
#include <timezoneapi.h>
#include <wchar.h>

/* Cf. <https://ss64.com/timezones.html>  */
# define FRENCH_TZ  "Romance Standard Time"

int main ()
{
  _putenv ("TZ=" FRENCH_TZ);

  DWORD ret;

  TIME_ZONE_INFORMATION info1;
  ret = GetTimeZoneInformation (&info1);
  printf ("GetTimeZoneInformation:\n"
          "ret = %lu  info1 =\n"
          "standard: |%ls| bias=%ld date=%4d-%02d-%02d %02d:%02d:%02d.%03d\n"
          "daylight: |%ls| bias=%ld date=%4d-%02d-%02d %02d:%02d:%02d.%03d\n",
          ret,
          info1.StandardName,
          info1.StandardBias,
          info1.StandardDate.wYear,
          info1.StandardDate.wMonth,
          info1.StandardDate.wDay,
          info1.StandardDate.wHour,
          info1.StandardDate.wMinute,
          info1.StandardDate.wSecond,
          info1.StandardDate.wMilliseconds,
          info1.DaylightName,
          info1.DaylightBias,
          info1.DaylightDate.wYear,
          info1.DaylightDate.wMonth,
          info1.DaylightDate.wDay,
          info1.DaylightDate.wHour,
          info1.DaylightDate.wMinute,
          info1.DaylightDate.wSecond,
          info1.DaylightDate.wMilliseconds);

  DYNAMIC_TIME_ZONE_INFORMATION info2;
  ret = GetDynamicTimeZoneInformation (&info2);
  printf ("GetDynamicTimeZoneInformation:\n"
          "ret = %lu  info2 = bias=%ld timezonekey=%ls dynamicdisabled=%d\n"
          "standard: |%ls| bias=%ld date=%4d-%02d-%02d %02d:%02d:%02d.%03d\n"
          "daylight: |%ls| bias=%ld date=%4d-%02d-%02d %02d:%02d:%02d.%03d\n",
          ret,
          info2.Bias,
          info2.TimeZoneKeyName,
          info2.DynamicDaylightTimeDisabled,
          info2.StandardName,
          info2.StandardBias,
          info2.StandardDate.wYear,
          info2.StandardDate.wMonth,
          info2.StandardDate.wDay,
          info2.StandardDate.wHour,
          info2.StandardDate.wMinute,
          info2.StandardDate.wSecond,
          info2.StandardDate.wMilliseconds,
          info2.DaylightName,
          info2.DaylightBias,
          info2.DaylightDate.wYear,
          info2.DaylightDate.wMonth,
          info2.DaylightDate.wDay,
          info2.DaylightDate.wHour,
          info2.DaylightDate.wMinute,
          info2.DaylightDate.wSecond,
          info2.DaylightDate.wMilliseconds);

  DYNAMIC_TIME_ZONE_INFORMATION info3i;
  TIME_ZONE_INFORMATION info3;
  {
    DWORD i;
    for (i = 0; ; i++)
      {
        if (EnumDynamicTimeZoneInformation (i, &info3i) == ERROR_SUCCESS // Link error in mingw, OK in MSVC.
            && wcscmp (info3i.TimeZoneKeyName, L"Romance Standard Time") == 0)
          break;
      }
  }
  ret = GetTimeZoneInformationForYear (2007, &info3i, &info3);
  printf ("GetTimeZoneInformationForYear(2007):\n"
          "ret = %lu  info3 =\n"
          "standard: |%ls| bias=%ld date=%4d-%02d-%02d %02d:%02d:%02d.%03d\n"
          "daylight: |%ls| bias=%ld date=%4d-%02d-%02d %02d:%02d:%02d.%03d\n",
          ret,
          info3.StandardName,
          info3.StandardBias,
          info3.StandardDate.wYear,
          info3.StandardDate.wMonth,
          info3.StandardDate.wDay,
          info3.StandardDate.wHour,
          info3.StandardDate.wMinute,
          info3.StandardDate.wSecond,
          info3.StandardDate.wMilliseconds,
          info3.DaylightName,
          info3.DaylightBias,
          info3.DaylightDate.wYear,
          info3.DaylightDate.wMonth,
          info3.DaylightDate.wDay,
          info3.DaylightDate.wHour,
          info3.DaylightDate.wMinute,
          info3.DaylightDate.wSecond,
          info3.DaylightDate.wMilliseconds);

}
/* Compile:
   $CC tzi.c -Wall -D_WIN32_WINNT=_WIN32_WINNT_WIN8 -ladvapi32
 */
/* Results:

GetTimeZoneInformation:
ret = 1  info1 =
standard: |W. Europe Standard Time| bias=0 date=   0-10-05 03:00:00.000
daylight: |W. Europe Daylight Time| bias=-60 date=   0-03-05 02:00:00.000
GetDynamicTimeZoneInformation:
ret = 1  info2 = bias=-60 timezonekey=W. Europe Standard Time dynamicdisabled=0
standard: |W. Europe Standard Time| bias=0 date=   0-10-05 03:00:00.000
daylight: |W. Europe Daylight Time| bias=-60 date=   0-03-05 02:00:00.000
GetTimeZoneInformationForYear(2007):
ret = 1  info3 =
standard: |Romance Standard Time| bias=0 date=   0-10-05 03:00:00.000
daylight: |Romance Daylight Time| bias=-60 date=   0-03-05 02:00:00.000

*/

Re: localtime on native Windows

[Paul Eggert]

On 2024-02-11 04:46, Bruno Haible wrote:
> The first question is how to include tzdata in gnulib.
>    - AFAICS, the main data file (without comments) is tzdata.zi and is about
>      100 KB large. It can be upgraded simply by copying the newest tzdata.zi
>      from a newer tzdata distribution. Including such a file in gnulib would
>      be OK (re copyright, number of files, total size), right?

No problem with copyright; the file is in the public domain.

>   - Whereas including all files from /usr/share/zoneinfo is probably not
>     acceptable (> 1300 files, ca. 6 MB total size).

That can be automatically generated from tzdata.zi, by using zic, the 
source code of which is also in the public domain. This is common in 
GNU/Linux systems. If memory serves Android uses another approach: a 
single binary file indexed by timezone name.

Re: localtime on native Windows

[Bruno Haible]

Paul Eggert wrote:
> > The first question is how to include tzdata in gnulib.
> >    - AFAICS, the main data file (without comments) is tzdata.zi and is about
> >      100 KB large. It can be upgraded simply by copying the newest tzdata.zi
> >      from a newer tzdata distribution. Including such a file in gnulib would
> >      be OK (re copyright, number of files, total size), right?
> 
> No problem with copyright; the file is in the public domain.
> 
> >   - Whereas including all files from /usr/share/zoneinfo is probably not
> >     acceptable (> 1300 files, ca. 6 MB total size).
> 
> That can be automatically generated from tzdata.zi, by using zic, the 
> source code of which is also in the public domain.

Thanks for the confirmation.

My current plan, in order to cover the two goals
  - localtime_r on native Windows,
  - nstrftime, c_nstrftime, parse-datetime, which all take a timezone_t
    argument,
is to write four pieces of code:
  1) code that represents the data for a single time zone in memory,
     (based on glibc's tzfile.c),
  2) code that reads a binary time zone file, for use on Unix,
     (based on glibc's tzfile.c), returning it in the format 1).
  3) code that reads a tzdata.zi, for use on Windows,
     (based on glibc's zic.c and tzfile.c), returning it in the format 1).
  4) an implementation of localtime_r for a given time zone
     (based on glibc's tzfile.c:__tzfile_compute, tzset.c:__tz_convert, and
     localtime_r).

This should fulfil Gnulib's needs (code under LGPLv2+, small number
of source files, small number and size of data files to include).

Note that this will have to wait for a few weeks. I have more urgent work on
gettext to do first.

Note also: The above approach will need locking, in order to protect caches
(in 1) the data for a single time zone, in 3) the contents of tzdata.zi).

Bruno

Re: localtime on native Windows

[Paul Eggert]

On 2024-02-17 18:38, Bruno Haible wrote:
> Note also: The above approach will need locking, in order to protect caches
> (in 1) the data for a single time zone, in 3) the contents of tzdata.zi).

The idea I had was to go more in the NetBSD / TZDB direction. These 
implementations don't need either locks or lock-free caching algorithms, 
because it's the caller's responsibility to cache.

Re: localtime on native Windows

[Bruno Haible]

Paul Eggert wrote:
> > Note also: The above approach will need locking, in order to protect caches
> > (in 1) the data for a single time zone, in 3) the contents of tzdata.zi).
> 
> The idea I had was to go more in the NetBSD / TZDB direction. These 
> implementations don't need either locks or lock-free caching algorithms, 
> because it's the caller's responsibility to cache.

I don't understand what you mean:
  - How can the caller do the caching for something internal to the callee?
    I've never seen callbacks being used for that purpose. I've once seen
    a calling convention where the caller allocates memory for a cache and
    passes it to the callee; but that was for a case where the cache's
    lifetime ends when the callee returns.
  - The NetBSD code [1][2] does locking in the top-level functions (tzset,
    mktime, strftime). I guess that it is because NetBSD uses global variables
    (tzname, timezone, daylight, altzone).

For Gnulib, I intend to avoid global variables of this kind — they belong
in data allocated for each time zone — and therefore move the locking to
lower levels. In the frequent case that a timezone is used for the second
time or later, no locking will be needed at all; this will thus be faster
than what NetBSD does.

Bruno

[1] http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/lib/libc/time/localtime.c?rev=1.141&only_with_tag=MAIN
[2] http://cvsweb.netbsd.org/bsdweb.cgi/~checkout~/src/lib/libc/time/strftime.c?rev=1.55

Re: localtime on native Windows

[Paul Eggert]

On 2024-02-18 06:38, Bruno Haible wrote:
>    - How can the caller do the caching for something internal to the callee?

The caller calls tzalloc, gets a timezone object, and calls tzfree when 
it no longer needs that timezone object.

>   - The NetBSD code [1][2] does locking in the top-level functions (tzset,
>     mktime, strftime). I guess that it is because NetBSD uses global variables
>     (tzname, timezone, daylight, altzone).

This is for backward compatibility, and is required for POSIX 
compatibility. It has obvious problems with threads. New code should not 
use these vestigial interfaces. They should use localtime_z and 
mktime_z. (There should be no need for strftime_z.)

https://data.iana.org/time-zones/theory.html#vestigial

> For Gnulib, I intend to avoid global variables of this kind — they belong
> in data allocated for each time zone — and therefore move the locking to
> lower levels. In the frequent case that a timezone is used for the second
> time or later, no locking will be needed at all; this will thus be faster
> than what NetBSD does.

You're right to avoid the global variables, but I don't see why tzalloc 
needs to lock, even in MS-Windows. Perhaps there could be an "I'll do 
the caching" layer above tzalloc, but the lower-level code need not lock 
at all.