Re: Error in backend/storage/lmgr/proc.c: ProcSleep()

Tomasz Zielonka <tomek(at)mult(dot)i(dot)pl> writes:
> Platform: PostgreSQL 7.1.3, Linux 2.4.8, egcs 2.91.66
> PostgreSQL forgets to release lock until shutdown in this scenario:

Good catch! This has been broken since 7.1 ... surprising that no one
discovered the problem sooner.

I think that rather than removing the early-deadlock-detection code as
you suggest, it's better to make it work correctly. I have applied a
patch that allows RemoveFromWaitQueue() to be used, so that the recovery
path is the same as if HandleDeadLock had been invoked.

regards, tom lane

*** /home/postgres/pgsql/src/backend/storage/lmgr/proc.c.orig Fri Jul 6 17:04:26 2001
--- /home/postgres/pgsql/src/backend/storage/lmgr/proc.c Mon Sep 3 22:26:57 2001
***************
*** 506,521 ****
SPINLOCK spinlock = lockctl->masterLock;
PROC_QUEUE *waitQueue = &(lock->waitProcs);
int myHeldLocks = MyProc->heldLocks;
PROC *proc;
int i;
-
#ifndef __BEOS__
struct itimerval timeval,
dummy;
-
#else
bigtime_t time_interval;
-
#endif

/*
--- 506,519 ----
SPINLOCK spinlock = lockctl->masterLock;
PROC_QUEUE *waitQueue = &(lock->waitProcs);
int myHeldLocks = MyProc->heldLocks;
+ bool early_deadlock = false;
PROC *proc;
int i;
#ifndef __BEOS__
struct itimerval timeval,
dummy;
#else
bigtime_t time_interval;
#endif

/*
***************
*** 535,541 ****
* immediately. This is the same as the test for immediate grant in
* LockAcquire, except we are only considering the part of the wait
* queue before my insertion point.
- *
*/
if (myHeldLocks != 0)
{
--- 533,538 ----
***************
*** 550,558 ****
/* Must I wait for him ? */
if (lockctl->conflictTab[lockmode] & proc->heldLocks)
{
! /* Yes, can report deadlock failure immediately */
! MyProc->errType = STATUS_ERROR;
! return STATUS_ERROR;
}
/* I must go before this waiter. Check special case. */
if ((lockctl->conflictTab[lockmode] & aheadRequests) == 0 &&
--- 547,560 ----
/* Must I wait for him ? */
if (lockctl->conflictTab[lockmode] & proc->heldLocks)
{
! /*
! * Yes, so we have a deadlock. Easiest way to clean up
! * correctly is to call RemoveFromWaitQueue(), but we
! * can't do that until we are *on* the wait queue.
! * So, set a flag to check below, and break out of loop.
! */
! early_deadlock = true;
! break;
}
/* I must go before this waiter. Check special case. */
if ((lockctl->conflictTab[lockmode] & aheadRequests) == 0 &&
***************
*** 600,606 ****
MyProc->waitHolder = holder;
MyProc->waitLockMode = lockmode;

! MyProc->errType = STATUS_OK;/* initialize result for success */

/* mark that we are waiting for a lock */
waitingForLock = true;
--- 602,620 ----
MyProc->waitHolder = holder;
MyProc->waitLockMode = lockmode;

! MyProc->errType = STATUS_OK; /* initialize result for success */
!
! /*
! * If we detected deadlock, give up without waiting. This must agree
! * with HandleDeadLock's recovery code, except that we shouldn't release
! * the semaphore since we haven't tried to lock it yet.
! */
! if (early_deadlock)
! {
! RemoveFromWaitQueue(MyProc);
! MyProc->errType = STATUS_ERROR;
! return STATUS_ERROR;
! }

/* mark that we are waiting for a lock */
waitingForLock = true;