Re: nl_langinfo-mt failure on Solaris 10 x86

[Bruno Haible]

On 2020-01-03 I wrote:
> > I have one failing test on Solaris 10 x86:
> > 
> > FAIL: test-nl_langinfo-mt
> > =========================
> > 
> > thread5 disturbed by threadN!
> > FAIL test-nl_langinfo-mt (exit status: 134)
> > 
> > 
> > This looks like a new failure to me.
> 
> It's a new test, that verifies that the nl_langinfo function is
> multithread-safe. Its failure is harmless for GNU grep, since this
> program does not use multiple threads so far.
> 
> But I cannot reproduce the failure on the 'unstable10x' machine,
> even when passing the argument '30' (= timeout for the test, in seconds)
> to the test program.
> 
> Could you please tell
>   - on which version of Solaris exactly you're experiencing it,
>   - which compiler and options and configure options you specified,
>   - which of the three locales en_US.UTF-8, fr_FR.UTF-8, de_DE.UTF-8
>     your machine has (check "locale -a" output).

But I could reproduce the failure on the 'unstable11x' machine (Solaris 11.3).
Two parallel threads that only invoke nl_langinfo make the program crash:

static void *
thread5_func (void *arg)
{
  for (;;)
    {
      const char *value = nl_langinfo (CRNCYSTR);
      if (strcmp (expected5, value) != 0)
        {
          fprintf (stderr, "thread5 disturbed by threadN!\n"); fflush (stderr);
          abort ();
        }
    }

  /*NOTREACHED*/
  return NULL;
}

static void *
threadN_func (void *arg)
{
  for (;;)
    {
      nl_langinfo (CODESET);   /* LC_CTYPE */    /* locale charmap */
      //nl_langinfo (AM_STR);    /* LC_TIME */     /* locale -k am_pm */
      //nl_langinfo (PM_STR);    /* LC_TIME */     /* locale -k am_pm */
      //nl_langinfo (DAY_2);     /* LC_TIME */     /* locale -k day */
      //nl_langinfo (DAY_5);     /* LC_TIME */     /* locale -k day */
      //nl_langinfo (ALTMON_2);  /* LC_TIME */     /* locale -k alt_mon */
      //nl_langinfo (ALTMON_9);  /* LC_TIME */     /* locale -k alt_mon */
      nl_langinfo (CRNCYSTR);  /* LC_MONETARY */ /* locale -k currency_symbol */
      //nl_langinfo (RADIXCHAR); /* LC_NUMERIC */  /* locale -k decimal_point */
      //nl_langinfo (THOUSEP);   /* LC_NUMERIC */  /* locale -k thousands_sep */
    }

  /*NOTREACHED*/
  return NULL;
}


This patch fixes it.


2020-09-19  Bruno Haible  <bruno@clisp.org>

        nl_langinfo: Make multithread-safe on Solaris 10 and Solaris 11.3.
        Reported for Solaris 10 by Dagobert Michelsen via Paul Eggert in
        <https://lists.gnu.org/archive/html/bug-gnulib/2020-01/msg00023.html>
        and for Solaris 11.3 by Jeffrey Walton <noloader@gmail.com> in
        <https://lists.gnu.org/archive/html/bug-grep/2020-06/msg00013.html>.
        * lib/nl_langinfo-lock.c: New file, based on lib/setlocale_null-lock.c.
        * lib/nl_langinfo.c: Include <stdlib.h> and <windows.h> or <pthread.h>
        or <threads.h>.
        (ITEMS, MAX_RESULT_LEN): New macros.
        (nl_langinfo_unlocked): New function.
        (gl_get_nl_langinfo_lock): New declaration.
        (nl_langinfo_with_lock): New function, based on lib/setlocale_null.c.
        (rpl_nl_langinfo): Use nl_langinfo_with_lock instead of nl_langinfo.
        * m4/nl_langinfo.m4 (gl_FUNC_NL_LANGINFO): Require gl_PTHREADLIB. Define
        HAVE_THREADS_H. Set NL_LANGINFO_MTSAFE. If setting it to 0, also set
        REPLACE_NL_LANGINFO.
        (gl_PREREQ_NL_LANGINFO_LOCK): New macro.
        * modules/nl_langinfo (Files): Add lib/nl_langinfo-lock.c,
        lib/windows-initguard.h, m4/threadlib.m4, m4/visibility.m4.
        (configure.ac): Compile nl_langinfo-lock.c when NL_LANGINFO_MTSAFE is 0.
        * doc/posix-functions/nl_langinfo.texi: Mention the Solaris bug.

