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

This reverts commit 8e993bff53.

It causes various build failures on the buildfarm, to be investigated.

Discussion: https://postgr.es/m/CWZBBRR6YA8D.8EHMDRGLCKCD%40neon.tech

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"

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

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'],

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
 

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

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
 

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

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++;

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)
 

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;

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

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;
 

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;
 

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

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 \

src/port/locale.c

@@ -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 */

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',

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;