Re: A question regarding LWLock in ProcSleep

On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao <kyao(at)pivotal(dot)io> wrote:

> Hi there,
>
> In function ProcSleep, after the process has been waken up, either with
> lock granted or deadlock detected, it would re-acquire the lock table's
> partition LWLock.
>
> The code episode is here:
>
> /*
> * Re-acquire the lock table's partition lock. We have to do this to hold
> * off cancel/die interrupts before we can mess with lockAwaited (else we
> * might have a missed or duplicated locallock update).
> */
> LWLockAcquire(partitionLock, LW_EXCLUSIVE);
>
> /*
> * We no longer want LockErrorCleanup to do anything.
> */
> lockAwaited = NULL;
>
> /*
> * If we got the lock, be sure to remember it in the locallock table.
> */
> if (MyProc->waitStatus == STATUS_OK)
> GrantAwaitedLock();
>
> /*
> * We don't have to do anything else, because the awaker did all the
> * necessary update of the lock table and MyProc.
> */
> return MyProc->waitStatus;
>
> ​
> Questions are:
>
> (1) The comment says that "we might have a missed or duplicated locallock
> update", in what cases would we hit this if without holding the LWLock?
>
> (2) The comment says "we have to do this to hold off cancel/die
> interrupts", then:
>
> - why using LWLockAcquire instead of HOLD_INTERRUPTS directly?
> - From the handler of SIGINT and SIGTERM, seems nothing serious would
> be processed here, since no CHECK_FOR_INTERRUPS is called before releasing
> this LWLock. Why we should hold off cancel/die interrupts here?
>
> (3) Before releasing this LWLock, the only share memory access is
> MyProc->waitStatus; since the process has been granted the lock or removed
> from the lock's waiting list because of deadlock, is it possible some other
> processes would access this field? if not, then why we need LWLock here?
> what does this lock protect?
>
>
I think the other thing which needs protection of LWLock is
access to proclock which is done in the caller
(LockAcquireExtended).

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com