Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 89629f2

Browse files
committed
Guard against spurious signals in LockBufferForCleanup.
When LockBufferForCleanup() has to wait for getting a cleanup lock on a buffer it does so by setting a flag in the buffer header and then wait for other backends to signal it using ProcWaitForSignal(). Unfortunately LockBufferForCleanup() missed that ProcWaitForSignal() can return for other reasons than the signal it is hoping for. If such a spurious signal arrives the wait flags on the buffer header will still be set. That then triggers "ERROR: multiple backends attempting to wait for pincount 1". The fix is simple, unset the flag if still set when retrying. That implies an additional spinlock acquisition/release, but that's unlikely to matter given the cost of waiting for a cleanup lock. Alternatively it'd have been possible to move responsibility for maintaining the relevant flag to the waiter all together, but that might have had negative consequences due to possible floods of signals. Besides being more invasive. This looks to be a very longstanding bug. The relevant code in LockBufferForCleanup() hasn't changed materially since its introduction and ProcWaitForSignal() was documented to return for unrelated reasons since 8.2. The master only patch series removing ImmediateInterruptOK made it much easier to hit though, as ProcSendSignal/ProcWaitForSignal now uses a latch shared with other tasks. Per discussion with Kevin Grittner, Tom Lane and me. Backpatch to all supported branches. Discussion: 11553.1423805224@sss.pgh.pa.us
1 parent 0214a61 commit 89629f2

File tree

1 file changed

+14
-0
lines changed

1 file changed

+14
-0
lines changed

src/backend/storage/buffer/bufmgr.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2902,6 +2902,20 @@ LockBufferForCleanup(Buffer buffer)
29022902
else
29032903
ProcWaitForSignal();
29042904

2905+
/*
2906+
* Remove flag marking us as waiter. Normally this will not be set
2907+
* anymore, but ProcWaitForSignal() can return for other signals as
2908+
* well. We take care to only reset the flag if we're the waiter, as
2909+
* theoretically another backend could have started waiting. That's
2910+
* impossible with the current usages due to table level locking, but
2911+
* better be safe.
2912+
*/
2913+
LockBufHdr(bufHdr);
2914+
if ((bufHdr->flags & BM_PIN_COUNT_WAITER) != 0 &&
2915+
bufHdr->wait_backend_pid == MyProcPid)
2916+
bufHdr->flags &= ~BM_PIN_COUNT_WAITER;
2917+
UnlockBufHdr(bufHdr);
2918+
29052919
PinCountWaitBuf = NULL;
29062920
/* Loop back and try again */
29072921
}

0 commit comments

Comments
 (0)