diff --git a/doc/posix-functions/nl_langinfo.texi 
b/doc/posix-functions/nl_langinfo.texi
index 8a65728..6aa3554 100644
--- a/doc/posix-functions/nl_langinfo.texi
+++ b/doc/posix-functions/nl_langinfo.texi
@@ -26,6 +26,9 @@ OpenBSD 6.7.
 The constants @code{YESEXPR} and @code{NOEXPR} do not return a valid
 string on some platforms:
 IRIX 6.5.
+@item
+This function is not multithread-safe on some platforms:
+Solaris 11.3.
 @end itemize
 
 Portability problems not fixed by Gnulib:
diff --git a/lib/nl_langinfo-lock.c b/lib/nl_langinfo-lock.c
new file mode 100644
index 0000000..c2328ab
--- /dev/null
+++ b/lib/nl_langinfo-lock.c
@@ -0,0 +1,150 @@
+/* Return the internal lock used by nl_langinfo.
+   Copyright (C) 2019-2020 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>, 2019-2020.  */
+
+#include <config.h>
+
+/* When it is known that the gl_get_nl_langinfo_lock function is defined
+   by a dependency library, it should not be defined here.  */
+#if OMIT_NL_LANGINFO_LOCK
+
+/* This declaration is solely to ensure that after preprocessing
+   this file is never empty.  */
+typedef int dummy;
+
+#else
+
+/* This file defines the internal lock used by nl_langinfo.
+   It is a separate compilation unit, so that only one copy of it is
+   present when linking statically.  */
+
+/* Prohibit renaming this symbol.  */
+# undef gl_get_nl_langinfo_lock
+
+/* Macro for exporting a symbol (function, not variable) defined in this file,
+   when compiled into a shared library.  */
+# ifndef DLL_EXPORTED
+#  if HAVE_VISIBILITY
+  /* Override the effect of the compiler option '-fvisibility=hidden'.  */
+#   define DLL_EXPORTED __attribute__((__visibility__("default")))
+#  elif defined _WIN32 || defined __CYGWIN__
+#   define DLL_EXPORTED __declspec(dllexport)
+#  else
+#   define DLL_EXPORTED
+#  endif
+# endif
+
+# if defined _WIN32 && !defined __CYGWIN__
+
+#  define WIN32_LEAN_AND_MEAN  /* avoid including junk */
+#  include <windows.h>
+
+#  include "windows-initguard.h"
+
+/* The return type is a 'CRITICAL_SECTION *', not a 'glwthread_mutex_t *',
+   because the latter is not guaranteed to be a stable ABI in the future.  */
+
+/* Make sure the function gets exported from DLLs.  */
+DLL_EXPORTED CRITICAL_SECTION *gl_get_nl_langinfo_lock (void);
+
+static glwthread_initguard_t guard = GLWTHREAD_INITGUARD_INIT;
+static CRITICAL_SECTION lock;
+
+/* Returns the internal lock used by nl_langinfo.  */
+CRITICAL_SECTION *
+gl_get_nl_langinfo_lock (void)
+{
+  if (!guard.done)
+    {
+      if (InterlockedIncrement (&guard.started) == 0)
+        {
+          /* This thread is the first one to need the lock.  Initialize it.  */
+          InitializeCriticalSection (&lock);
+          guard.done = 1;
+        }
+      else
+        {
+          /* Don't let guard.started grow and wrap around.  */
+          InterlockedDecrement (&guard.started);
+          /* Yield the CPU while waiting for another thread to finish
+             initializing this mutex.  */
+          while (!guard.done)
+            Sleep (0);
+        }
+    }
+  return &lock;
+}
+
+# elif HAVE_PTHREAD_API
+
+#  include <pthread.h>
+
+static pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER;
+
+/* Make sure the function gets exported from shared libraries.  */
+DLL_EXPORTED pthread_mutex_t *gl_get_nl_langinfo_lock (void);
+
+/* Returns the internal lock used by nl_langinfo.  */
+pthread_mutex_t *
+gl_get_nl_langinfo_lock (void)
+{
+  return &mutex;
+}
+
+# elif HAVE_THREADS_H
+
+#  include <threads.h>
+#  include <stdlib.h>
+
+static int volatile init_needed = 1;
+static once_flag init_once = ONCE_FLAG_INIT;
+static mtx_t mutex;
+
+static void
+atomic_init (void)
+{
+  if (mtx_init (&mutex, mtx_plain) != thrd_success)
+    abort ();
+  init_needed = 0;
+}
+
+/* Make sure the function gets exported from shared libraries.  */
+DLL_EXPORTED mtx_t *gl_get_nl_langinfo_lock (void);
+
+/* Returns the internal lock used by nl_langinfo.  */
+mtx_t *
+gl_get_nl_langinfo_lock (void)
+{
+  if (init_needed)
+    call_once (&init_once, atomic_init);
+  return &mutex;
+}
+
+# endif
+
+# if (defined _WIN32 || defined __CYGWIN__) && !defined _MSC_VER
+/* Make sure the '__declspec(dllimport)' in nl_langinfo.c does not cause
+   a link failure when no DLLs are involved.  */
+#  if defined _WIN64 || defined _LP64
+#   define IMP(x) __imp_##x
+#  else
+#   define IMP(x) _imp__##x
+#  endif
+void * IMP(gl_get_nl_langinfo_lock) = &gl_get_nl_langinfo_lock;
+# endif
+
+#endif
diff --git a/lib/nl_langinfo.c b/lib/nl_langinfo.c
index 869d831..2b4c0c5 100644
--- a/lib/nl_langinfo.c
+++ b/lib/nl_langinfo.c
@@ -21,6 +21,7 @@
 #include <langinfo.h>
 
 #include <locale.h>
