diff options
author | Peter Eisentraut <peter@eisentraut.org> | 2025-03-28 21:27:37 +0100 |
---|---|---|
committer | Peter Eisentraut <peter@eisentraut.org> | 2025-03-28 21:27:37 +0100 |
commit | 3c8e463b0d885e0d976f6a13a1fb78187b25c86f (patch) | |
tree | 081e2a55ca02580aa2b069fb0ede573392e98a0d /src | |
parent | 6be53c27673a5fca64a00a684c36c29db6ca33a5 (diff) | |
download | postgresql-3c8e463b0d885e0d976f6a13a1fb78187b25c86f.tar.gz postgresql-3c8e463b0d885e0d976f6a13a1fb78187b25c86f.zip |
Revert "Tidy up locale thread safety in ECPG library."
This reverts commit 8e993bff5326b00ced137c837fce7cd1e0ecae14.
It causes various build failures on the buildfarm, to be investigated.
Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
Diffstat (limited to 'src')
-rw-r--r-- | src/include/pg_config.h.in | 6 | ||||
-rw-r--r-- | src/include/port.h | 31 | ||||
-rw-r--r-- | src/include/port/win32_port.h | 1 | ||||
-rw-r--r-- | src/interfaces/ecpg/ecpglib/connect.c | 39 | ||||
-rw-r--r-- | src/interfaces/ecpg/ecpglib/data.c | 2 | ||||
-rw-r--r-- | src/interfaces/ecpg/ecpglib/descriptor.c | 37 | ||||
-rw-r--r-- | src/interfaces/ecpg/ecpglib/ecpglib_extern.h | 12 | ||||
-rw-r--r-- | src/interfaces/ecpg/ecpglib/execute.c | 53 | ||||
-rw-r--r-- | src/interfaces/ecpg/pgtypeslib/dt_common.c | 6 | ||||
-rw-r--r-- | src/interfaces/ecpg/pgtypeslib/interval.c | 4 | ||||
-rw-r--r-- | src/interfaces/ecpg/pgtypeslib/numeric.c | 2 | ||||
-rw-r--r-- | src/port/Makefile | 1 | ||||
-rw-r--r-- | src/port/locale.c | 84 | ||||
-rw-r--r-- | src/port/meson.build | 1 | ||||
-rw-r--r-- | src/port/snprintf.c | 55 |
15 files changed, 147 insertions, 187 deletions
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index c2427bc9988..c6f055b3905 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -355,9 +355,6 @@ /* Define to 1 if you have the `setproctitle_fast' function. */ #undef HAVE_SETPROCTITLE_FAST -/* Define to 1 if you have the `snprintf_l' function. */ -#undef HAVE_SNPRINTF_L - /* Define to 1 if the system has the type `socklen_t'. */ #undef HAVE_SOCKLEN_T @@ -403,9 +400,6 @@ /* Define to 1 if you have the `strsignal' function. */ #undef HAVE_STRSIGNAL -/* Define to 1 if you have the `strtod_l' function. */ -#undef HAVE_STRTOD_L - /* Define to 1 if the system has the type `struct option'. */ #undef HAVE_STRUCT_OPTION diff --git a/src/include/port.h b/src/include/port.h index 8df0ac59992..3faae03d246 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -218,37 +218,6 @@ extern int pg_fprintf(FILE *stream, const char *fmt,...) pg_attribute_printf(2, extern int pg_vprintf(const char *fmt, va_list args) pg_attribute_printf(1, 0); extern int pg_printf(const char *fmt,...) pg_attribute_printf(1, 2); -/* - * A couple of systems offer a fast constant locale_t value representing the - * "C" locale. We use that if possible, but fall back to creating a singleton - * object otherwise. To check that it is available, call pg_ensure_c_locale() - * and assume out of memory if it returns false. - */ -#ifdef LC_C_LOCALE -#define PG_C_LOCALE LC_C_LOCALE -#define pg_ensure_c_locale() true -#else -extern locale_t pg_get_c_locale(void); -#define PG_C_LOCALE pg_get_c_locale() -#define pg_ensure_c_locale() (PG_C_LOCALE != 0) -#endif - -#if !defined(HAVE_STRTOD_L) && !defined(WIN32) -/* - * POSIX doesn't define this function, but we can implement it with thread-safe - * save-and-restore. - */ -static inline double -strtod_l(const char *nptr, char **endptr, locale_t loc) -{ - locale_t save = uselocale(loc); - double result = strtod(nptr, endptr); - - uselocale(save); - return result; -} -#endif - #ifndef WIN32 /* * We add a pg_ prefix as a warning that the Windows implementations have the diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h index ff0b7a0de18..ff7028bdc81 100644 --- a/src/include/port/win32_port.h +++ b/src/include/port/win32_port.h @@ -453,7 +453,6 @@ extern int _pglstat64(const char *name, struct stat *buf); #define isspace_l _isspace_l #define iswspace_l _iswspace_l #define strcoll_l _strcoll_l -#define strtod_l _strtod_l #define strxfrm_l _strxfrm_l #define wcscoll_l _wcscoll_l diff --git a/src/interfaces/ecpg/ecpglib/connect.c b/src/interfaces/ecpg/ecpglib/connect.c index 475f0215603..2bbb70333dc 100644 --- a/src/interfaces/ecpg/ecpglib/connect.c +++ b/src/interfaces/ecpg/ecpglib/connect.c @@ -10,6 +10,10 @@ #include "ecpgtype.h" #include "sqlca.h" +#ifdef HAVE_USELOCALE +locale_t ecpg_clocale = (locale_t) 0; +#endif + static pthread_mutex_t connections_mutex = PTHREAD_MUTEX_INITIALIZER; static pthread_key_t actual_connection_key; static pthread_once_t actual_connection_key_once = PTHREAD_ONCE_INIT; @@ -264,7 +268,7 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p const char **conn_keywords; const char **conn_values; - if (sqlca == NULL || !pg_ensure_c_locale()) + if (sqlca == NULL) { ecpg_raise(lineno, ECPG_OUT_OF_MEMORY, ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL); @@ -479,6 +483,39 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p /* add connection to our list */ pthread_mutex_lock(&connections_mutex); + /* + * ... but first, make certain we have created ecpg_clocale. Rely on + * holding connections_mutex to ensure this is done by only one thread. + */ +#ifdef HAVE_USELOCALE + if (!ecpg_clocale) + { + ecpg_clocale = newlocale(LC_NUMERIC_MASK, "C", (locale_t) 0); + if (!ecpg_clocale) + { + pthread_mutex_unlock(&connections_mutex); + ecpg_raise(lineno, ECPG_OUT_OF_MEMORY, + ECPG_SQLSTATE_ECPG_OUT_OF_MEMORY, NULL); + if (host) + ecpg_free(host); + if (port) + ecpg_free(port); + if (options) + ecpg_free(options); + if (realname) + ecpg_free(realname); + if (dbname) + ecpg_free(dbname); + if (conn_keywords) + ecpg_free(conn_keywords); + if (conn_values) + ecpg_free(conn_values); + free(this); + return false; + } + } +#endif + if (connection_name != NULL) this->name = ecpg_strdup(connection_name, lineno); else diff --git a/src/interfaces/ecpg/ecpglib/data.c b/src/interfaces/ecpg/ecpglib/data.c index 856f4c9472d..fa562767585 100644 --- a/src/interfaces/ecpg/ecpglib/data.c +++ b/src/interfaces/ecpg/ecpglib/data.c @@ -466,7 +466,7 @@ ecpg_get_data(const PGresult *results, int act_tuple, int act_field, int lineno, pval++; if (!check_special_value(pval, &dres, &scan_length)) - dres = strtod_l(pval, &scan_length, PG_C_LOCALE); + dres = strtod(pval, &scan_length); if (isarray && *scan_length == '"') scan_length++; diff --git a/src/interfaces/ecpg/ecpglib/descriptor.c b/src/interfaces/ecpg/ecpglib/descriptor.c index a769e1b11e9..651d5c8b2ed 100644 --- a/src/interfaces/ecpg/ecpglib/descriptor.c +++ b/src/interfaces/ecpg/ecpglib/descriptor.c @@ -475,9 +475,46 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...) 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 */ + /* (see comments in execute.c) */ +#ifdef HAVE_USELOCALE + + /* + * To get here, the above PQnfields() test must have found nonzero + * fields. One needs a connection to create such a descriptor. (EXEC + * SQL SET DESCRIPTOR can populate the descriptor's "items", but it + * can't change the descriptor's PQnfields().) Any successful + * connection initializes ecpg_clocale. + */ + Assert(ecpg_clocale); + stmt.oldlocale = uselocale(ecpg_clocale); +#else +#ifdef WIN32 + stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE); +#endif + stmt.oldlocale = ecpg_strdup(setlocale(LC_NUMERIC, NULL), lineno); + setlocale(LC_NUMERIC, "C"); +#endif + /* desperate try to guess something sensible */ stmt.connection = ecpg_get_connection(NULL); ecpg_store_result(ECPGresult, index, &stmt, &data_var); + +#ifdef HAVE_USELOCALE + if (stmt.oldlocale != (locale_t) 0) + uselocale(stmt.oldlocale); +#else + if (stmt.oldlocale) + { + setlocale(LC_NUMERIC, stmt.oldlocale); + ecpg_free(stmt.oldlocale); + } +#ifdef WIN32 + 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/ecpglib_extern.h b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h index 40988d53575..75cc68275bd 100644 --- a/src/interfaces/ecpg/ecpglib/ecpglib_extern.h +++ b/src/interfaces/ecpg/ecpglib/ecpglib_extern.h @@ -56,6 +56,10 @@ struct ECPGtype_information_cache enum ARRAY_TYPE isarray; }; +#ifdef HAVE_USELOCALE +extern locale_t ecpg_clocale; /* LC_NUMERIC=C */ +#endif + /* structure to store one statement */ struct statement { @@ -69,6 +73,14 @@ struct statement bool questionmarks; struct variable *inlist; struct variable *outlist; +#ifdef HAVE_USELOCALE + locale_t oldlocale; +#else + char *oldlocale; +#ifdef WIN32 + int oldthreadlocale; +#endif +#endif int nparams; char **paramvalues; int *paramlengths; diff --git a/src/interfaces/ecpg/ecpglib/execute.c b/src/interfaces/ecpg/ecpglib/execute.c index 2bbd8522ef2..f52da06de9a 100644 --- a/src/interfaces/ecpg/ecpglib/execute.c +++ b/src/interfaces/ecpg/ecpglib/execute.c @@ -101,6 +101,9 @@ free_statement(struct statement *stmt) free_variable(stmt->outlist); ecpg_free(stmt->command); ecpg_free(stmt->name); +#ifndef HAVE_USELOCALE + ecpg_free(stmt->oldlocale); +#endif ecpg_free(stmt); } @@ -1971,6 +1974,43 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator, return false; /* + * Make sure we do NOT honor the locale for numeric input/output since the + * database wants the standard decimal point. If available, use + * uselocale() for this because it's thread-safe. Windows doesn't have + * that, but it does have _configthreadlocale(). + */ +#ifdef HAVE_USELOCALE + + /* + * Since ecpg_init() succeeded, we have a connection. Any successful + * connection initializes ecpg_clocale. + */ + Assert(ecpg_clocale); + stmt->oldlocale = uselocale(ecpg_clocale); + if (stmt->oldlocale == (locale_t) 0) + { + ecpg_do_epilogue(stmt); + return false; + } +#else +#ifdef WIN32 + 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) + { + ecpg_do_epilogue(stmt); + return false; + } + setlocale(LC_NUMERIC, "C"); +#endif + + /* * If statement type is ECPGst_prepnormal we are supposed to prepare the * statement before executing them */ @@ -2176,6 +2216,19 @@ 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 WIN32 + _configthreadlocale(stmt->oldthreadlocale); +#endif + } +#endif + free_statement(stmt); } diff --git a/src/interfaces/ecpg/pgtypeslib/dt_common.c b/src/interfaces/ecpg/pgtypeslib/dt_common.c index f8349b66ce9..c4119ab7932 100644 --- a/src/interfaces/ecpg/pgtypeslib/dt_common.c +++ b/src/interfaces/ecpg/pgtypeslib/dt_common.c @@ -1218,7 +1218,7 @@ DecodeNumber(int flen, char *str, int fmask, return DecodeNumberField(flen, str, (fmask | DTK_DATE_M), tmask, tm, fsec, is2digits); - *fsec = strtod_l(cp, &cp, PG_C_LOCALE); + *fsec = strtod(cp, &cp); if (*cp != '\0') return -1; } @@ -2030,7 +2030,7 @@ DecodeDateTime(char **field, int *ftype, int nf, { double frac; - frac = strtod_l(cp, &cp, PG_C_LOCALE); + frac = strtod(cp, &cp); if (*cp != '\0') return -1; *fsec = frac * 1000000; @@ -2054,7 +2054,7 @@ DecodeDateTime(char **field, int *ftype, int nf, { double time; - time = strtod_l(cp, &cp, PG_C_LOCALE); + time = strtod(cp, &cp); if (*cp != '\0') return -1; diff --git a/src/interfaces/ecpg/pgtypeslib/interval.c b/src/interfaces/ecpg/pgtypeslib/interval.c index 155c6cc7770..936a6883816 100644 --- a/src/interfaces/ecpg/pgtypeslib/interval.c +++ b/src/interfaces/ecpg/pgtypeslib/interval.c @@ -60,7 +60,7 @@ ParseISO8601Number(const char *str, char **endptr, int *ipart, double *fpart) if (!(isdigit((unsigned char) *str) || *str == '-' || *str == '.')) return DTERR_BAD_FORMAT; errno = 0; - val = strtod_l(str, endptr, PG_C_LOCALE); + val = strtod(str, endptr); /* did we not see anything that looks like a double? */ if (*endptr == str || errno != 0) return DTERR_BAD_FORMAT; @@ -455,7 +455,7 @@ DecodeInterval(char **field, int *ftype, int nf, /* int range, */ else if (*cp == '.') { errno = 0; - fval = strtod_l(cp, &cp, PG_C_LOCALE); + fval = strtod(cp, &cp); if (*cp != '\0' || errno != 0) return DTERR_BAD_FORMAT; diff --git a/src/interfaces/ecpg/pgtypeslib/numeric.c b/src/interfaces/ecpg/pgtypeslib/numeric.c index 634add7a701..bb2a86981ca 100644 --- a/src/interfaces/ecpg/pgtypeslib/numeric.c +++ b/src/interfaces/ecpg/pgtypeslib/numeric.c @@ -1455,7 +1455,7 @@ numericvar_to_double(numeric *var, double *dp) * strtod does not reset errno to 0 in case of success. */ errno = 0; - val = strtod_l(tmp, &endptr, PG_C_LOCALE); + val = strtod(tmp, &endptr); if (errno == ERANGE) { free(tmp); diff --git a/src/port/Makefile b/src/port/Makefile index 3e51bf1bd65..f11896440d5 100644 --- a/src/port/Makefile +++ b/src/port/Makefile @@ -41,7 +41,6 @@ OBJS = \ bsearch_arg.o \ chklocale.o \ inet_net_ntop.o \ - locale.o \ noblock.o \ path.o \ pg_bitutils.o \ diff --git a/src/port/locale.c b/src/port/locale.c deleted file mode 100644 index b3d48628915..00000000000 --- a/src/port/locale.c +++ /dev/null @@ -1,84 +0,0 @@ -/*------------------------------------------------------------------------- - * - * locale.c - * Helper routines for thread-safe system locale usage. - * - * - * Portions Copyright (c) 1996-2024, PostgreSQL Global Development Group - * Portions Copyright (c) 1994, Regents of the University of California - * - * - * IDENTIFICATION - * src/port/locale.c - * - *------------------------------------------------------------------------- - */ - -#include "c.h" - -#ifndef LC_C_LOCALE - -#ifndef WIN32 -#include <pthread.h> -#else -#include <synchapi.h> -#endif - -/* A process-lifetime singleton, allocated on first need. */ -static locale_t c_locale; - -#ifndef WIN32 -static void -init_c_locale_once(void) -{ - c_locale = newlocale(LC_ALL, "C", NULL); -} -#else -static BOOL -init_c_locale_once(PINIT_ONCE once, PVOID parameter, PVOID *context) -{ - c_locale = _create_locale(LC_ALL, "C"); - return true; -} -#endif - -/* - * Access a process-lifetime singleton locale_t object. Use the macro - * PG_C_LOCALE instead of calling this directly, as it can skip the function - * call on some systems. - */ -locale_t -pg_get_c_locale(void) -{ - /* - * Fast path if already initialized. This assumes that we can read a - * locale_t (in practice, a pointer) without tearing in a multi-threaded - * program. - */ - if (c_locale != (locale_t) 0) - return c_locale; - - /* Make a locale_t. It will live until process exit. */ - { -#ifndef WIN32 - static pthread_once_t once = PTHREAD_ONCE_INIT; - - pthread_once(&once, init_c_locale_once); -#else - static INIT_ONCE once; - InitOnceExecuteOnce(&once, init_c_locale_once, NULL, NULL); -#endif - } - - /* - * It's possible that the allocation of the locale failed due to low - * memory, and then (locale_t) 0 will be returned. Users of PG_C_LOCALE - * should defend against that by checking pg_ensure_c_locale() at a - * convenient time, so that they can treat it as a simple constant after - * that. - */ - - return c_locale; -} - -#endif /* not LC_C_LOCALE */ diff --git a/src/port/meson.build b/src/port/meson.build index 45954dd2808..cf7f07644b9 100644 --- a/src/port/meson.build +++ b/src/port/meson.build @@ -4,7 +4,6 @@ pgport_sources = [ 'bsearch_arg.c', 'chklocale.c', 'inet_net_ntop.c', - 'locale.c', 'noblock.c', 'path.c', 'pg_bitutils.c', diff --git a/src/port/snprintf.c b/src/port/snprintf.c index 376d5c906a0..f8f2018ea0c 100644 --- a/src/port/snprintf.c +++ b/src/port/snprintf.c @@ -109,36 +109,6 @@ #undef vprintf #undef printf -#if defined(FRONTEND) -/* - * Frontend code might be multi-threaded. When calling the system snprintf() - * for floats, we have to use either the non-standard snprintf_l() variant, or - * save-and-restore the calling thread's locale using uselocale(), depending on - * availability. - */ -#if defined(HAVE_SNPRINTF_L) -/* At least macOS and the BSDs have the snprintf_l() extension. */ -#define USE_SNPRINTF_L -#elif defined(WIN32) -/* Windows has a version with a different name and argument order. */ -#define snprintf_l(str, size, loc, format, ...) _snprintf_l(str, size, format, loc, __VA_ARGS__) -#define USE_SNPRINTF_L -#else -/* We have to do a thread-safe save-and-restore around snprintf(). */ -#define NEED_USE_LOCALE -#endif -#else -/* - * Backend code doesn't need to worry about locales here, because LC_NUMERIC is - * set to "C" in main() and doesn't change. Plain old snprintf() is always OK - * without uselocale(). - * - * XXX If the backend were multithreaded, we would have to be 100% certain that - * no one is calling setlocale() concurrently, even transiently, to be able to - * keep this small optimization. - */ -#endif - /* * Info about where the formatted output is going. * @@ -1250,9 +1220,6 @@ fmtfloat(double value, char type, int forcesign, int leftjust, * according to == but not according to memcmp. */ static const double dzero = 0.0; -#ifdef NEED_USE_LOCALE - locale_t save_locale = uselocale(PG_C_LOCALE); -#endif if (adjust_sign((value < 0.0 || (value == 0.0 && @@ -1274,11 +1241,7 @@ fmtfloat(double value, char type, int forcesign, int leftjust, fmt[2] = '*'; fmt[3] = type; fmt[4] = '\0'; -#ifdef USE_SNPRINTF_L - vallen = snprintf_l(convert, sizeof(convert), PG_C_LOCALE, fmt, prec, value); -#else vallen = snprintf(convert, sizeof(convert), fmt, prec, value); -#endif } else { @@ -1287,11 +1250,6 @@ fmtfloat(double value, char type, int forcesign, int leftjust, fmt[2] = '\0'; vallen = snprintf(convert, sizeof(convert), fmt, value); } - -#ifdef NEED_USE_LOCALE - uselocale(save_locale); -#endif - if (vallen < 0) goto fail; @@ -1414,25 +1372,12 @@ pg_strfromd(char *str, size_t count, int precision, double value) } else { -#ifdef NEED_USE_LOCALE - locale_t save_locale = uselocale(PG_C_LOCALE); -#endif - fmt[0] = '%'; fmt[1] = '.'; fmt[2] = '*'; fmt[3] = 'g'; fmt[4] = '\0'; -#ifdef USE_SNPRINTF_L - vallen = snprintf_l(convert, sizeof(convert), PG_C_LOCALE, fmt, precision, value); -#else vallen = snprintf(convert, sizeof(convert), fmt, precision, value); -#endif - -#ifdef NEED_USE_LOCALE - uselocale(save_locale); -#endif - if (vallen < 0) { target.failed = true; |