Re: LWLock deadlock and gdb advice

On 06/30/2015 10:09 PM, Andres Freund wrote:
> On 2015-06-30 21:08:53 +0300, Heikki Linnakangas wrote:
>>>             /*
>>>              * XXX: We can significantly optimize this on platforms with 64bit
>>>              * atomics.
>>>              */
>>>             value = *valptr;
>>>             if (value != oldval)
>>>             {
>>>                 result = false;
>>>                 mustwait = false;
>>>                 *newval = value;
>>>             }
>>>             else
>>>                 mustwait = true;
>>>             SpinLockRelease(&lock->mutex);
>>>         }
>>>         else
>>>             mustwait = false;
>>>
>>>         if (!mustwait)
>>>             break;                /* the lock was free or value didn't match */
>>>
>>>         /*
>>>          * Add myself to wait queue. Note that this is racy, somebody else
>>>          * could wakeup before we're finished queuing. NB: We're using nearly
>>>          * the same twice-in-a-row lock acquisition protocol as
>>>          * LWLockAcquire(). Check its comments for details.
>>>          */
>>>         LWLockQueueSelf(lock, LW_WAIT_UNTIL_FREE, false);
>>
>> After the spinlock is released above, but before the LWLockQueueSelf() call,
>> it's possible that another backend comes in, acquires the lock, changes the
>> variable's value, and releases the lock again. In 9.4, the spinlock was not
>> released until the process was queued.
>
> Hm. Right. A recheck of the value after the queuing should be sufficient
> to fix? That's how we deal with the exact same scenarios for the normal
> lock state, so that shouldn't be very hard to add.

Yeah. It's probably more efficient to not release the spinlock between 
checking the value and LWLockQueueSelf() though.

>> Looking at LWLockAcquireWithVar(), it's also no longer holding the spinlock
>> while it updates the Var. That's not cool either :-(.
>
> Hm. I'd somehow assumed holding the lwlock ought to be sufficient, but
> it really isn't. This var stuff isn't fitting all that well into the
> lwlock code.

I'll take a stab at fixing this tomorrow. I take the blame for not 
documenting the semantics of LWLockAcquireWithVar() and friends well 
enough...

- Heikki