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 majord...@freehaven.net with
unsubscribe libevent-users in the body.
***********************************************************************
To unsubscribe, send an e-mail to majord...@freehaven.net with
unsubscribe libevent-users in the body.
