Don't treat EINVAL from semget() as a hard failure.

tglsfdc · tglsfdc · commit e3b3fa863810 · 2025-08-13T11:59:47.000-04:00
It turns out that on some platforms (at least current macOS, NetBSD, OpenBSD) semget(2) will return EINVAL if there is a pre-existing semaphore set with the same key and too few semaphores. Our code expects EEXIST in that case and treats EINVAL as a hard failure, resulting in failure during initdb or postmaster start. POSIX does document EINVAL for too-few-semaphores-in-set, and is silent on its priority relative to EEXIST, so this behavior arguably conforms to spec. Nonetheless it's quite problematic because EINVAL is also documented to mean that nsems is greater than the system's limit on the number of semaphores per set (SEMMSL). If that is where the problem lies, retrying would just become an infinite loop. To resolve this contradiction, retry after EINVAL, but also install a loop limit that will make us give up regardless of the specific errno after trying 1000 different keys. (1000 is a pretty arbitrary number, but it seems like it should be sufficient.) I like this better than the previous infinite-looping behavior, since it will also keep us out of trouble if (say) we get EACCES due to a system-level permissions problem rather than anything to do with a specific semaphore set. This problem has only been observed in the field in PG 17, which uses a higher nsems value than other branches (cf. 38da053, 810a8b1). That makes it possible to get the failure if a new v17 postmaster has a key collision with an existing postmaster of another branch. In principle though, we might see such a collision against a semaphore set created by some other application, in which case all branches are vulnerable on these platforms. Hence, backpatch. Reported-by: Gavin Panella <gavinpanella@gmail.com> Author: Tom Lane <tgl@sss.pgh.pa.us> Discussion: https://postgr.es/m/CALL7chmzY3eXHA7zHnODUVGZLSvK3wYCSP0RmcDFHJY8f28Q3g@mail.gmail.com Backpatch-through: 13
diff --git a/src/backend/port/sysv_sema.c b/src/backend/port/sysv_sema.c
@@ -73,7 +73,7 @@ static int	nextSemaNumber;		/* next free sem num in last sema set */
 
 
 static IpcSemaphoreId InternalIpcSemaphoreCreate(IpcSemaphoreKey semKey,
-												 int numSems);
+												 int numSems, bool retry_ok);
 static void IpcSemaphoreInitialize(IpcSemaphoreId semId, int semNum,
 								   int value);
 static void IpcSemaphoreKill(IpcSemaphoreId semId);
@@ -92,9 +92,13 @@ static void ReleaseSemaphores(int status, Datum arg);
  * If we fail with a failure code other than collision-with-existing-set,
  * print out an error and abort.  Other types of errors suggest nonrecoverable
  * problems.
+ *
+ * Unfortunately, it's sometimes hard to tell whether errors are
+ * nonrecoverable.  Our caller keeps track of whether continuing to retry
+ * is sane or not; if not, we abort on failure regardless of the errno.
  */
 static IpcSemaphoreId
-InternalIpcSemaphoreCreate(IpcSemaphoreKey semKey, int numSems)
+InternalIpcSemaphoreCreate(IpcSemaphoreKey semKey, int numSems, bool retry_ok)
 {
 	int			semId;
 
@@ -105,16 +109,27 @@ InternalIpcSemaphoreCreate(IpcSemaphoreKey semKey, int numSems)
 		int			saved_errno = errno;
 
 		/*
-		 * Fail quietly if error indicates a collision with existing set. One
-		 * would expect EEXIST, given that we said IPC_EXCL, but perhaps we
-		 * could get a permission violation instead?  Also, EIDRM might occur
-		 * if an old set is slated for destruction but not gone yet.
+		 * Fail quietly if error suggests a collision with an existing set and
+		 * our caller has not lost patience.
+		 *
+		 * One would expect EEXIST, given that we said IPC_EXCL, but perhaps
+		 * we could get a permission violation instead.  On some platforms
+		 * EINVAL will be reported if the existing set has too few semaphores.
+		 * Also, EIDRM might occur if an old set is slated for destruction but
+		 * not gone yet.
+		 *
+		 * EINVAL is the key reason why we need the caller-level loop limit,
+		 * as it can also mean that the platform's SEMMSL is less than
+		 * numSems, and that condition can't be fixed by trying another key.
 		 */
-		if (saved_errno == EEXIST || saved_errno == EACCES
+		if (retry_ok &&
+			(saved_errno == EEXIST
+			 || saved_errno == EACCES
+			 || saved_errno == EINVAL
 #ifdef EIDRM
-			|| saved_errno == EIDRM
+			 || saved_errno == EIDRM
 #endif
-			)
+			 ))
 			return -1;
 
 		/*
@@ -211,17 +226,22 @@ IpcSemaphoreGetLastPID(IpcSemaphoreId semId, int semNum)
 static IpcSemaphoreId
 IpcSemaphoreCreate(int numSems)
 {
+	int			num_tries = 0;
 	IpcSemaphoreId semId;
 	union semun semun;
 	PGSemaphoreData mysema;
 
 	/* Loop till we find a free IPC key */
-	for (nextSemaKey++;; nextSemaKey++)
+	for (nextSemaKey++;; nextSemaKey++, num_tries++)
 	{
 		pid_t		creatorPID;
 
-		/* Try to create new semaphore set */
-		semId = InternalIpcSemaphoreCreate(nextSemaKey, numSems + 1);
+		/*
+		 * Try to create new semaphore set.  Give up after trying 1000
+		 * distinct IPC keys.
+		 */
+		semId = InternalIpcSemaphoreCreate(nextSemaKey, numSems + 1,
+										   num_tries < 1000);
 		if (semId >= 0)
 			break;				/* successful create */
 
@@ -258,7 +278,7 @@ IpcSemaphoreCreate(int numSems)
 		/*
 		 * Now try again to create the sema set.
 		 */
-		semId = InternalIpcSemaphoreCreate(nextSemaKey, numSems + 1);
+		semId = InternalIpcSemaphoreCreate(nextSemaKey, numSems + 1, true);
 		if (semId >= 0)
 			break;				/* successful create */
 
