Menu
▾
▴
[Levent-commits] [Libevent/master 1/4] Make event_reinit() more robust and maintainable
|
From: Nick M. <ni...@us...> - 2012-01-27 20:12:44
|
Author: Nick Mathewson <ni...@to...>
Date: Fri, 27 Jan 2012 13:54:05 -0500
Subject: Make event_reinit() more robust and maintainable
Commit: 272033efe575a9dc7ec6f123a96afba5c69ff1c6
Previously, event_reinit required a bunch of really dubious hacks,
and violated a lot of abstraction barriers to mess around with lists
of internal events and "pretend" to re-add them.
The new (and fairly well commented!) implementation tries to be much
smarter, by isolating the changes as much as possible to the backend
state, and minimizing the amount of abstraction violations.
Specifically, we now use event_del() to remove events we want to
remove, rather than futzing around with queues in event_reinit().
To avoid bogus calls to evsel->del(), we temporarily replace evsel
with a null-object stub.
Also, we now push the responsibility for calling evsel->add() down
into the evmap code, so that we don't actually need to unlink and
re-link all of our events.
---
event.c | 92 +++++++++++++++++++++++++++++++++++-------------------
evmap-internal.h | 3 ++
evmap.c | 59 ++++++++++++++++++++++++++++++++++
3 files changed, 122 insertions(+), 32 deletions(-)
diff --git a/event.c b/event.c
index 79bf6f8..e5a2d9d 100644
--- a/event.c
+++ b/event.c
@@ -833,6 +833,25 @@ event_base_free(struct event_base *base)
mm_free(base);
}
+/* Fake eventop; used to disable the backend temporarily inside event_reinit
+ * so that we can call event_del() on an event without telling the backend.
+ */
+static int
+nil_backend_del(struct event_base *b, evutil_socket_t fd, short old,
+ short events, void *fdinfo)
+{
+ return 0;
+}
+const struct eventop nil_eventop = {
+ "nil",
+ NULL, /* init: unused. */
+ NULL, /* add: unused. */
+ nil_backend_del, /* del: used, so needs to be killed. */
+ NULL, /* dispatch: unused. */
+ NULL, /* dealloc: unused. */
+ 0, 0, 0
+};
+
/* reinitialize the event base after a fork */
int
event_reinit(struct event_base *base)
@@ -857,13 +876,25 @@ event_reinit(struct event_base *base)
goto done;
#endif
- /* prevent internal delete */
+ /* 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
+ * thread-notification fd. Otherwise, we'll still share those with
+ * the parent process, which would make any notification sent to them
+ * get received by one or both of the event loops, more or less at
+ * random.
+ */
if (base->sig.ev_signal_added) {
- /* we cannot call event_del here because the base has
- * not been reinitialized yet. */
- event_queue_remove_inserted(base, &base->sig.ev_signal);
- if (base->sig.ev_signal.ev_flags & EVLIST_ACTIVE)
- event_queue_remove_active(base, &base->sig.ev_signal);
+ event_del(&base->sig.ev_signal);
+ event_debug_unassign(&base->sig.ev_signal);
+ memset(&base->sig.ev_signal, 0, sizeof(base->sig.ev_signal));
if (base->sig.ev_signal_pair[0] != -1)
EVUTIL_CLOSESOCKET(base->sig.ev_signal_pair[0]);
if (base->sig.ev_signal_pair[1] != -1)
@@ -871,13 +902,8 @@ event_reinit(struct event_base *base)
base->sig.ev_signal_added = 0;
}
if (base->th_notify_fd[0] != -1) {
- /* we cannot call event_del here because the base has
- * not been reinitialized yet. */
was_notifiable = 1;
- event_queue_remove_inserted(base, &base->th_notify);
- if (base->th_notify.ev_flags & EVLIST_ACTIVE)
- event_queue_remove_active(base, &base->th_notify);
- base->sig.ev_signal_added = 0;
+ event_del(&base->th_notify);
EVUTIL_CLOSESOCKET(base->th_notify_fd[0]);
if (base->th_notify_fd[1] != -1)
EVUTIL_CLOSESOCKET(base->th_notify_fd[1]);
@@ -886,7 +912,16 @@ event_reinit(struct event_base *base)
event_debug_unassign(&base->th_notify);
base->th_notify_fn = NULL;
}
+ /* Replace the original evsel. */
+ base->evsel = evsel;
+ /* Reconstruct the backend through brute-force, so that we do not
+ * share any structures with the parent process. For some backends,
+ * this is necessary: epoll and kqueue, for instance, have events
+ * associated with a kernel structure. If didn't reinitialize, we'd
+ * share that structure with the parent process, and any changes made
+ * by the parent would affect our backend's behavior (and vice versa).
+ */
if (base->evsel->dealloc != NULL)
base->evsel->dealloc(base);
base->evbase = evsel->init(base);
@@ -897,27 +932,20 @@ event_reinit(struct event_base *base)
goto done;
}
- event_changelist_freemem(&base->changelist); /* XXX */
- evmap_io_clear(&base->io);
- evmap_signal_clear(&base->sigmap);
+ /* Empty out the changelist (if any): we are starting from a blank
+ * slate. */
+ event_changelist_freemem(&base->changelist);
- TAILQ_FOREACH(ev, &base->eventqueue, ev_next) {
- if (ev->ev_events & (EV_READ|EV_WRITE)) {
- if (ev == &base->sig.ev_signal) {
- /* If we run into the ev_signal event, it's only
- * in eventqueue because some signal event was
- * added, which made evsig_add re-add ev_signal.
- * So don't double-add it. */
- continue;
- }
- if (evmap_io_add(base, ev->ev_fd, ev) == -1)
- res = -1;
- } else if (ev->ev_events & EV_SIGNAL) {
- if (evmap_signal_add(base, (int)ev->ev_fd, ev) == -1)
- res = -1;
- }
- }
+ /* 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
+ * notifiable again. */
if (was_notifiable && res == 0)
res = evthread_make_base_notifiable(base);
diff --git a/evmap-internal.h b/evmap-internal.h
index 8bf655f..6426954 100644
--- a/evmap-internal.h
+++ b/evmap-internal.h
@@ -87,6 +87,9 @@ void evmap_signal_active(struct event_base *base, evutil_socket_t signum, int nc
void *evmap_io_get_fdinfo(struct event_io_map *ctx, evutil_socket_t fd);
+int evmap_io_reinit(struct event_base *base);
+int evmap_signal_reinit(struct event_base *base);
+
void evmap_check_integrity(struct event_base *base);
#endif /* _EVMAP_H_ */
diff --git a/evmap.c b/evmap.c
index 8537b0e..3d1d98b 100644
--- a/evmap.c
+++ b/evmap.c
@@ -488,6 +488,65 @@ evmap_io_get_fdinfo(struct event_io_map *map, evutil_socket_t fd)
return NULL;
}
+int
+evmap_io_reinit(struct event_base *base)
+{
+ int res = 0;
+ evutil_socket_t i;
+ void *extra;
+ short events;
+ const struct eventop *evsel = base->evsel;
+ struct event_io_map *io = &base->io;
+
+#ifdef EVMAP_USE_HT
+ struct event_map_entry **mapent;
+ HT_FOREACH(mapent, event_io_map, io) {
+ struct evmap_io *ctx = &(*mapent)->ent.evmap_io;
+ i = (*mapent)->fd;
+#else
+ for (i = 0; i < io->nentries; ++i) {
+ struct evmap_io *ctx = io->entries[i];
+ if (!ctx)
+ continue;
+#endif
+ events = 0;
+ extra = ((char*)ctx) + sizeof(struct evmap_io);
+ if (ctx->nread)
+ events |= EV_READ;
+ if (ctx->nread)
+ events |= EV_WRITE;
+ if (evsel->fdinfo_len)
+ memset(extra, 0, evsel->fdinfo_len);
+ if (events && LIST_FIRST(&ctx->events) &&
+ (LIST_FIRST(&ctx->events)->ev_events & EV_ET))
+ events |= EV_ET;
+ if (evsel->add(base, i, 0, events, extra) == -1)
+ res = -1;
+ }
+
+ return res;
+}
+
+int
+evmap_signal_reinit(struct event_base *base)
+{
+ struct event_signal_map *sigmap = &base->sigmap;
+ const struct eventop *evsel = base->evsigsel;
+ int res = 0;
+ int i;
+
+ for (i = 0; i < sigmap->nentries; ++i) {
+ struct evmap_signal *ctx = sigmap->entries[i];
+ if (!ctx)
+ continue;
+ if (!LIST_EMPTY(&ctx->events)) {
+ if (evsel->add(base, i, 0, EV_SIGNAL, NULL) == -1)
+ res = -1;
+ }
+ }
+ return res;
+}
+
/** Per-fd structure for use with changelists. It keeps track, for each fd or
* signal using the changelist, of where its entry in the changelist is.
*/
--
1.7.4.1
|