Avoid thread-safety problem in ecpglib.

ecpglib attempts to force the LC_NUMERIC locale to "C" while reading
server output, to avoid problems with strtod() and related functions.
Historically it's just issued setlocale() calls to do that, but that
has major problems if we're in a threaded application.  setlocale()
itself is not required by POSIX to be thread-safe (and indeed is not,
on recent OpenBSD).  Moreover, its effects are process-wide, so that
we could cause unexpected results in other threads, or another thread
could change our setting.

On platforms having uselocale(), which is required by POSIX:2008,
we can avoid these problems by using uselocale() instead.  Windows
goes its own way as usual, but we can make it safe by using
_configthreadlocale().  Platforms having neither continue to use the
old code, but that should be pretty much nobody among current systems.

(Subsequent buildfarm results show that recent NetBSD versions still
lack uselocale(), but it's not a big problem because they also do not
support non-"C" settings for LC_NUMERIC.)

Back-patch of commits 8eb4a9312 and ee27584c4.

Michael Meskes and Tom Lane; thanks also to Takayuki Tsunakawa.

Discussion: https://postgr.es/m/31420.1547783697@sss.pgh.pa.us

diff --git a/configure b/configure
index e8c0341e18c10e3ae3fa4e27becb990a061d4f57..6747f1f7aec32a6bee7a588c7ed972ae5fd44547 100755 (executable)
--- a/configure
+++ b/configure
@@ -12474,7 +12474,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
+for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower uselocale utime utimes wcstombs wcstombs_l
 do :
   as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
 ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var"
@@ -13229,6 +13229,17 @@ fi
 
 # Win32 (really MinGW) support
 if test "$PORTNAME" = "win32"; then
+  for ac_func in _configthreadlocale
+do :
+  ac_fn_c_check_func "$LINENO" "_configthreadlocale" "ac_cv_func__configthreadlocale"
+if test "x$ac_cv_func__configthreadlocale" = xyes; then :
+  cat >>confdefs.h <<_ACEOF
+#define HAVE__CONFIGTHREADLOCALE 1
+_ACEOF
+
+fi
+done
+
   ac_fn_c_check_func "$LINENO" "gettimeofday" "ac_cv_func_gettimeofday"
 if test "x$ac_cv_func_gettimeofday" = xyes; then :
   $as_echo "#define HAVE_GETTIMEOFDAY 1" >>confdefs.h

diff --git a/configure.in b/configure.in
index 54ea8e9570ff42e61676b809dbafebffc780aee6..70c5fb11338a50ca8846349b4dba78c3895f99d2 100644 (file)
--- a/configure.in
+++ b/configure.in
@@ -1437,7 +1437,33 @@ PGAC_FUNC_WCSTOMBS_L
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-AC_CHECK_FUNCS([cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll posix_fallocate pstat pthread_is_threaded_np readlink setproctitle setsid shm_open sigprocmask symlink sync_file_range towlower utime utimes wcstombs wcstombs_l])
+AC_CHECK_FUNCS(m4_normalize([
+   cbrt
+   dlopen
+   fdatasync
+   getifaddrs
+   getpeerucred
+   getrlimit
+   mbstowcs_l
+   memmove
+   poll
+   posix_fallocate
+   pstat
+   pthread_is_threaded_np
+   readlink
+   setproctitle
+   setsid
+   shm_open
+   sigprocmask
+   symlink
+   sync_file_range
+   towlower
+   uselocale
+   utime
+   utimes
+   wcstombs
+   wcstombs_l
+]))
 
 AC_REPLACE_FUNCS(fseeko)
 case $host_os in
@@ -1602,6 +1628,7 @@ fi
 
 # Win32 (really MinGW) support
 if test "$PORTNAME" = "win32"; then
+  AC_CHECK_FUNCS(_configthreadlocale)
   AC_REPLACE_FUNCS(gettimeofday)
   AC_LIBOBJ(dirmod)
   AC_LIBOBJ(kill)

diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 1cfcf8b067ad746283dcd7bb5e5f9703375de527..498799fc9de4e06e07246a30ecbdfd38ffaa6fdd 100644 (file)
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -630,6 +630,9 @@
 /* Define to 1 if the system has the type `unsigned long long int'. */
 #undef HAVE_UNSIGNED_LONG_LONG_INT
 
+/* Define to 1 if you have the `uselocale' function. */
+#undef HAVE_USELOCALE
+
 /* Define to 1 if you have the `utime' function. */
 #undef HAVE_UTIME
 
@@ -681,6 +684,9 @@
 /* Define to 1 if your compiler understands __builtin_unreachable. */
 #undef HAVE__BUILTIN_UNREACHABLE
 
+/* Define to 1 if you have the `_configthreadlocale' function. */
+#undef HAVE__CONFIGTHREADLOCALE
+
 /* Define to 1 if your compiler understands _Static_assert. */
 #undef HAVE__STATIC_ASSERT
 

diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32
index 10ae9ec869e3f5be34283692605ed38e082b02aa..9564324a3b7200ee7e0347242e57617c91d3817f 100644 (file)
--- a/src/include/pg_config.h.win32
+++ b/src/include/pg_config.h.win32
@@ -509,6 +509,9 @@
 /* Define to 1 if you have the `unsetenv' function. */
 /* #undef HAVE_UNSETENV */
 
+/* Define to 1 if you have the `uselocale' function. */
+/* #undef HAVE_USELOCALE */
+
 /* Define to 1 if you have the `utime' function. */
 #define HAVE_UTIME 1
 
@@ -545,6 +548,9 @@
 /* Define to 1 if your compiler understands __builtin_unreachable. */
 /* #undef HAVE__BUILTIN_UNREACHABLE */
 
+/* Define to 1 if you have the `_configthreadlocale' function. */
+#define HAVE__CONFIGTHREADLOCALE 1
+
 /* Define to 1 if your compiler understands _Static_assert. */
 /* #undef HAVE__STATIC_ASSERT */
 

diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c
index 899656107833731a203b873f490adbbdb815a511..3caeff5806d405bb33229ed3bf9c1197bd668c73 100644 (file)
--- a/src/interfaces/ecpg/ecpglib/descriptor.c
+++ b/src/interfaces/ecpg/ecpglib/descriptor.c
@@ -482,22 +482,45 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
    if (data_var.type != ECPGt_EORT)
    {
        struct statement stmt;
-       char       *oldlocale;
+
+       memset(&stmt, 0, sizeof stmt);
+       stmt.lineno = lineno;
 
        /* Make sure we do NOT honor the locale for numeric input */
        /* since the database gives the standard decimal point */
-       oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
+       /* (see comments in execute.c) */
+#ifdef HAVE_USELOCALE
+       stmt.clocale = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0);
+       if (stmt.clocale != (locale_t) 0)
+           stmt.oldlocale = uselocale(stmt.clocale);
+#else
+#ifdef HAVE__CONFIGTHREADLOCALE
+       stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
+#endif
+       stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
        setlocale(LC_NUMERIC, "C");
-
-       memset(&stmt, 0, sizeof stmt);
-       stmt.lineno = lineno;
+#endif
 
        /* desperate try to guess something sensible */
        stmt.connection = ecpg_get_connection(NULL);
        ecpg_store_result(ECPGresult, index, &stmt, &data_var);
 
