Remove libpq's use of abort(3) to handle mutex failure cases.
authorTom Lane <[email protected]>
Tue, 29 Jun 2021 15:31:08 +0000 (11:31 -0400)
committerTom Lane <[email protected]>
Tue, 29 Jun 2021 15:31:08 +0000 (11:31 -0400)
Doing an abort() seems all right in development builds, but not in
production builds of general-purpose libraries.  However, the functions
that were doing this lack any way to report a failure back up to their
callers.  It seems like we can just get away with ignoring failures in
production builds, since (a) no such failures have been reported in the
dozen years that the code's been like this, and (b) failure to enforce
mutual exclusion during fe-auth.c operations would likely not cause any
problems anyway in most cases.  (The OpenSSL callbacks that use this
macro are obsolete, so even less likely to cause interesting problems.)

Possibly a better answer would be to break compatibility of the
pgthreadlock_t callback API, but in the absence of field problem
reports, it doesn't really seem worth the trouble.

Discussion: https://postgr.es/m/3131385.1624746109@sss.pgh.pa.us

src/interfaces/libpq/fe-connect.c
src/interfaces/libpq/fe-secure-openssl.c
src/interfaces/libpq/libpq-int.h

index 3faf05a7e71877540938165c626e968712b30771..fc65e490ef8244d0ef438508235821bdb0f5e1c5 100644 (file)
@@ -7254,6 +7254,11 @@ pqGetHomeDirectory(char *buf, int bufsize)
 /*
  * To keep the API consistent, the locking stubs are always provided, even
  * if they are not required.
+ *
+ * Since we neglected to provide any error-return convention in the
+ * pgthreadlock_t API, we can't do much except Assert upon failure of any
+ * mutex primitive.  Fortunately, such failures appear to be nonexistent in
+ * the field.
  */
 
 static void
@@ -7273,7 +7278,7 @@ default_threadlock(int acquire)
        if (singlethread_lock == NULL)
        {
            if (pthread_mutex_init(&singlethread_lock, NULL))
-               PGTHREAD_ERROR("failed to initialize mutex");
+               Assert(false);
        }
        InterlockedExchange(&mutex_initlock, 0);
    }
@@ -7281,12 +7286,12 @@ default_threadlock(int acquire)
    if (acquire)
    {
        if (pthread_mutex_lock(&singlethread_lock))
-           PGTHREAD_ERROR("failed to lock mutex");
+           Assert(false);
    }
    else
    {
        if (pthread_mutex_unlock(&singlethread_lock))
-           PGTHREAD_ERROR("failed to unlock mutex");
+           Assert(false);
    }
 #endif
 }
index 67feaedc4e07b3fda8f8e8e52e28c4be67a88e5d..2ee5a0a40aad905891d1a14ae206c16a53d0d5f8 100644 (file)
@@ -611,15 +611,20 @@ static pthread_mutex_t *pq_lockarray;
 static void
 pq_lockingcallback(int mode, int n, const char *file, int line)
 {
+   /*
+    * There's no way to report a mutex-primitive failure, so we just Assert
+    * in development builds, and ignore any errors otherwise.  Fortunately
+    * this is all obsolete in modern OpenSSL.
+    */
    if (mode & CRYPTO_LOCK)
    {
        if (pthread_mutex_lock(&pq_lockarray[n]))
-           PGTHREAD_ERROR("failed to lock mutex");
+           Assert(false);
    }
    else
    {
        if (pthread_mutex_unlock(&pq_lockarray[n]))
-           PGTHREAD_ERROR("failed to unlock mutex");
+           Assert(false);
    }
 }
 #endif                         /* ENABLE_THREAD_SAFETY && HAVE_CRYPTO_LOCK */
index e81dc37906b8bdc1f58a705e522d5c3075b62c20..6b7fd2c267ad246897af40d649a5d70855aaa174 100644 (file)
@@ -626,13 +626,6 @@ extern bool pqGetHomeDirectory(char *buf, int bufsize);
 #ifdef ENABLE_THREAD_SAFETY
 extern pgthreadlock_t pg_g_threadlock;
 
-#define PGTHREAD_ERROR(msg) \
-   do { \
-       fprintf(stderr, "%s\n", msg); \
-       abort(); \
-   } while (0)
-
-
 #define pglock_thread()        pg_g_threadlock(true)
 #define pgunlock_thread()  pg_g_threadlock(false)
 #else