]> granicus.if.org Git - postgresql/commit
Guard against spurious signals in LockBufferForCleanup.
authorAndres Freund <andres@anarazel.de>
Mon, 23 Feb 2015 15:11:11 +0000 (16:11 +0100)
committerAndres Freund <andres@anarazel.de>
Mon, 23 Feb 2015 15:14:14 +0000 (16:14 +0100)
commitbc208a5a2f9eb34bf795374fa5381e0c82073255
tree86851d46b4746e9a78f3ca3eb3b11d6593132f6f
parent5d2b45e3f78a85639f30431181c06d4c3221c5a1
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
src/backend/storage/buffer/bufmgr.c