Re: [Libevent-users] Deadlock when calling bufferevent_free from an other thread

[Nick Mathewson <nickm@xxxxxxxxxxxxx>]

On Wed, Aug 8, 2012 at 6:44 PM, Adrian Chadd <adrian@xxxxxxxxxxxxxxx> wrote:
> On 8 August 2012 15:30, Nick Mathewson <nickm@xxxxxxxxxxxxx> wrote:
>
>> Specifically, let me follow up with what I need: I need people with
>> good programming taste to read and understand that message, and let me
>> know whether you've got any good ideas for fixing it, or whether you
>> think my ideas are reasonable.  Any of my ideas would require some
>> tricky changes to the guarantees that event_del() provides for
>> multithreaded applications; I'm not sure there's one that doesn't.
>>
>> I am not going to be able to think as well as I would like about this
>> bug this without some help... and if I am doing this on my own, I will
>> deliver a solution that will not be as good as it could have been. So:
>> anybody with a good taste in multithreading and API design able to
>> comment?
>
> Personally, I'm of the opinion that it shouldn't be libevent that does
> this - that the application should be responsible for ensuring that
> event create/delete/modify occurs in the same thread as the base.
> Unfortunately that's not the kind of network application environment
> people want .. and this is why locking gets hard.
>
> So what's the lock? You have a lock being held in thread A, and the
> same lock w/ conditional in thread B, and there's your deadlock? I'm
> just staring at this code now to see what you've done.

No, it's stupider.  It's a condition variable meant to some kind of
out-of-thread changes to an event while the event's callback is
active.  It could be a lock instead.  (Though it would still be
susceptible to the same deadlock issues; see that mail I sent again.)

> All of that "EVBASE_IN_THREAD()" checks for doing locking and waiting
> is unfortunately very hairy. Is there any reason you haven't just used
> a dead list? (ie, mark the event as dead, put event on the dead list,
> have it reaped by the owning thread event loop.)

The problem is that events can be stack-allocated, or allocated inside
other structures.  In that case, in single-threaded code, after you
call event_del() on an event, you know that it is safe to exit the
function or free the structure in which the event is allocated.  The
idea here was to try to provide a similar guarantee for multithreaded
code, so that once you call event_del() on something, it is safe to
free() anything that contains it.

But the problem is that it's quite common to run into deadlock
situations where the code that's trying to do the event_del() holds a
lock, and the callback that's about to run wants to grab the lock.
The code's current "solution" involves making every _del() of an event
whose callback is running block until that callback is finished.  I'm
hoping for a better solution.


yrs,
-- 
Nick
***********************************************************************
To unsubscribe, send an e-mail to majordomo@xxxxxxxxxxxxx with
unsubscribe libevent-users    in the body.