> 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);
 }

Reply via email to