-       setlocale(LC_NUMERIC, oldlocale);
-       ecpg_free(oldlocale);
+#ifdef HAVE_USELOCALE
+       if (stmt.oldlocale != (locale_t) 0)
+           uselocale(stmt.oldlocale);
+       if (stmt.clocale)
+           freelocale(stmt.clocale);
+#else
+       if (stmt.oldlocale)
+       {
+           setlocale(LC_NUMERIC, stmt.oldlocale);
+           ecpg_free(stmt.oldlocale);
+       }
+#ifdef HAVE__CONFIGTHREADLOCALE
+       if (stmt.oldthreadlocale != -1)
+           _configthreadlocale(stmt.oldthreadlocale);
+#endif
+#endif
    }
    else if (data_var.ind_type != ECPGt_NO_INDICATOR && data_var.ind_pointer != NULL)
 

diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c
index f6cc33c31447d7491bebb1bc482b69bd74ca7d39..eb414f7ca15d4495910157d4d5d966eead099712 100644 (file)
--- a/src/interfaces/ecpg/ecpglib/execute.c
+++ b/src/interfaces/ecpg/ecpglib/execute.c
@@ -104,7 +104,12 @@ free_statement(struct statement * stmt)
    free_variable(stmt->outlist);
    ecpg_free(stmt->command);
    ecpg_free(stmt->name);
+#ifdef HAVE_USELOCALE
+   if (stmt->clocale)
+       freelocale(stmt->clocale);
+#else
    ecpg_free(stmt->oldlocale);
+#endif
    ecpg_free(stmt);
 }
 
@@ -1772,8 +1777,32 @@ 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
+    * database wants the standard decimal point.  If available, use
+    * uselocale() for this because it's thread-safe.  Windows doesn't have
+    * that, but it usually does have _configthreadlocale().
     */
+#ifdef HAVE_USELOCALE
+   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;
+   }
+#else
+#ifdef HAVE__CONFIGTHREADLOCALE
+   stmt->oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
+   if (stmt->oldthreadlocale == -1)
+   {
+       ecpg_do_epilogue(stmt);
+       return false;
+   }
+#endif
    stmt->oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno);
    if (stmt->oldlocale == NULL)
    {
@@ -1781,6 +1810,7 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
        return false;
    }
    setlocale(LC_NUMERIC, "C");
+#endif
 
 #ifdef ENABLE_THREAD_SAFETY
    ecpg_pthreads_init();
@@ -1983,8 +2013,18 @@ ecpg_do_epilogue(struct statement * stmt)
    if (stmt == NULL)
        return;
 
+#ifdef HAVE_USELOCALE
+   if (stmt->oldlocale != (locale_t) 0)
+       uselocale(stmt->oldlocale);
+#else
    if (stmt->oldlocale)
+   {
        setlocale(LC_NUMERIC, stmt->oldlocale);
+#ifdef HAVE__CONFIGTHREADLOCALE
+       _configthreadlocale(stmt->oldthreadlocale);
+#endif
+   }
+#endif
 
    free_statement(stmt);
 }

diff --git a/src/interfaces/ecpg/ecpglib/extern.h b/src/interfaces/ecpg/ecpglib/extern.h
index f0752bb9ca5ce6086fab3c0f3fd1544f5b37d11c..f4f8f4dfea34125cdb1f538d074d2b7d4917d19a 100644 (file)
--- a/src/interfaces/ecpg/ecpglib/extern.h
+++ b/src/interfaces/ecpg/ecpglib/extern.h
@@ -12,6 +12,9 @@
 #ifndef CHAR_BIT
 #include <limits.h>
 #endif
+#ifdef LOCALE_T_IN_XLOCALE
+#include <xlocale.h>
+#endif
 
 enum COMPAT_MODE
 {
@@ -60,7 +63,15 @@ struct statement
    bool        questionmarks;
    struct variable *inlist;
    struct variable *outlist;
+#ifdef HAVE_USELOCALE
+   locale_t    clocale;
+   locale_t    oldlocale;
+#else
    char       *oldlocale;
+#ifdef HAVE__CONFIGTHREADLOCALE
+   int         oldthreadlocale;
+#endif
+#endif
    int         nparams;
    char      **paramvalues;
    PGresult   *results;