[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [Libevent-users] RFC: strategy for deadlock avoidance and safe finalization in Libevent 2.1



The idea is good, but why to not just register finalizer callback on
event initialization? A data pointer is passed to event_assign() or event_new()
so we can pass finalizer along with the data.

With this approach,
- finalizer, if set, will be called automatically after calling
  event_free() or event_del() and after event execution
- The event initialization code includes reference to event deinitialization one,
  which makes the code easier to understand
- event_assign(), event_new() and event_get_assignment() should be extended
  (for compatibility, new functions should be added)
- event_get_finalizer() should also be added
- event_del_noblock() & event_free_noblock() or event_del_ext() & event_free_ext()
  with extra flags is still needed. One extra flag DONT_FINALIZE is desirable to
  allow user to call finalizer manually.

10.04.2013 02:50, Nick Mathewson wrote:
A solution that can work: finalizer callbacks

    The solution seems to be to delay the final cleanup so that it
    can happen once the event is definitely no longer running.  To do
    this, we can use Yet Another Kind of Callback.

       typedef void (*event_finalize_callback_fn)(
               struct event *ev,
               void *arg);

       event_free_finalize(
               unsigned int flags,
               struct event *ev,
               event_finalize_callback_fn cb);

    With this interface, we can tell Libevent to free the event, and
    to run a given callback on that event and its user-configured
    data before the event gets freed.

    We can provide a similar interface for events allocated on the
    stack or as parts of larger structures:

       event_finalize(
               unsigned int flags,
               struct event *ev,
               event_finalize_callback_fn cb);

    Once you've called event_finalize() or event_free_finalize() on
    an event, any attempt to event_add() or event_del() or
    event_active() that event will fail.  Once the provided
    finalization callback has returned, subsequent.

    To avoid the kind of insanity that made Java finalizers a piece
    of gibbering madness, the finalizer callback is not allowed to
    try to un-finalize the event or prevent it from getting freed.

Retaining backward compatibility:

    We're not quite done, though.  Remember that our goal here was to
    make it viable for event_del() to not block while a callback is
    running.  Because the current event_del() blocks, and some
    existing code relies on it blocking, we can't just make
    event_del() nonblocking by default.

    Instead, we provide a flag for marking events as "safe to delete
    without blocking."  To make event_del() not block when given an
    event 'ev', just set up that event with the EV_FINALIZE flag when
    constructing it.

    (The name of this option is in flux.  Do we have a better name
    for it?)

    (I'm considering whether there shouldn't be an option to make
    EV_FINALIZE on by default, plus a flag to turn it off for
    particular events.)

    Additionally, there are now two variants of event_del(): one that
    always blocks until the event's callback is no longer running,
    and one that never blocks while the event's callback is running:

       event_del_noblock(ev);
       event_del_block(ev);

What about bufferevents, evbuffers, evconnlisteners, etc?

    They need the same treatment, I think.  Otherwise, the choice is
    force bufferevent_free() to block while bufferevent callbacks are
    running (current behavior, problematic), or to make it so that

      bufferevent_free(bev);
      free(bev_extra_data);

    is no longer allowed.

    Adding a bufferevent_free_finalize() function is one option here.

    Adding a new option when constructing a bufferevent, plus a new
    special BEV_EVENT_* flag that gets used for finalization is
    another option.

    For evbuffers and evconnlisteners, a *_free_finalize() option
    seems like the best choice.

What about that 'flags' argument to event_finalize()?

    When I first designed this, I thought there might want to be a
    "TRY_IMMEDIATE" flag to optionally tell the finalizer that, if it
    _could_ run immediately, it should.

    I'm not yet sure whether that's a good idea.

Another compatibility note:

    Remember, single-threaded programs shouldn't actually need to
    change at all here, and multi-threaded programs that work today
    will keep working.  So there's no need to freak out about that.

Status:

    I've got a sketchy incomplete under-tested implementation in the
    branch "21_deadlock_fix" of my personal libevent repo at
    https://github.com/nmathewson/Libevent/ .  It implements the core
    features, but needs more polish, documentation, and testing.

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

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