Re: A question regarding LWLock in ProcSleep

Hi Amit,

Thanks for the reply!

Yes, in LockAcquireExtended, after exiting WaitOnLock, there would be a
read access to proclock, however, since the lock has either been granted or
rejected by deadlock check, I can think of no possible concurrent write
access to the proclock, so is it really necessary to acquire the LWLock?

Thanks,
Kenan

On Fri, Dec 18, 2015 at 10:25 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
wrote:

> 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
>