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.kapil...@gmail.com> wrote: > On Thu, Dec 17, 2015 at 1:51 PM, Kenan Yao <k...@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 >