+#include <stdlib.h>
 #include <string.h>
 #if defined _WIN32 && ! defined __CYGWIN__
 # define WIN32_LEAN_AND_MEAN  /* avoid including junk */
@@ -28,6 +29,30 @@
 # include <stdio.h>
 #endif
 
+#if REPLACE_NL_LANGINFO && !NL_LANGINFO_MTSAFE
+# if defined _WIN32 && !defined __CYGWIN__
+
+#  define WIN32_LEAN_AND_MEAN  /* avoid including junk */
+#  include <windows.h>
+
+# elif HAVE_PTHREAD_API
+
+#  include <pthread.h>
+#  if HAVE_THREADS_H && HAVE_WEAK_SYMBOLS
+#   include <threads.h>
+#   pragma weak thrd_exit
+#   define c11_threads_in_use() (thrd_exit != NULL)
+#  else
+#   define c11_threads_in_use() 0
+#  endif
+
+# elif HAVE_THREADS_H
+
+#  include <threads.h>
+
+# endif
+#endif
+
 /* nl_langinfo() must be multithread-safe.  To achieve this without using
    thread-local storage:
      1. We use a specific static buffer for each possible argument.
@@ -117,6 +142,137 @@ ctype_codeset (void)
 
 # undef nl_langinfo
 
+/* Without locking, on Solaris 11.3, test-nl_langinfo-mt fails, with message
+   "thread5 disturbed by threadN!", even when threadN invokes only
+      nl_langinfo (CODESET);
+      nl_langinfo (CRNCYSTR);
+   Similarly on Solaris 10.  */
+
+# if !NL_LANGINFO_MTSAFE /* Solaris */
+
+#  define ITEMS (MAXSTRMSG + 1)
+#  define MAX_RESULT_LEN 80
+
+static char *
+nl_langinfo_unlocked (nl_item item)
+{
+  static char result[ITEMS][MAX_RESULT_LEN];
+
+  /* The result of nl_langinfo is in storage that can be overwritten by
+     other calls to nl_langinfo.  */
+  char *tmp = nl_langinfo (item);
+  if (item >= 0 && item < ITEMS && tmp != NULL)
+    {
+      size_t tmp_len = strlen (tmp);
+      if (tmp_len < MAX_RESULT_LEN)
+        strcpy (result[item], tmp);
+      else
+        {
+          /* Produce a truncated result.  Oh well...  */
+          result[item][MAX_RESULT_LEN - 1] = '\0';
+          memcpy (result[item], tmp, MAX_RESULT_LEN - 1);
+        }
+      return result[item];
+    }
+  else
+    return tmp;
+}
+
+/* Use a lock, so that no two threads can invoke nl_langinfo_unlocked
+   at the same time.  */
+
+/* Prohibit renaming this symbol.  */
+#  undef gl_get_nl_langinfo_lock
+
+#  if defined _WIN32 && !defined __CYGWIN__
+
+extern __declspec(dllimport) CRITICAL_SECTION *gl_get_nl_langinfo_lock (void);
+
+static char *
+nl_langinfo_with_lock (nl_item item)
+{
+  CRITICAL_SECTION *lock = gl_get_nl_langinfo_lock ();
+  char *ret;
+
+  EnterCriticalSection (lock);
+  ret = nl_langinfo_unlocked (item);
+  LeaveCriticalSection (lock);
+
+  return ret;
+}
+
+#  elif HAVE_PTHREAD_API
+
+extern
+#   if defined _WIN32 || defined __CYGWIN__
+  __declspec(dllimport)
+#   endif
+  pthread_mutex_t *gl_get_nl_langinfo_lock (void);
+
+#   if HAVE_WEAK_SYMBOLS /* musl libc, FreeBSD, NetBSD, OpenBSD, Haiku */
+
+     /* Avoid the need to link with '-lpthread'.  */
+#    pragma weak pthread_mutex_lock
+#    pragma weak pthread_mutex_unlock
+
+     /* Determine whether libpthread is in use.  */
+#    pragma weak pthread_mutexattr_gettype
+     /* See the comments in lock.h.  */
+#    define pthread_in_use() \
+       (pthread_mutexattr_gettype != NULL || c11_threads_in_use ())
+
+#   else
+#    define pthread_in_use() 1
+#   endif
+
+static char *
+nl_langinfo_with_lock (nl_item item)
+{
+  if (pthread_in_use())
+    {
+      pthread_mutex_t *lock = gl_get_nl_langinfo_lock ();
+      char *ret;
+
+      if (pthread_mutex_lock (lock))
+        abort ();
+      ret = nl_langinfo_unlocked (item);
+      if (pthread_mutex_unlock (lock))
+        abort ();
+
+      return ret;
+    }
+  else
+    return nl_langinfo_unlocked (item);
+}
+
+#  elif HAVE_THREADS_H
+
+extern mtx_t *gl_get_nl_langinfo_lock (void);
+
+static char *
+nl_langinfo_with_lock (nl_item item)
+{
+  mtx_t *lock = gl_get_nl_langinfo_lock ();
+  char *ret;
+
+  if (mtx_lock (lock) != thrd_success)
+    abort ();
+  ret = nl_langinfo_unlocked (item);
+  if (mtx_unlock (lock) != thrd_success)
+    abort ();
+
+  return ret;
+}
+
+#  endif
+
+# else
+
+/* On other platforms, no lock is needed.  */
+#  define nl_langinfo_with_lock nl_langinfo
+
+# endif
+
 char *
 rpl_nl_langinfo (nl_item item)
 {
@@ -183,7 +339,7 @@ rpl_nl_langinfo (nl_item item)
     default:
       break;
     }
