Restore fast-path event_reinit() for slower backends

We used to use the needs_reinit flag in struct eventop to indicate
whether an event backend had shared state across a fork(), and
therefore would require us to construct a new event backend.  But
when we realized that the signal notification fds and the thread
notification fds would always be shared across forks, we stopped
looking at it.

This patch restores the old behavior so that poll, select, and
win32select don't need to do a linear scan over all pending
fds/signals when they do a reinit.  Their life is hard enough
already.
This commit is contained in:
Nick Mathewson 2012-01-27 14:30:41 -05:00
parent 272033efe5
commit 2c4b5de16a
2 changed files with 59 additions and 46 deletions

96
event.c
View File

@ -860,30 +860,24 @@ event_reinit(struct event_base *base)
int res = 0; int res = 0;
struct event *ev; struct event *ev;
int was_notifiable = 0; int was_notifiable = 0;
int had_signal_added = 0;
EVBASE_ACQUIRE_LOCK(base, th_base_lock); EVBASE_ACQUIRE_LOCK(base, th_base_lock);
evsel = base->evsel; evsel = base->evsel;
#if 0 /* check if this event mechanism requires reinit on the backend */
/* Right now, reinit always takes effect, since even if the if (evsel->need_reinit) {
backend doesn't require it, the signal socketpair code does. /* We're going to call event_del() on our notify events (the
* ones that tell about signals and wakeup events). But we
XXX * don't actually want to tell the backend to change its
*/ * state, since it might still share some resource (a kqueue,
/* check if this event mechanism requires reinit */ * an epoll fd) with the parent process, and we don't want to
if (!evsel->need_reinit) * delete the fds from _that_ backend, we temporarily stub out
goto done; * the evsel with a replacement.
#endif */
base->evsel = &nil_eventop;
/* We're going to call event_del() on our notify events (the ones that }
* tell about signals and wakeup events). But we don't actually want
* to tell the backend to change its state, since it might still share
* some resource (a kqueue, an epoll fd) with the parent process, and
* we don't want to delete the fds from _that_ backend, we temporarily
* stub out the evsel with a replacement.
*/
base->evsel = &nil_eventop;
/* We need to re-create a new signal-notification fd and a new /* We need to re-create a new signal-notification fd and a new
* thread-notification fd. Otherwise, we'll still share those with * thread-notification fd. Otherwise, we'll still share those with
@ -899,6 +893,7 @@ event_reinit(struct event_base *base)
EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]); EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]);
if (base->sig.ev_signal_pair[1] != -1) if (base->sig.ev_signal_pair[1] != -1)
EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[1]); EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[1]);
had_signal_added = 1;
base->sig.ev_signal_added = 0; base->sig.ev_signal_added = 0;
} }
if (base->th_notify_fd[0] != -1) { if (base->th_notify_fd[0] != -1) {
@ -912,38 +907,47 @@ event_reinit(struct event_base *base)
event_debug_unassign(&base->th_notify); event_debug_unassign(&base->th_notify);
base->th_notify_fn = NULL; base->th_notify_fn = NULL;
} }
/* Replace the original evsel. */ /* Replace the original evsel. */
base->evsel = evsel; base->evsel = evsel;
/* Reconstruct the backend through brute-force, so that we do not if (evsel->need_reinit) {
* share any structures with the parent process. For some backends, /* Reconstruct the backend through brute-force, so that we do
* this is necessary: epoll and kqueue, for instance, have events * not share any structures with the parent process. For some
* associated with a kernel structure. If didn't reinitialize, we'd * backends, this is necessary: epoll and kqueue, for
* share that structure with the parent process, and any changes made * instance, have events associated with a kernel
* by the parent would affect our backend's behavior (and vice versa). * structure. If didn't reinitialize, we'd share that
*/ * structure with the parent process, and any changes made by
if (base->evsel->dealloc != NULL) * the parent would affect our backend's behavior (and vice
base->evsel->dealloc(base); * versa).
base->evbase = evsel->init(base); */
if (base->evbase == NULL) { if (base->evsel->dealloc != NULL)
event_errx(1, "%s: could not reinitialize event mechanism", base->evsel->dealloc(base);
__func__); base->evbase = evsel->init(base);
res = -1; if (base->evbase == NULL) {
goto done; event_errx(1,
"%s: could not reinitialize event mechanism",
__func__);
res = -1;
goto done;
}
/* Empty out the changelist (if any): we are starting from a
* blank slate. */
event_changelist_freemem(&base->changelist);
/* Tell the event maps to re-inform the backend about all
* pending events. This will make the signal notification
* event get re-created if necessary. */
if (evmap_io_reinit(base) < 0)
res = -1;
if (evmap_signal_reinit(base) < 0)
res = -1;
} else {
if (had_signal_added)
res = evsig_init(base);
} }
/* Empty out the changelist (if any): we are starting from a blank
* slate. */
event_changelist_freemem(&base->changelist);
/* Tell the event maps to re-inform the backend about all pending
* events. This will make the signal notification event get re-created
* if necessary. */
if (evmap_io_reinit(base) < 0)
res = -1;
if (evmap_signal_reinit(base) < 0)
res = -1;
/* If we were notifiable before, and nothing just exploded, become /* If we were notifiable before, and nothing just exploded, become
* notifiable again. */ * notifiable again. */
if (was_notifiable && res == 0) if (was_notifiable && res == 0)

View File

@ -188,6 +188,9 @@ evsig_init(struct event_base *base)
evutil_make_socket_closeonexec(base->sig.ev_signal_pair[0]); evutil_make_socket_closeonexec(base->sig.ev_signal_pair[0]);
evutil_make_socket_closeonexec(base->sig.ev_signal_pair[1]); evutil_make_socket_closeonexec(base->sig.ev_signal_pair[1]);
if (base->sig.sh_old) {
mm_free(base->sig.sh_old);
}
base->sig.sh_old = NULL; base->sig.sh_old = NULL;
base->sig.sh_old_max = 0; base->sig.sh_old_max = 0;
@ -330,6 +333,12 @@ _evsig_restore_handler(struct event_base *base, int evsignal)
ev_sighandler_t *sh; ev_sighandler_t *sh;
#endif #endif
if (evsignal >= sig->sh_old_max) {
/* Can't actually restore. */
/* XXXX.*/
return 0;
}
/* restore previous handler */ /* restore previous handler */
sh = sig->sh_old[evsignal]; sh = sig->sh_old[evsignal];
sig->sh_old[evsignal] = NULL; sig->sh_old[evsignal] = NULL;