Re: Thread-unsafe coding in ecpg

Michael Meskes <meskes@postgresql.org>

From: Michael Meskes <meskes@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
Date: 2019-01-19T19:15:37Z
Lists: pgsql-hackers

Attachments

> So my conclusion is that this version of setlocale() has some
> thread-safety issues.  I was all set to go file a bug report
> when I noticed this in the POSIX spec for setlocale:
> 
> 	The setlocale() function need not be thread-safe.
> 
> as well as this:
> 
> 	The global locale established using setlocale() shall only be
> used
> 	in threads for which no current locale has been set using
> 	uselocale() or whose current locale has been set to the global
> 	locale using uselocale(LC_GLOBAL_LOCALE).

This one was new to me.

> IOW, not only is setlocale() not necessarily thread-safe itself,
> but using it to change locales in a multithread program is seriously
> unsafe because of concurrent effects on other threads.

Agreed.

> Therefore, it's plain crazy for ecpg to be calling setlocale() inside
> threaded code.  It looks to me like what ecpg is doing is trying to
> defend
> itself against non-C LC_NUMERIC settings, which is laudable, but this
> implementation of that is totally unsafe.
> 
> Don't know what's the best way out of this.  The simplest thing would
> be to just remove that code and document that you'd better run ecpg
> in LC_NUMERIC locale, but it'd be nice if we could do better.

How about attached patch? According to my manpages it should only
affect the calling threat. I only tested it on my own system so far.
Could you please have a look and/or test on other systems? 

Michael
-- 
Michael Meskes
Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Jabber: michael at xmpp dot meskes dot org
VfL Borussia! Força Barça! SF 49ers! Use Debian GNU/Linux, PostgreSQL

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.