Fix race condition at backend exit when deleting element in syncrep queue

michaelpq · michaelpq · commit f88f7206edbf · 2019-11-01T22:39:09.000+09:00
When a backend exits, it gets deleted from the syncrep queue if present. The queue was checked without SyncRepLock taken in exclusive mode, so it would have been possible for a backend to remove itself after a WAL sender already did the job. Fix this issue based on a suggestion from Fujii Masao, by first checking the queue without the lock. Then, if the backend is present in the queue, take the lock and perform an additional lookup check before doing the element deletion. Author: Dongming Liu Reviewed-by: Kyotaro Horiguchi, Fujii Masao, Michael Paquier Discussion: https://postgr.es/m/a0806273-8bbb-43b3-bbe1-c45a58f6ae21.lingce.ldm@alibaba-inc.com Backpatch-through: 9.4
diff --git a/src/backend/replication/syncrep.c b/src/backend/replication/syncrep.c
@@ -319,10 +319,18 @@ SyncRepCancelWait(void)
 void
 SyncRepCleanupAtProcExit(void)
 {
+	/*
+	 * First check if we are removed from the queue without the lock to not
+	 * slow down backend exit.
+	 */
 	if (!SHMQueueIsDetached(&(MyProc->syncRepLinks)))
 	{
 		LWLockAcquire(SyncRepLock, LW_EXCLUSIVE);
-		SHMQueueDelete(&(MyProc->syncRepLinks));
+
+		/* maybe we have just been removed, so recheck */
+		if (!SHMQueueIsDetached(&(MyProc->syncRepLinks)))
+			SHMQueueDelete(&(MyProc->syncRepLinks));
+
 		LWLockRelease(SyncRepLock);
 	}
 }

Original file line number	Diff line number	Diff line change
`@@ -319,10 +319,18 @@ SyncRepCancelWait(void)`
`319`	`319`	`void`
`320`	`320`	`SyncRepCleanupAtProcExit(void)`
`321`	`321`	`{`
	`322`	`+ /*`
	`323`	`+ * First check if we are removed from the queue without the lock to not`
	`324`	`+ * slow down backend exit.`
	`325`	`+ */`
`322`	`326`	`if (!SHMQueueIsDetached(&(MyProc->syncRepLinks)))`
`323`	`327`	`{`
`324`	`328`	`LWLockAcquire(SyncRepLock, LW_EXCLUSIVE);`
`325`		`- SHMQueueDelete(&(MyProc->syncRepLinks));`
	`329`	`+`
	`330`	`+ /* maybe we have just been removed, so recheck */`
	`331`	`+ if (!SHMQueueIsDetached(&(MyProc->syncRepLinks)))`
	`332`	`+ SHMQueueDelete(&(MyProc->syncRepLinks));`
	`333`	`+`
`326`	`334`	`LWLockRelease(SyncRepLock);`
`327`	`335`	`}`
`328`	`336`	`}`