Horton

Re: Thread-unsafe coding in ecpg

Tom Lane <tgl@sss.pgh.pa.us>

View thread View on postgr.es
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Michael Meskes <meskes@postgresql.org>
Cc: pgsql-hackers@lists.postgresql.org
Date: 2019-01-20T17:30:48Z
Lists: pgsql-hackers 
Michael Meskes <meskes@postgresql.org> writes:
>> No, we shouldn't use setlocale(), because it clearly is hazardous
>> even on platforms where it doesn't fail outright.  I don't see
>> anything so wrong with just documenting the hazard.  The situation

> Actually I meant using setlocale() and documenting that it must not be
> used with threads, or document it must not be used with locales?

I tend to think that has more downside than upside, in situations where
people don't read the manual closely and try to do it anyway.

First, there's the probable crash if setlocale() is thread-unsafe.
(Though the lack of previous reports suggests that on most platforms,
it isn't.)

Second, if the program is indeed trying to run with non-C LC_NUMERIC,
using setlocale() will have unsynchronized, hard-to-debug side effects
on other threads.  Not using it will have no downside at all if ecpg
isn't trying to read numeric data, while if it does do so, the failures
will be reproducible and easy to understand/debug.

Admittedly, removing the setlocale() call will be a net negative for
single-threaded applications, which are likely the majority.  But
I don't know any safe way to tell whether the app is multi threaded.

On the third hand, the lack of previous reports suggests that maybe
this whole thing is seldom a problem in practice.  Maybe we should
just use uselocale() where available and otherwise hope it's OK
to keep on doing what we were doing.

			regards, tom lane

Commits

  1. Remove _configthreadlocale() calls in ecpg test suite.

  2. Remove infinite-loop hazards in ecpg test suite.

  3. Blind attempt to fix _configthreadlocale() failures on MinGW.

  4. Avoid thread-safety problem in ecpglib.

  5. Second try at fixing ecpglib thread-safety problem.