Avoid concurrent calls to bindtextdomain().

We previously supposed that it was okay for different threads to
call bindtextdomain() concurrently (cf. commit 1f655fdc3).
It now emerges that there's at least one gettext implementation
in which that triggers an abort() crash, so let's stop doing that.
Add mutexes guarding libpq's and ecpglib's calls, which are the
only ones that need worry about multithreaded callers.

Note: in libpq, we could perhaps have piggybacked on
default_threadlock() to avoid defining a new mutex variable.
I judge that not terribly safe though, since libpq_gettext could
be called from code that is holding the default mutex.  If that
were the first such call in the process, it'd fail.  An extra
mutex is cheap insurance against unforeseen interactions.

Per bug #18312 from Christian Maurer.  Back-patch to all
supported versions.

Discussion: https://postgr.es/m/18312-bbbabc8113592b78@postgresql.org
Discussion: https://postgr.es/m/264860.1707163416@sss.pgh.pa.us

diff --git a/src/interfaces/ecpg/ecpglib/misc.c b/src/interfaces/ecpg/ecpglib/misc.c
index 68fc627f10daecb58d6b9eabc29d3a50fcd752e0..b2c822dfc81b6dd0d3f318b95fe8b151fe7865ab 100644 (file)
--- a/src/interfaces/ecpg/ecpglib/misc.c
+++ b/src/interfaces/ecpg/ecpglib/misc.c
@@ -512,13 +512,14 @@ char *
 ecpg_gettext(const char *msgid)
 {
    /*
-    * If multiple threads come through here at about the same time, it's okay
-    * for more than one of them to call bindtextdomain().  But it's not okay
-    * for any of them to reach dgettext() before bindtextdomain() is
-    * complete, so don't set the flag till that's done.  Use "volatile" just
-    * to be sure the compiler doesn't try to get cute.
+    * At least on Windows, there are gettext implementations that fail if
+    * multiple threads call bindtextdomain() concurrently.  Use a mutex and
+    * flag variable to ensure that we call it just once per process.  It is
+    * not known that similar bugs exist on non-Windows platforms, but we
+    * might as well do it the same way everywhere.
     */
    static volatile bool already_bound = false;
+   static pthread_mutex_t binddomain_mutex = PTHREAD_MUTEX_INITIALIZER;
 
    if (!already_bound)
    {
@@ -528,14 +529,26 @@ ecpg_gettext(const char *msgid)
 #else
        int         save_errno = errno;
 #endif
-       const char *ldir;
-
-       /* No relocatable lookup here because the binary could be anywhere */
-       ldir = getenv("PGLOCALEDIR");
-       if (!ldir)
-           ldir = LOCALEDIR;
-       bindtextdomain(PG_TEXTDOMAIN("ecpglib"), ldir);
-       already_bound = true;
+
+       (void) pthread_mutex_lock(&binddomain_mutex);
+
+       if (!already_bound)
+       {
+           const char *ldir;
+
+           /*
+            * No relocatable lookup here because the calling executable could
+            * be anywhere
+            */
+           ldir = getenv("PGLOCALEDIR");
+           if (!ldir)
+               ldir = LOCALEDIR;
+           bindtextdomain(PG_TEXTDOMAIN("ecpglib"), ldir);
+           already_bound = true;
+       }
+
+       (void) pthread_mutex_unlock(&binddomain_mutex);
+
 #ifdef WIN32
        SetLastError(save_errno);
 #else

diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c
index d76bb3957ae87a78d8cc0e7d452739d538bcadaf..128f056825d6669b2f8cdf7f95d4ba266becce63 100644 (file)
--- a/src/interfaces/libpq/fe-misc.c
+++ b/src/interfaces/libpq/fe-misc.c
@@ -1233,13 +1233,14 @@ static void
 libpq_binddomain(void)
 {
    /*
-    * If multiple threads come through here at about the same time, it's okay
-    * for more than one of them to call bindtextdomain().  But it's not okay
-    * for any of them to return to caller before bindtextdomain() is
-    * complete, so don't set the flag till that's done.  Use "volatile" just
-    * to be sure the compiler doesn't try to get cute.
+    * At least on Windows, there are gettext implementations that fail if
+    * multiple threads call bindtextdomain() concurrently.  Use a mutex and
+    * flag variable to ensure that we call it just once per process.  It is
+    * not known that similar bugs exist on non-Windows platforms, but we
+    * might as well do it the same way everywhere.
     */
    static volatile bool already_bound = false;
+   static pthread_mutex_t binddomain_mutex = PTHREAD_MUTEX_INITIALIZER;
 
    if (!already_bound)
    {
@@ -1249,14 +1250,26 @@ libpq_binddomain(void)
 #else
        int         save_errno = errno;
 #endif
-       const char *ldir;
-
-       /* No relocatable lookup here because the binary could be anywhere */
-       ldir = getenv("PGLOCALEDIR");
-       if (!ldir)
-           ldir = LOCALEDIR;
-       bindtextdomain(PG_TEXTDOMAIN("libpq"), ldir);
-       already_bound = true;
+
+       (void) pthread_mutex_lock(&binddomain_mutex);
+
+       if (!already_bound)
+       {
+           const char *ldir;
+
+           /*
+            * No relocatable lookup here because the calling executable could
+            * be anywhere
+            */
+           ldir = getenv("PGLOCALEDIR");
+           if (!ldir)
+               ldir = LOCALEDIR;
+           bindtextdomain(PG_TEXTDOMAIN("libpq"), ldir);
+           already_bound = true;
+       }
+
+       (void) pthread_mutex_unlock(&binddomain_mutex);
+
 #ifdef WIN32
        SetLastError(save_errno);
 #else