Re: LWLock deadlock and gdb advice

On 07/30/2015 09:14 PM, Andres Freund wrote:
> On 2015-07-30 17:36:52 +0300, Heikki Linnakangas wrote:
>> In 9.4, LWLockAcquire holds the spinlock when it marks the lock as held,
>> until it has updated the variable. And LWLockWaitForVar() holds the spinlock
>> when it checks that the lock is held and that the variable's value matches.
>> So it cannot happen on 9.4.
>
> The first paragraph talked about "the same value", but that was just
> referring to it not yet having been cleared i think...
>
>> To reiterate, with 9.5, it's possible that a backend is sleeping in
>> LWLockWaitForVar(oldvar=123), even though the lock is currently held by
>> another backend with value 124. That seems wrong, or surprising at the very
>> least.
>
> With my patch that can't really happen that way though? The value is
> re-checked after queuing. If it has changed by then we're done. And if
> it hasn't yet changed we're guaranteed to be woken up once it's being
> changed?

Ok, let me try explaining it again. With your patch, 
LWLockAcquireWithVar looks like this (simplified, assuming the lock is 
free):

1. Set LW_VAL_EXCLUSIVE
3. (Acquire spinlock) Set *valptr = val (Release spinlock)

LWLockWaitForVar looks like this:

1. Check if lock is free
2. (Acquire Spinlock) Read *valptr, compare with oldval (Release spinlock)
3. LWLockQueueSelf
4. Re-check that *valptr is still equal to oldval
5. sleep.

This is the race condition:

Backend A            Backend B
---------            ---------
LWLockAcquireWithVar(123)  Set LW_VAL_EXCLUSIVE  Set *valptr = 123            LWLockWaitForVar(123)              See
thatthe lock is held              Read *valptr, it matches              LWLockQueueSelf
 
LWLockRelease()
LWLockAcquireWithVar(124)  Set LW_VAL_EXCLUSIVE              wakes up              See that the lock is still/again
held             Read *valptr, it's still 123              LWLockQueueSelf              Re-check that *valptr is still
123.             go to sleep.  Set *valptr = 124
 


Now, Backend B's LWLockWaitForVar(123) call is sleeping, even though the 
lock was free'd and reacquired with different value, 124. It won't wake 
up until A updates the value or releases the lock again.

This was not possible in 9.4. It was possible in 9.4 too when the value 
was same in both LWLockAcquireWithVar() calls, and I think that's 
acceptable, but not when the values differ.

> I generaly don't mind adding some sort of flag clearing or such, but I'd
> rather not have it in the retry loop in the general LWLockAttemptLock
> path - I found that very small amounts of instructions in there have a
> measurable impact.

I doubt clearing a bit would matter there, although you might have a 
better instinct on that...

- Heikki