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.

diff --git a/src/backend/storage/lmgr/deadlock.c b/src/backend/storage/lmgr/deadlock.c
index 8334a8876180e1d4a1da4773833e4284aef10198..0a8dd5eb7c22416882d1ea16691271c0b1df8ff0 100644 (file)
--- 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 c7a001b3b795d1ca33c894d80be98c640f180f0e..8560a903bc8cb6e988bad38e6bf2e617ff191b49 100644 (file)
--- 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;
 }
 
 /*