Remove libpq's use of abort(3) to handle mutex failure cases.

tglsfdc · tglsfdc · commit aaddf6ba09e2 · 2021-06-29T11:31:08.000-04:00
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
diff --git a/src/interfaces/libpq/fe-connect.c b/src/interfaces/libpq/fe-connect.c
@@ -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,20 +7278,20 @@ 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);
 	}
 #endif
 	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
 }
diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c
@@ -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 */
diff --git a/src/interfaces/libpq/libpq-int.h b/src/interfaces/libpq/libpq-int.h
@@ -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

Original file line number	Diff line number	Diff line change
`@@ -7254,6 +7254,11 @@ pqGetHomeDirectory(char *buf, int bufsize)`
`7254`	`7254`	`/*`
`7255`	`7255`	`* To keep the API consistent, the locking stubs are always provided, even`
`7256`	`7256`	`* if they are not required.`
	`7257`	`+ *`
	`7258`	`+ * Since we neglected to provide any error-return convention in the`
	`7259`	`+ * pgthreadlock_t API, we can't do much except Assert upon failure of any`
	`7260`	`+ * mutex primitive. Fortunately, such failures appear to be nonexistent in`
	`7261`	`+ * the field.`
`7257`	`7262`	`*/`
`7258`	`7263`
`7259`	`7264`	`static void`
`@@ -7273,20 +7278,20 @@ default_threadlock(int acquire)`
`7273`	`7278`	`if (singlethread_lock == NULL)`
`7274`	`7279`	`{`
`7275`	`7280`	`if (pthread_mutex_init(&singlethread_lock, NULL))`
`7276`		`- PGTHREAD_ERROR("failed to initialize mutex");`
	`7281`	`+ Assert(false);`
`7277`	`7282`	`}`
`7278`	`7283`	`InterlockedExchange(&mutex_initlock, 0);`
`7279`	`7284`	`}`
`7280`	`7285`	`#endif`
`7281`	`7286`	`if (acquire)`
`7282`	`7287`	`{`
`7283`	`7288`	`if (pthread_mutex_lock(&singlethread_lock))`
`7284`		`- PGTHREAD_ERROR("failed to lock mutex");`
	`7289`	`+ Assert(false);`
`7285`	`7290`	`}`
`7286`	`7291`	`else`
`7287`	`7292`	`{`
`7288`	`7293`	`if (pthread_mutex_unlock(&singlethread_lock))`
`7289`		`- PGTHREAD_ERROR("failed to unlock mutex");`
	`7294`	`+ Assert(false);`
`7290`	`7295`	`}`
`7291`	`7296`	`#endif`
`7292`	`7297`	`}`
Original file line number	Diff line number	Diff line change
`@@ -611,15 +611,20 @@ static pthread_mutex_t *pq_lockarray;`
`611`	`611`	`static void`
`612`	`612`	`pq_lockingcallback(int mode, int n, const char *file, int line)`
`613`	`613`	`{`
	`614`	`+ /*`
	`615`	`+ * There's no way to report a mutex-primitive failure, so we just Assert`
	`616`	`+ * in development builds, and ignore any errors otherwise. Fortunately`
	`617`	`+ * this is all obsolete in modern OpenSSL.`
	`618`	`+ */`
`614`	`619`	`if (mode & CRYPTO_LOCK)`
`615`	`620`	`{`
`616`	`621`	`if (pthread_mutex_lock(&pq_lockarray[n]))`
`617`		`- PGTHREAD_ERROR("failed to lock mutex");`
	`622`	`+ Assert(false);`
`618`	`623`	`}`
`619`	`624`	`else`
`620`	`625`	`{`
`621`	`626`	`if (pthread_mutex_unlock(&pq_lockarray[n]))`
`622`		`- PGTHREAD_ERROR("failed to unlock mutex");`
	`627`	`+ Assert(false);`
`623`	`628`	`}`
`624`	`629`	`}`
`625`	`630`	`#endif /* ENABLE_THREAD_SAFETY && HAVE_CRYPTO_LOCK */`