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

[Zack Weinberg <zackw@xxxxxxxxx>]

On 2012-08-08 2:52 PM, Nick Mathewson wrote:
> On Mon, Aug 6, 2012 at 2:42 PM, Matthieu Nottale wrote:
> > I'm experiencing a deadlock on 2.0.19 while calling bufferevent_free frome
> > thread A, while thread B is in event_base_dispatch.
>
> Ouch.  This is a known bug.  This fix is going to be hard.  I wrote
> about it here:
> http://archives.seul.org/libevent/users/Feb-2012/msg00053.html

I've been thinking about this a little.  As I mentioned quite some time 
ago, I've seen what seems to be the same root problem (but of course 
with a different manifestation) in a single-threaded application: 
http://archives.seul.org/libevent/users/Jun-2012/msg00043.html  Because 
of that, I think this is fundamentally an ownership issue rather than a 
threading issue.  The problem is that the application is responsible for 
deallocating events, bufferevents, etc. and their respective context 
data, and the APIs for doing so are synchronous, but it cannot in 
general know whether libevent holds live references to any of these 
items.  I don't think this can be fixed without application changes, but 
I think I have a solution that involves only a pretty narrow change to 
the API.  (I'd like to make more drastic changes but those are not 2.1 
material.)

To fix things, we need to change abstract behavior in two places. 
First, when you event_del() an event while the event loop to which it 
belongs is running, it does not actually get disassociated from the 
event loop until the next time the loop is about to either block or exit 
(this is called the "stabilization point").  It is not safe to 
deallocate the event object before then.  Second, we consider each event 
object to _own_ its context data (if any), and therefore to be 
responsible for deallocating it at the appropriate time, via an 
application-provided callback (henceforth the "deallocation hook").

Concretely, when you call event_free, it _may_ synchronously deallocate 
the event and call your deallocation hook to free the context data, but 
if the event loop is running, it will just event_del the event and 
return, and the actual deallocation will be queued to occur when the 
event loop reaches the stabilization point.  Similarly for 
bufferevent_free and all other libevent objects that are wrapped around 
an event.

For events allocated with event_new, if the application provides 
nontrivial context data but does _not_ set a deallocation hook, we have 
to allow the application to deallocate its context itself, when it 
currently does (near-contemporaneous with event_free).  I think the way 
to handle that is, if the deallocation hook isn't set, we clear the 
context pointer when event_free is called.  This means that the 
application may subsequently see an unexpected event callback with 
context NULL, or be unable to retrieve its context point from the event 
object, but that can only happen in circumstances where we would 
previously have had a use-after-free condition, so we've converted a 
dangerous bug into a less dangerous one.

For events allocated with event_assign, if you set a deallocation hook, 
then it becomes safe to call event_free on that event.  At the next 
stabilization point, instead of the above behavior, the deallocation 
hook will be called with the *event* as its argument; it is responsible 
for deallocating everything at that time (event_get_callback_arg() is 
guaranteed still to work).

Ideally you'd set the deallocation hook in event_new(), but we can't 
easily add arguments to that function, so as the short-term narrow API 
change I propose

typedef void (*event_dealloc_cb)(void *);

void event_set_dealloc(struct event *, event_dealloc_cb);
void bufferevent_set_dealloc(struct bufferevent *, event_dealloc_cb);
/* and so on for anything else that needs it */

What do you think?  Most importantly, does this solve the problem? 
Could it be polished?

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