Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 52afe56

Browse files
committed
Avoid concurrent calls to bindtextdomain().
We previously supposed that it was okay for different threads to call bindtextdomain() concurrently (cf. commit 1f655fd). 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
1 parent 9440d23 commit 52afe56

File tree

2 files changed

+52
-26
lines changed

2 files changed

+52
-26
lines changed

src/interfaces/ecpg/ecpglib/misc.c

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -512,13 +512,14 @@ char *
512512
ecpg_gettext(const char *msgid)
513513
{
514514
/*
515-
* If multiple threads come through here at about the same time, it's okay
516-
* for more than one of them to call bindtextdomain(). But it's not okay
517-
* for any of them to reach dgettext() before bindtextdomain() is
518-
* complete, so don't set the flag till that's done. Use "volatile" just
519-
* to be sure the compiler doesn't try to get cute.
515+
* At least on Windows, there are gettext implementations that fail if
516+
* multiple threads call bindtextdomain() concurrently. Use a mutex and
517+
* flag variable to ensure that we call it just once per process. It is
518+
* not known that similar bugs exist on non-Windows platforms, but we
519+
* might as well do it the same way everywhere.
520520
*/
521521
static volatile bool already_bound = false;
522+
static pthread_mutex_t binddomain_mutex = PTHREAD_MUTEX_INITIALIZER;
522523

523524
if (!already_bound)
524525
{
@@ -528,14 +529,26 @@ ecpg_gettext(const char *msgid)
528529
#else
529530
int save_errno = errno;
530531
#endif
531-
const char *ldir;
532-
533-
/* No relocatable lookup here because the binary could be anywhere */
534-
ldir = getenv("PGLOCALEDIR");
535-
if (!ldir)
536-
ldir = LOCALEDIR;
537-
bindtextdomain(PG_TEXTDOMAIN("ecpglib"), ldir);
538-
already_bound = true;
532+
533+
(void) pthread_mutex_lock(&binddomain_mutex);
534+
535+
if (!already_bound)
536+
{
537+
const char *ldir;
538+
539+
/*
540+
* No relocatable lookup here because the calling executable could
541+
* be anywhere
542+
*/
543+
ldir = getenv("PGLOCALEDIR");
544+
if (!ldir)
545+
ldir = LOCALEDIR;
546+
bindtextdomain(PG_TEXTDOMAIN("ecpglib"), ldir);
547+
already_bound = true;
548+
}
549+
550+
(void) pthread_mutex_unlock(&binddomain_mutex);
551+
539552
#ifdef WIN32
540553
SetLastError(save_errno);
541554
#else

src/interfaces/libpq/fe-misc.c

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1225,13 +1225,14 @@ static void
12251225
libpq_binddomain(void)
12261226
{
12271227
/*
1228-
* If multiple threads come through here at about the same time, it's okay
1229-
* for more than one of them to call bindtextdomain(). But it's not okay
1230-
* for any of them to return to caller before bindtextdomain() is
1231-
* complete, so don't set the flag till that's done. Use "volatile" just
1232-
* to be sure the compiler doesn't try to get cute.
1228+
* At least on Windows, there are gettext implementations that fail if
1229+
* multiple threads call bindtextdomain() concurrently. Use a mutex and
1230+
* flag variable to ensure that we call it just once per process. It is
1231+
* not known that similar bugs exist on non-Windows platforms, but we
1232+
* might as well do it the same way everywhere.
12331233
*/
12341234
static volatile bool already_bound = false;
1235+
static pthread_mutex_t binddomain_mutex = PTHREAD_MUTEX_INITIALIZER;
12351236

12361237
if (!already_bound)
12371238
{
@@ -1241,14 +1242,26 @@ libpq_binddomain(void)
12411242
#else
12421243
int save_errno = errno;
12431244
#endif
1244-
const char *ldir;
1245-
1246-
/* No relocatable lookup here because the binary could be anywhere */
1247-
ldir = getenv("PGLOCALEDIR");
1248-
if (!ldir)
1249-
ldir = LOCALEDIR;
1250-
bindtextdomain(PG_TEXTDOMAIN("libpq"), ldir);
1251-
already_bound = true;
1245+
1246+
(void) pthread_mutex_lock(&binddomain_mutex);
1247+
1248+
if (!already_bound)
1249+
{
1250+
const char *ldir;
1251+
1252+
/*
1253+
* No relocatable lookup here because the calling executable could
1254+
* be anywhere
1255+
*/
1256+
ldir = getenv("PGLOCALEDIR");
1257+
if (!ldir)
1258+
ldir = LOCALEDIR;
1259+
bindtextdomain(PG_TEXTDOMAIN("libpq"), ldir);
1260+
already_bound = true;
1261+
}
1262+
1263+
(void) pthread_mutex_unlock(&binddomain_mutex);
1264+
12521265
#ifdef WIN32
12531266
SetLastError(save_errno);
12541267
#else

0 commit comments

Comments
 (0)