]> git.ipfire.org Git - thirdparty/postgresql.git/commitdiff
ecpglib: call newlocale() once per process.
authorNoah Misch <noah@leadboat.com>
Sat, 2 Jul 2022 20:00:30 +0000 (13:00 -0700)
committerNoah Misch <noah@leadboat.com>
Sat, 2 Jul 2022 20:00:35 +0000 (13:00 -0700)
ecpglib has been calling it once per SQL query and once per EXEC SQL GET
DESCRIPTOR.  Instead, if newlocale() has not succeeded before, call it
while establishing a connection.  This mitigates three problems:
- If newlocale() failed in EXEC SQL GET DESCRIPTOR, the command silently
  proceeded without the intended locale change.
- On AIX, each newlocale()+freelocale() cycle leaked memory.
- newlocale() CPU usage may have been nontrivial.

Fail the connection attempt if newlocale() fails.  Rearrange
ecpg_do_prologue() to validate the connection before its uselocale().

The sort of program that may regress is one running in an environment
where newlocale() fails.  If that program establishes connections
without running SQL statements, it will stop working in response to this
change.  I'm betting against the importance of such an ECPG use case.
Most SQL execution (any using ECPGdo()) has long required newlocale()
success, so there's little a connection could do without newlocale().

Back-patch to v10 (all supported versions).

Reviewed by Tom Lane.  Reported by Guillaume Lelarge.

Discussion: https://postgr.es/m/20220101074055.GA54621@rfd.leadboat.com

src/interfaces/ecpg/ecpglib/connect.c
src/interfaces/ecpg/ecpglib/descriptor.c
src/interfaces/ecpg/ecpglib/execute.c
src/interfaces/ecpg/ecpglib/extern.h

index 96b29237b5228e02a14793fc32e02d48431acf3d..0e405ebe75948f8c8b5ebd05a2b52e42ef6fd1dc 100644 (file)
 #include "extern.h"
 #include "sqlca.h"
 
+#ifdef HAVE_USELOCALE
+locale_t       ecpg_clocale;
+#endif
+
 #ifdef ENABLE_THREAD_SAFETY
 static pthread_mutex_t connections_mutex = PTHREAD_MUTEX_INITIALIZER;
 static pthread_key_t actual_connection_key;
@@ -531,6 +535,42 @@ ECPGconnect(int lineno, int c, const char *name, const char *user, const char *p
 #ifdef ENABLE_THREAD_SAFETY
        pthread_mutex_lock(&connections_mutex);
 #endif
+
+       /*
+        * ... 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)
+               {
+#ifdef ENABLE_THREAD_SAFETY
+                       pthread_mutex_unlock(&connections_mutex);
+#endif
+                       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
index 99cf3aa7beadd519c6e0665df9b4e8fa79e815ff..c097adfa1a5124272380105ccf1e9b3d7f344610 100644 (file)
@@ -491,9 +491,16 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
                /* since the database gives the standard decimal point */
                /* (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);
+
+               /*
+                * 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 HAVE__CONFIGTHREADLOCALE
                stmt.oldthreadlocale = _configthreadlocale(_ENABLE_PER_THREAD_LOCALE);
@@ -509,8 +516,6 @@ ECPGget_desc(int lineno, const char *desc_name, int index,...)
 #ifdef HAVE_USELOCALE
                if (stmt.oldlocale != (locale_t) 0)
                        uselocale(stmt.oldlocale);
-               if (stmt.clocale)
-                       freelocale(stmt.clocale);
 #else
                if (stmt.oldlocale)
                {
index 1d84b59c7cdbe16a1e3cc5703f5b88a93d0bb6cf..02c2087e51f0f2b566da4acebd48db94129190bc 100644 (file)
@@ -103,10 +103,7 @@ 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
+#ifndef HAVE_USELOCALE
        ecpg_free(stmt->oldlocale);
 #endif
        ecpg_free(stmt);
@@ -1776,6 +1773,15 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
                return false;
        }
 
+#ifdef ENABLE_THREAD_SAFETY
+       ecpg_pthreads_init();
+#endif
+
+       con = ecpg_get_connection(connection_name);
+
+       if (!ecpg_init(con, connection_name, lineno))
+               return false;
+
        stmt = (struct statement *) ecpg_alloc(sizeof(struct statement), lineno);
 
        if (stmt == NULL)
@@ -1790,13 +1796,13 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
         * treat that situation as if the function doesn't exist.
         */
 #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);
+
+       /*
+        * 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);
@@ -1815,18 +1821,6 @@ ecpg_do_prologue(int lineno, const int compat, const int force_indicator,
        setlocale(LC_NUMERIC, "C");
 #endif
 
-#ifdef ENABLE_THREAD_SAFETY
-       ecpg_pthreads_init();
-#endif
-
-       con = ecpg_get_connection(connection_name);
-
-       if (!ecpg_init(con, connection_name, lineno))
-       {
-               ecpg_do_epilogue(stmt);
-               return false;
-       }
-
        /*
         * If statement type is ECPGst_prepnormal we are supposed to prepare the
         * statement before executing them
index 04c81116091689d691905d57a5c0c3eb32809d52..415244971304828d385da768d8d8d0d997f63765 100644 (file)
@@ -51,6 +51,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
 {
@@ -65,7 +69,6 @@ struct statement
        struct variable *inlist;
        struct variable *outlist;
 #ifdef HAVE_USELOCALE
-       locale_t        clocale;
        locale_t        oldlocale;
 #else
        char       *oldlocale;