> 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
diff --git a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h index 1c9bce1456..92ff7126d9 100644 --- a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h +++ b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h @@ -61,7 +61,8 @@ struct statement bool questionmarks; struct variable *inlist; struct variable *outlist; - char *oldlocale; + locale_t clocale; + locale_t oldlocale; int nparams; char **paramvalues; PGresult *results; diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 3f5034e792..ba03607d5c 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -102,7 +102,8 @@ free_statement(struct statement *stmt) free_variable(stmt->outlist); ecpg_free(stmt->command); ecpg_free(stmt->name); - ecpg_free(stmt->oldlocale); + if (stmt->clocale) + freelocale(stmt->clocale); ecpg_free(stmt); } @@ -1773,13 +1774,18 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, * Make sure we do NOT honor the locale for numeric input/output since the * database wants the standard decimal point */ - stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno); - if (stmt->oldlocale == NULL) + stmt->clocale = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0); + if (stmt->clocale == (locale_t)0) + { + ecpg_do_epilogue(stmt); + return false; + } + stmt->oldlocale = uselocale(stmt->clocale); + if (stmt->oldlocale == (locale_t)0) { ecpg_do_epilogue(stmt); return false; } - setlocale(LC_NUMERIC, "C"); #ifdef ENABLE_THREAD_SAFETY ecpg_pthreads_init(); @@ -1983,7 +1989,7 @@ ecpg_do_epilogue(struct statement *stmt) return; if (stmt->oldlocale) - setlocale(LC_NUMERIC, stmt->oldlocale); + uselocale(stmt->oldlocale); free_statement(stmt); }