-  return nl_langinfo (item);
+  return nl_langinfo_with_lock (item);
 }
 
 #else
diff --git a/m4/nl_langinfo.m4 b/m4/nl_langinfo.m4
index 6cafe9d..342afd2 100644
--- a/m4/nl_langinfo.m4
+++ b/m4/nl_langinfo.m4
@@ -1,4 +1,4 @@
-# nl_langinfo.m4 serial 7
+# nl_langinfo.m4 serial 8
 dnl Copyright (C) 2009-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -9,8 +9,10 @@ AC_DEFUN([gl_FUNC_NL_LANGINFO],
   AC_REQUIRE([gl_LANGINFO_H_DEFAULTS])
   AC_REQUIRE([gl_LANGINFO_H])
   AC_CHECK_FUNCS_ONCE([nl_langinfo])
-  AC_REQUIRE([AC_CANONICAL_HOST]) dnl for cross-compiles
+  AC_REQUIRE([AC_CANONICAL_HOST])
   AC_REQUIRE([gl_FUNC_SETLOCALE_NULL])
+  AC_REQUIRE([gl_PTHREADLIB])
+  AC_CHECK_HEADERS_ONCE([threads.h])
   if test $ac_cv_func_nl_langinfo = yes; then
     # On Irix 6.5, YESEXPR is defined, but nl_langinfo(YESEXPR) is broken.
     AC_CACHE_CHECK([whether YESEXPR works],
@@ -37,11 +39,19 @@ AC_DEFUN([gl_FUNC_NL_LANGINFO],
     AC_DEFINE_UNQUOTED([FUNC_NL_LANGINFO_YESEXPR_WORKS],
       [$FUNC_NL_LANGINFO_YESEXPR_WORKS],
       [Define to 1 if nl_langinfo (YESEXPR) returns a non-empty string.])
+    # On Solaris 10 and Solaris 11.3, nl_langinfo is not multithread-safe.
+    case "$host_os" in
+      solaris*) NL_LANGINFO_MTSAFE=0 ;;
+      *)        NL_LANGINFO_MTSAFE=1 ;;
+    esac
+    AC_DEFINE_UNQUOTED([NL_LANGINFO_MTSAFE], [$NL_LANGINFO_MTSAFE],
+      [Define to 1 if nl_langinfo is multithread-safe.])
     if test $HAVE_LANGINFO_CODESET = 1 \
        && test $HAVE_LANGINFO_T_FMT_AMPM = 1 \
        && test $HAVE_LANGINFO_ALTMON = 1 \
        && test $HAVE_LANGINFO_ERA = 1 \
-       && test $FUNC_NL_LANGINFO_YESEXPR_WORKS = 1; then
+       && test $FUNC_NL_LANGINFO_YESEXPR_WORKS = 1 \
+       && test $NL_LANGINFO_MTSAFE = 1; then
       :
     else
       REPLACE_NL_LANGINFO=1
@@ -59,3 +69,9 @@ AC_DEFUN([gl_FUNC_NL_LANGINFO],
   dnl LIB_NL_LANGINFO is expected to be empty everywhere.
   AC_SUBST([LIB_NL_LANGINFO])
 ])
