Revert "Tidy up locale thread safety in ECPG library."

petere · petere · commit 3c8e463b0d88 · 2025-03-28T21:27:37.000+01:00
This reverts commit 8e993bf. It causes various build failures on the buildfarm, to be investigated. Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech
diff --git a/configure b/configure
@@ -15401,7 +15401,7 @@ fi
 LIBS_including_readline="$LIBS"
 LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
 
-for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast snprintf_l strchrnul strsignal strtod_l syncfs sync_file_range uselocale wcstombs_l
+for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale 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"
diff --git a/configure.ac b/configure.ac
@@ -1772,10 +1772,8 @@ AC_CHECK_FUNCS(m4_normalize([
 	pthread_is_threaded_np
 	setproctitle
 	setproctitle_fast
-	snprintf_l
 	strchrnul
 	strsignal
-	strtod_l
 	syncfs
 	sync_file_range
 	uselocale
diff --git a/meson.build b/meson.build
@@ -2753,7 +2753,6 @@ func_checks = [
   ['shm_open', {'dependencies': [rt_dep], 'define': false}],
   ['shm_unlink', {'dependencies': [rt_dep], 'define': false}],
   ['shmget', {'dependencies': [cygipc_dep], 'define': false}],
-  ['snprintf_l'],
   ['socket', {'dependencies': [socket_dep], 'define': false}],
   ['strchrnul'],
   ['strerror_r', {'dependencies': [thread_dep]}],
@@ -2762,7 +2761,6 @@ func_checks = [
   ['strnlen'],
   ['strsep'],
   ['strsignal'],
-  ['strtod_l'],
   ['sync_file_range'],
   ['syncfs'],
   ['uselocale'],
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);
 }
 
@@ -1970,6 +1973,43 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
 	if (stmt == NULL)
 		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
@@ -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
@@ -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
@@ -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
@@ -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
diff --git a/src/port/meson.build b/src/port/meson.build
diff --git a/src/port/snprintf.c b/src/port/snprintf.c
