[PATCH] event-loop: Allow source dispatches to remove other sources

[Andreas Ericsson]

When a dispatch for sourceA wishes to remove sourceB and sourceB
has input that isn't yet processed, we would run into the dreaded
"undefined behaviour" previously.

With this patch, the destroyed source is ignored while processing
input and is later free()'d in the post_dispatch_check() loop.

Signed-off-by: Andreas Ericsson <ae at op5.se>
---

I actually haven't managed to build wayland yet, and since I'm stuck
with an nvidia videocard it's not likely to happen anytime in the
near future either, so this patch is, unfortunately, totally untested.

 src/event-loop.c |   42 ++++++++++++++++++++++++++++++++++++++----
 1 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/src/event-loop.c b/src/event-loop.c
index 2dfe0ae..02d5163 100644
--- a/src/event-loop.c
+++ b/src/event-loop.c
@@ -37,6 +37,7 @@
 
 struct wl_event_loop {
 	int epoll_fd;
+	int *source_flags;
 	struct wl_list check_list;
 	struct wl_list idle_list;
 };
@@ -48,6 +49,7 @@ struct wl_event_source_interface {
 };
 
 struct wl_event_source {
+	int destroyed;
 	struct wl_event_source_interface *interface;
 	struct wl_event_loop *loop;
 	struct wl_list link;
@@ -85,7 +87,21 @@ wl_event_source_fd_remove(struct wl_event_source *source)
 	int fd;
 
 	fd = fd_source->fd;
-	free(source);
+
+	/*
+	 * a running dispatch can remove another source, which
+	 * will cause crashes if that other source is also queued
+	 * for dispatch, since we'll grab the source pointer from
+	 * the epoll event. If that happens, we have to mark the
+	 * source as destroyed here instead of free()'ing so we
+	 * know not to touch it later.
+	 */
+	if (loop->dispatching) {
+		source->destroyed = 1;
+		wl_event_source_check(source);
+	} else {
+		free(source);
+	}
 
 	return epoll_ctl(loop->epoll_fd, EPOLL_CTL_DEL, fd, NULL);
 }
@@ -104,7 +120,10 @@ wl_event_loop_add_fd(struct wl_event_loop *loop,
 	struct wl_event_source_fd *source;
 	struct epoll_event ep;
 
-	source = malloc(sizeof *source);
+	if (fd < 0)
+		return NULL;
+
+	source = calloc(1, sizeof *source);
 	if (source == NULL)
 		return NULL;
 
@@ -394,7 +413,7 @@ wl_event_loop_create(void)
 {
 	struct wl_event_loop *loop;
 
-	loop = malloc(sizeof *loop);
+	loop = calloc(1, sizeof *loop);
 	if (loop == NULL)
 		return NULL;
 
@@ -425,8 +444,13 @@ post_dispatch_check(struct wl_event_loop *loop)
 
 	ep.events = 0;
 	n = 0;
-	wl_list_for_each_safe(source, next, &loop->check_list, link)
+	wl_list_for_each_safe(source, next, &loop->check_list, link) {
+		if (source->destroyed) {
+			free(source);
+			continue;
+		}
 		n += source->interface->dispatch(source, &ep);
+	}
 
 	return n;
 }
@@ -457,10 +481,20 @@ wl_event_loop_dispatch(struct wl_event_loop *loop, int timeout)
 	if (count < 0)
 		return -1;
 	n = 0;
+	loop->dispatching = 1;
 	for (i = 0; i < count; i++) {
 		source = ep[i].data.ptr;
+		if (source->destroyed) {
+			/*
+			 * An earlier dispatch removed this source, so it's
+			 * no longer there. We ignore its input and let the
+			 * post_dispatch_check() free() it.
+			 */
+			continue;
+		}
 		n += source->interface->dispatch(source, &ep[i]);
 	}
+	loop->dispatching = 0;
 
 	while (n > 0)
 		n = post_dispatch_check(loop);
-- 
1.7.4.4