Re: A question regarding LWLock in ProcSleep - Mailing list pgsql-hackers

On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao <kyao@pivotal.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