mirrors/postgres
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().

Browse Source 
The tests added in 9f8a050f68d failed nearly reliably on FreeBSD in CI, and
occasionally on the buildfarm. That turns out to be caused not by a bug in the
test, but by a longstanding bug in recovery conflict handling.

The standby timeout handler, used by ResolveRecoveryConflictWithBufferPin(),
executed SendRecoveryConflictWithBufferPin() inside a signal handler. A bad
idea, because the deadlock timeout handler (or a spurious latch set) could
have interrupted ProcWaitForSignal(). If unlucky that could cause a
self-deadlock on ProcArrayLock, if the deadlock check is in
SendRecoveryConflictWithBufferPin()->CancelDBBackends().

To fix, set a flag in StandbyTimeoutHandler(), and check the flag in
ResolveRecoveryConflictWithBufferPin().

Subsequently the recovery conflict tests will be backpatched.

Discussion: https://postgr.es/m/20220413002626.udl7lll7f3o7nre7@alap3.anarazel.de
Backpatch: 10-
This commit is contained in:
Andres Freund 2022-05-02 18:25:00 -07:00
parent 21e184403b
commit 8f1537d10e
1 changed files with 10 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
src/backend/storage/ipc/standby.c
View File

@ -46,6 +46,7 @@ static HTAB *RecoveryLockLists;
/* Flags set by timeout handlers */ /* Flags set by timeout handlers */
static volatile sig_atomic_t got_standby_deadlock_timeout = false; static volatile sig_atomic_t got_standby_deadlock_timeout = false;
static volatile sig_atomic_t got_standby_delay_timeout = false;
static volatile sig_atomic_t got_standby_lock_timeout = false; static volatile sig_atomic_t got_standby_lock_timeout = false;
static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist, static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
@ -793,7 +794,8 @@ ResolveRecoveryConflictWithBufferPin(void)
} }
/* /*
* Wait to be signaled by UnpinBuffer(). * Wait to be signaled by UnpinBuffer() or for the wait to be interrupted
* by one of the timeouts established above.
* *
* We assume that only UnpinBuffer() and the timeout requests established * We assume that only UnpinBuffer() and the timeout requests established
* above can wake us up here. WakeupRecovery() called by walreceiver or * above can wake us up here. WakeupRecovery() called by walreceiver or
@ -802,7 +804,9 @@ ResolveRecoveryConflictWithBufferPin(void)
*/ */
ProcWaitForSignal(PG_WAIT_BUFFER_PIN); ProcWaitForSignal(PG_WAIT_BUFFER_PIN);
if (got_standby_deadlock_timeout) if (got_standby_delay_timeout)
SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
else if (got_standby_deadlock_timeout)
{ {
/* /*
* Send out a request for hot-standby backends to check themselves for * Send out a request for hot-standby backends to check themselves for
@ -828,6 +832,7 @@ ResolveRecoveryConflictWithBufferPin(void)
* individually, but that'd be slower. * individually, but that'd be slower.
*/ */
disable_all_timeouts(false); disable_all_timeouts(false);
got_standby_delay_timeout = false;
got_standby_deadlock_timeout = false; got_standby_deadlock_timeout = false;
} }
@ -887,8 +892,8 @@ CheckRecoveryConflictDeadlock(void)
*/ */
/* /*
* StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT * StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT is
* occurs before STANDBY_TIMEOUT. * exceeded.
*/ */
void void
StandbyDeadLockHandler(void) StandbyDeadLockHandler(void)
@ -898,16 +903,11 @@ StandbyDeadLockHandler(void)
/* /*
* StandbyTimeoutHandler() will be called if STANDBY_TIMEOUT is exceeded. * StandbyTimeoutHandler() will be called if STANDBY_TIMEOUT is exceeded.
* Send out a request to release conflicting buffer pins unconditionally,
* so we can press ahead with applying changes in recovery.
*/ */
void void
StandbyTimeoutHandler(void) StandbyTimeoutHandler(void)
{ {
/* forget any pending STANDBY_DEADLOCK_TIMEOUT request */ got_standby_delay_timeout = true;
disable_timeout(STANDBY_DEADLOCK_TIMEOUT, false);
SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
} }
/* /*