Avoiding concurrent calls to bindtextdomain()
According to the discussion in [1]/messages/by-id/18312-bbbabc8113592b78@postgresql.org, it's not as safe as we supposed
to allow different threads to call bindtextdomain() concurrently.
Here is a patchset to prevent that by acquiring a mutex around
the libpq and ecpglib calls that are at risk.
In libpq, this would've required yet a third copy of the
Windows-specific ugliness in default_threadlock() and pgtls_init().
I didn't particularly want to do that, so I stole some ideas
from ecpglib to create a more POSIX-compliant emulation of
pthread_mutex_lock(). 0001 attached is the refactoring needed
to make that happen, and then 0002 is the actual bug fix.
0001 also gets rid of the possibility that pthread_mutex_init/
pthread_mutex_lock could fail due to malloc failure. This seems
important since default_threadlock() assumes that pthread_mutex_lock
cannot fail in practice. I observe that ecpglib also assumes that,
although it's using CreateMutex() which has documented failure
conditions. So I wonder if we ought to copy this implementation
back into ecpglib; but I've not done that here.
regards, tom lane
Attachments:
v1-0001-Clean-up-unnecessarily-Windows-dependent-code-in-.patchtext/x-diff; charset=us-ascii; name*0=v1-0001-Clean-up-unnecessarily-Windows-dependent-code-in-.p; name*1=atchDownload+26-48
v1-0002-Avoid-concurrent-calls-to-bindtextdomain.patchtext/x-diff; charset=us-ascii; name=v1-0002-Avoid-concurrent-calls-to-bindtextdomain.patchDownload+52-27
I wrote:
0001 also gets rid of the possibility that pthread_mutex_init/
pthread_mutex_lock could fail due to malloc failure. This seems
important since default_threadlock() assumes that pthread_mutex_lock
cannot fail in practice. I observe that ecpglib also assumes that,
although it's using CreateMutex() which has documented failure
conditions. So I wonder if we ought to copy this implementation
back into ecpglib; but I've not done that here.
The cfbot seemed happy with v1, so here's a v2 that does copy that
code back into ecpglib. (I kind of wonder why this code exists in
libpq + ecpglib at all, rather than in src/port/; but that seems
like a refactoring exercise for another day.)
I double-checked that all the pthread_mutex_t variables in libpq
and ecpglib are static, so the change in that struct should not
pose an ABI hazard for back-patching.
Barring objections, I plan to push this pretty soon.
regards, tom lane