From 73d60ac385a93684f68297ae0ccb8f75bc6f23e1 Mon Sep 17 00:00:00 2001 From: Heikki Linnakangas Date: Mon, 9 Feb 2026 20:26:23 +0200 Subject: [PATCH] cleanup: Deadlock checker is no longer called from signal handler Clean up a few leftovers from when the deadlock checker was called from signal handler. We stopped doing that in commit 6753333f55, in year 2015. - CheckDeadLock can return a return value directly to the caller, there's no need to use a global variable for that. - Remove outdated comments that claimed that CheckDeadLock "signals ProcSleep". - It should be OK to ereport() from DeadLockCheck now. I considered getting rid of InitDeadLockChecking() and moving the workspace allocations into DeadLockCheck, but it's still good to avoid doing the allocations while we're holding all the partition locks. So just update the comment to give that as the reason we do the allocations up front. --- src/backend/storage/lmgr/deadlock.c | 10 ++++------ src/backend/storage/lmgr/proc.c | 28 ++++++++++++++++------------ 2 files changed, 20 insertions(+), 18 deletions(-) diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c index 8334a887618..0a8dd5eb7c2 100644 --- a/src/backend/storage/lmgr/deadlock.c +++ b/src/backend/storage/lmgr/deadlock.c @@ -135,10 +135,9 @@ static PGPROC *blocking_autovacuum_proc = NULL; * This does per-backend initialization of the deadlock checker; primarily, * allocation of working memory for DeadLockCheck. We do this per-backend * since there's no percentage in making the kernel do copy-on-write - * inheritance of workspace from the postmaster. We want to allocate the - * space at startup because (a) the deadlock checker might be invoked when - * there's no free memory left, and (b) the checker is normally run inside a - * signal handler, which is a very dangerous place to invoke palloc from. + * inheritance of workspace from the postmaster. We allocate the space at + * startup because the deadlock checker is run with all the partitions of the + * lock table locked, and we want to keep that section as short as possible. */ void InitDeadLockChecking(void) @@ -213,8 +212,7 @@ InitDeadLockChecking(void) * * On failure, deadlock details are recorded in deadlockDetails[] for * subsequent printing by DeadLockReport(). That activity is separate - * because (a) we don't want to do it while holding all those LWLocks, - * and (b) we are typically invoked inside a signal handler. + * because we don't want to do it while holding all those LWLocks. */ DeadLockState DeadLockCheck(PGPROC *proc) diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index c7a001b3b79..8560a903bc8 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -80,15 +80,13 @@ PROC_HDR *ProcGlobal = NULL; NON_EXEC_STATIC PGPROC *AuxiliaryProcs = NULL; PGPROC *PreparedXactProcs = NULL; -static DeadLockState deadlock_state = DS_NOT_YET_CHECKED; - /* Is a deadlock check pending? */ static volatile sig_atomic_t got_deadlock_timeout; static void RemoveProcFromArray(int code, Datum arg); static void ProcKill(int code, Datum arg); static void AuxiliaryProcKill(int code, Datum arg); -static void CheckDeadLock(void); +static DeadLockState CheckDeadLock(void); /* @@ -1321,6 +1319,7 @@ ProcSleep(LOCALLOCK *locallock) bool allow_autovacuum_cancel = true; bool logged_recovery_conflict = false; ProcWaitStatus myWaitStatus; + DeadLockState deadlock_state; /* The caller must've armed the on-error cleanup mechanism */ Assert(GetAwaitedLock() == locallock); @@ -1461,7 +1460,7 @@ ProcSleep(LOCALLOCK *locallock) /* check for deadlocks first, as that's probably log-worthy */ if (got_deadlock_timeout) { - CheckDeadLock(); + deadlock_state = CheckDeadLock(); got_deadlock_timeout = false; } CHECK_FOR_INTERRUPTS(); @@ -1784,14 +1783,14 @@ ProcLockWakeup(LockMethod lockMethodTable, LOCK *lock) * * We only get to this routine, if DEADLOCK_TIMEOUT fired while waiting for a * lock to be released by some other process. Check if there's a deadlock; if - * not, just return. (But signal ProcSleep to log a message, if - * log_lock_waits is true.) If we have a real deadlock, remove ourselves from - * the lock's wait queue and signal an error to ProcSleep. + * not, just return. If we have a real deadlock, remove ourselves from the + * lock's wait queue. */ -static void +static DeadLockState CheckDeadLock(void) { int i; + DeadLockState result; /* * Acquire exclusive lock on the entire shared lock data structures. Must @@ -1818,17 +1817,20 @@ CheckDeadLock(void) */ if (MyProc->links.prev == NULL || MyProc->links.next == NULL) + { + result = DS_NO_DEADLOCK; goto check_done; + } #ifdef LOCK_DEBUG if (Debug_deadlocks) DumpAllLocks(); #endif - /* Run the deadlock check, and set deadlock_state for use by ProcSleep */ - deadlock_state = DeadLockCheck(MyProc); + /* Run the deadlock check */ + result = DeadLockCheck(MyProc); - if (deadlock_state == DS_HARD_DEADLOCK) + if (result == DS_HARD_DEADLOCK) { /* * Oops. We have a deadlock. @@ -1840,7 +1842,7 @@ CheckDeadLock(void) * * RemoveFromWaitQueue sets MyProc->waitStatus to * PROC_WAIT_STATUS_ERROR, so ProcSleep will report an error after we - * return from the signal handler. + * return. */ Assert(MyProc->waitLock != NULL); RemoveFromWaitQueue(MyProc, LockTagHashCode(&(MyProc->waitLock->tag))); @@ -1867,6 +1869,8 @@ CheckDeadLock(void) check_done: for (i = NUM_LOCK_PARTITIONS; --i >= 0;) LWLockRelease(LockHashPartitionLockByIndex(i)); + + return result; } /* -- 2.47.3