From: Tom Lane Date: Tue, 22 Jan 2019 04:18:58 +0000 (-0500) Subject: Avoid thread-safety problem in ecpglib. X-Git-Tag: REL9_6_12~29 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=eb3e90cad1e48ce5ad55335514d639744a295b78;p=postgresql 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 6e9ae2689b..9a7c7849f0 100755 --- a/configure +++ b/configure @@ -12650,7 +12650,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 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 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" @@ -13404,6 +13404,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 86fbdff8d4..ecebc16dbc 100644 --- a/configure.in +++ b/configure.in @@ -1503,7 +1503,32 @@ 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 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 + symlink + sync_file_range + towlower + uselocale + utime + utimes + wcstombs + wcstombs_l +])) AC_REPLACE_FUNCS(fseeko) case $host_os in @@ -1667,6 +1692,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 b81b0670d6..639c18ab57 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -650,6 +650,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 @@ -707,6 +710,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 you have __cpuid. */ #undef HAVE__CPUID diff --git a/src/include/pg_config.h.win32 b/src/include/pg_config.h.win32 index b704b460b0..b32bc5df1e 100644 --- a/src/include/pg_config.h.win32 +++ b/src/include/pg_config.h.win32 @@ -497,6 +497,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 @@ -539,6 +542,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 you have __cpuid. */ #define HAVE__CPUID 1 diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c index 8996561078..3caeff5806 100644 --- 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 5b32e7cbcb..266b961eba 100644 --- 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); } @@ -1779,8 +1784,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) { @@ -1788,6 +1817,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(); @@ -1990,8 +2020,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 c3082beb4a..2af7f0cb8b 100644 --- a/src/interfaces/ecpg/ecpglib/extern.h +++ b/src/interfaces/ecpg/ecpglib/extern.h @@ -12,6 +12,9 @@ #ifndef CHAR_BIT #include #endif +#ifdef LOCALE_T_IN_XLOCALE +#include +#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;