+
+# Prerequisites of lib/nl_langinfo-lock.c.
+AC_DEFUN([gl_PREREQ_NL_LANGINFO_LOCK],
+[
+  gl_VISIBILITY
+])
diff --git a/modules/nl_langinfo b/modules/nl_langinfo
index aeeec09..ee6717f 100644
--- a/modules/nl_langinfo
+++ b/modules/nl_langinfo
@@ -3,7 +3,11 @@ nl_langinfo() function: query locale dependent information.
 
 Files:
 lib/nl_langinfo.c
+lib/nl_langinfo-lock.c
+lib/windows-initguard.h
 m4/nl_langinfo.m4
+m4/threadlib.m4
+m4/visibility.m4
 
 Depends-on:
 langinfo
@@ -15,6 +19,10 @@ gl_FUNC_NL_LANGINFO
 if test $HAVE_NL_LANGINFO = 0 || test $REPLACE_NL_LANGINFO = 1; then
   AC_LIBOBJ([nl_langinfo])
 fi
+if test $REPLACE_NL_LANGINFO = 1 && test $NL_LANGINFO_MTSAFE = 0; then
+  AC_LIBOBJ([nl_langinfo-lock])
+  gl_PREREQ_NL_LANGINFO_LOCK
+fi
 gl_LANGINFO_MODULE_INDICATOR([nl_langinfo])
 
 Makefile.am: