Message-ID: <0be2e83c4dec0d2f1f8eb55b1e95ad98b12ede24.camel@postgresql.org>
Subject: Re: Thread-unsafe coding in ecpg
From: Michael Meskes <meskes@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers@lists.postgresql.org
Date: Sat, 19 Jan 2019 20:15:37 +0100
In-Reply-To: <31420.1547783697@sss.pgh.pa.us>
References: <31420.1547783697@sss.pgh.pa.us>
Organization: PostgreSQL
Content-Type: multipart/mixed; boundary="=-aL79VPUAvp+VuQhOLlOn"
User-Agent: Evolution 3.30.4-1 
Mime-Version: 1.0
Precedence: bulk


--=-aL79VPUAvp+VuQhOLlOn
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit

> 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

--=-aL79VPUAvp+VuQhOLlOn
Content-Disposition: attachment; filename="ecpg_locale.diff"
Content-Type: text/x-patch; name="ecpg_locale.diff"; charset="UTF-8"
Content-Transfer-Encoding: base64

ZGlmZiAtLWdpdCBhL3NyYy9pbnRlcmZhY2VzL2VjcGcvZWNwZ2xpYi9lY3BnbGliX2V4dGVybi5o
IGIvc3JjL2ludGVyZmFjZXMvZWNwZy9lY3BnbGliL2VjcGdsaWJfZXh0ZXJuLmgKaW5kZXggMWM5
YmNlMTQ1Ni4uOTJmZjcxMjZkOSAxMDA2NDQKLS0tIGEvc3JjL2ludGVyZmFjZXMvZWNwZy9lY3Bn
bGliL2VjcGdsaWJfZXh0ZXJuLmgKKysrIGIvc3JjL2ludGVyZmFjZXMvZWNwZy9lY3BnbGliL2Vj
cGdsaWJfZXh0ZXJuLmgKQEAgLTYxLDcgKzYxLDggQEAgc3RydWN0IHN0YXRlbWVudAogCWJvb2wJ
CXF1ZXN0aW9ubWFya3M7CiAJc3RydWN0IHZhcmlhYmxlICppbmxpc3Q7CiAJc3RydWN0IHZhcmlh
YmxlICpvdXRsaXN0OwotCWNoYXIJICAgKm9sZGxvY2FsZTsKKwlsb2NhbGVfdAljbG9jYWxlOwor
CWxvY2FsZV90CW9sZGxvY2FsZTsKIAlpbnQJCQlucGFyYW1zOwogCWNoYXIJICAqKnBhcmFtdmFs
dWVzOwogCVBHcmVzdWx0ICAgKnJlc3VsdHM7CmRpZmYgLS1naXQgYS9zcmMvaW50ZXJmYWNlcy9l
Y3BnL2VjcGdsaWIvZXhlY3V0ZS5jIGIvc3JjL2ludGVyZmFjZXMvZWNwZy9lY3BnbGliL2V4ZWN1
dGUuYwppbmRleCAzZjUwMzRlNzkyLi5iYTAzNjA3ZDVjIDEwMDY0NAotLS0gYS9zcmMvaW50ZXJm
YWNlcy9lY3BnL2VjcGdsaWIvZXhlY3V0ZS5jCisrKyBiL3NyYy9pbnRlcmZhY2VzL2VjcGcvZWNw
Z2xpYi9leGVjdXRlLmMKQEAgLTEwMiw3ICsxMDIsOCBAQCBmcmVlX3N0YXRlbWVudChzdHJ1Y3Qg
c3RhdGVtZW50ICpzdG10KQogCWZyZWVfdmFyaWFibGUoc3RtdC0+b3V0bGlzdCk7CiAJZWNwZ19m
cmVlKHN0bXQtPmNvbW1hbmQpOwogCWVjcGdfZnJlZShzdG10LT5uYW1lKTsKLQllY3BnX2ZyZWUo
c3RtdC0+b2xkbG9jYWxlKTsKKwlpZiAoc3RtdC0+Y2xvY2FsZSkKKwkJZnJlZWxvY2FsZShzdG10
LT5jbG9jYWxlKTsKIAllY3BnX2ZyZWUoc3RtdCk7CiB9CiAKQEAgLTE3NzMsMTMgKzE3NzQsMTgg
QEAgZWNwZ19kb19wcm9sb2d1ZShpbnQgbGluZW5vLCBjb25zdCBpbnQgY29tcGF0LCBjb25zdCBp
bnQgZm9yY2VfaW5kaWNhdG9yLAogCSAqIE1ha2Ugc3VyZSB3ZSBkbyBOT1QgaG9ub3IgdGhlIGxv
Y2FsZSBmb3IgbnVtZXJpYyBpbnB1dC9vdXRwdXQgc2luY2UgdGhlCiAJICogZGF0YWJhc2Ugd2Fu
dHMgdGhlIHN0YW5kYXJkIGRlY2ltYWwgcG9pbnQKIAkgKi8KLQlzdG10LT5vbGRsb2NhbGUgPSBl
Y3BnX3N0cmR1cChzZXRsb2NhbGUoTENfTlVNRVJJQywgTlVMTCksIGxpbmVubyk7Ci0JaWYgKHN0
bXQtPm9sZGxvY2FsZSA9PSBOVUxMKQorCXN0bXQtPmNsb2NhbGUgPSBuZXdsb2NhbGUoTENfTlVN
RVJJQ19NQVNLLCAiQyIsIChsb2NhbGVfdCkgMCk7CisJaWYgKHN0bXQtPmNsb2NhbGUgPT0gKGxv
Y2FsZV90KTApCisJeworCQllY3BnX2RvX2VwaWxvZ3VlKHN0bXQpOworCQlyZXR1cm4gZmFsc2U7
CisJfQorCXN0bXQtPm9sZGxvY2FsZSA9IHVzZWxvY2FsZShzdG10LT5jbG9jYWxlKTsKKwlpZiAo
c3RtdC0+b2xkbG9jYWxlID09IChsb2NhbGVfdCkwKQogCXsKIAkJZWNwZ19kb19lcGlsb2d1ZShz
dG10KTsKIAkJcmV0dXJuIGZhbHNlOwogCX0KLQlzZXRsb2NhbGUoTENfTlVNRVJJQywgIkMiKTsK
IAogI2lmZGVmIEVOQUJMRV9USFJFQURfU0FGRVRZCiAJZWNwZ19wdGhyZWFkc19pbml0KCk7CkBA
IC0xOTgzLDcgKzE5ODksNyBAQCBlY3BnX2RvX2VwaWxvZ3VlKHN0cnVjdCBzdGF0ZW1lbnQgKnN0
bXQpCiAJCXJldHVybjsKIAogCWlmIChzdG10LT5vbGRsb2NhbGUpCi0JCXNldGxvY2FsZShMQ19O
VU1FUklDLCBzdG10LT5vbGRsb2NhbGUpOworCQl1c2Vsb2NhbGUoc3RtdC0+b2xkbG9jYWxlKTsK
IAogCWZyZWVfc3RhdGVtZW50KHN0bXQpOwogfQo=


--=-aL79VPUAvp+VuQhOLlOn--