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

Commit 57c5ad1

Browse files
committed
Fix possibility of self-deadlock in ResolveRecoveryConflictWithBufferPin().
The tests added in 9f8a050 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-
1 parent 90abe1e commit 57c5ad1

File tree

1 file changed

+12
-10
lines changed

1 file changed

+12
-10
lines changed

src/backend/storage/ipc/standby.c

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ static HTAB *RecoveryLockLists;
4444

4545
/* Flags set by timeout handlers */
4646
static volatile sig_atomic_t got_standby_deadlock_timeout = false;
47+
static volatile sig_atomic_t got_standby_delay_timeout = false;
4748
static volatile sig_atomic_t got_standby_lock_timeout = false;
4849

4950
static void ResolveRecoveryConflictWithVirtualXIDs(VirtualTransactionId *waitlist,
@@ -603,10 +604,15 @@ ResolveRecoveryConflictWithBufferPin(void)
603604
enable_timeouts(timeouts, cnt);
604605
}
605606

606-
/* Wait to be signaled by UnpinBuffer() */
607+
/*
608+
* Wait to be signaled by UnpinBuffer() or for the wait to be interrupted
609+
* by one of the timeouts established above.
610+
*/
607611
ProcWaitForSignal(PG_WAIT_BUFFER_PIN);
608612

609-
if (got_standby_deadlock_timeout)
613+
if (got_standby_delay_timeout)
614+
SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
615+
else if (got_standby_deadlock_timeout)
610616
{
611617
/*
612618
* Send out a request for hot-standby backends to check themselves for
@@ -632,6 +638,7 @@ ResolveRecoveryConflictWithBufferPin(void)
632638
* individually, but that'd be slower.
633639
*/
634640
disable_all_timeouts(false);
641+
got_standby_delay_timeout = false;
635642
got_standby_deadlock_timeout = false;
636643
}
637644

@@ -691,8 +698,8 @@ CheckRecoveryConflictDeadlock(void)
691698
*/
692699

693700
/*
694-
* StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT
695-
* occurs before STANDBY_TIMEOUT.
701+
* StandbyDeadLockHandler() will be called if STANDBY_DEADLOCK_TIMEOUT is
702+
* exceeded.
696703
*/
697704
void
698705
StandbyDeadLockHandler(void)
@@ -702,16 +709,11 @@ StandbyDeadLockHandler(void)
702709

703710
/*
704711
* StandbyTimeoutHandler() will be called if STANDBY_TIMEOUT is exceeded.
705-
* Send out a request to release conflicting buffer pins unconditionally,
706-
* so we can press ahead with applying changes in recovery.
707712
*/
708713
void
709714
StandbyTimeoutHandler(void)
710715
{
711-
/* forget any pending STANDBY_DEADLOCK_TIMEOUT request */
712-
disable_timeout(STANDBY_DEADLOCK_TIMEOUT, false);
713-
714-
SendRecoveryConflictWithBufferPin(PROCSIG_RECOVERY_CONFLICT_BUFFERPIN);
716+
got_standby_delay_timeout = true;
715717
}
716718

717719
/*

0 commit comments

Comments
 (0)