Re: LWLock deadlock and gdb advice

From: Heikki Linnakangas <hlinnaka(at)iki(dot)fi>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Jeff Janes <jeff(dot)janes(at)gmail(dot)com>, Peter Geoghegan <pg(at)heroku(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: LWLock deadlock and gdb advice
Date: 2015-06-30 19:19:02
Message-ID: 5592EBA6.2050503@iki.fi
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2015-06-30 20:24:43 Re: LWLock deadlock and gdb advice
Previous Message Andres Freund 2015-06-30 19:09:24 Re: LWLock deadlock and gdb advice