[PATCH] event-loop: Use two-step destruction of event loop sources.

[Ander Conselvan de Oliveira]

On 03/20/2012 03:21 PM, Jonas Ådahl wrote:
> Instead of directly destroying an event source upon removal only destroy
> it in case its event loop is not dispatching. If dispatching the event
> source is marked for destruction and added to the check list and
> destroyed in post_dispatch_check(); if not it is destroyed immediately.
>
> Signed-off-by: Jonas Ådahl<jadahl at gmail.com>

Looks good me. Though maybe we want a wl_event_source_init() function to 
reduce the duplicated code from wl_event_loop_add* and avoid the 
calloc()'s. That also could go in another patch. Either way,

Reviewed-by: Ander Conselvan de Oliveira <conselvan2 at gmail.com>

> ---
>   src/event-loop.c |   73 ++++++++++++++++++++++++++++++++++-------------------
>   1 files changed, 47 insertions(+), 26 deletions(-)
>
> diff --git a/src/event-loop.c b/src/event-loop.c
> index 2dfe0ae..7f2ef80 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -39,12 +39,15 @@ struct wl_event_loop {
>   	int epoll_fd;
>   	struct wl_list check_list;
>   	struct wl_list idle_list;
> +
> +	int dispatching;
>   };
>
>   struct wl_event_source_interface {
>   	int (*dispatch)(struct wl_event_source *source,
>   			struct epoll_event *ep);
>   	int (*remove)(struct wl_event_source *source);
> +	int (*destroy)(struct wl_event_source *source);
>   };
>
>   struct wl_event_source {
> @@ -52,6 +55,7 @@ struct wl_event_source {
>   	struct wl_event_loop *loop;
>   	struct wl_list link;
>   	void *data;
> +	int destroy;
>   };
>
>   struct wl_event_source_fd {
> @@ -61,6 +65,13 @@ struct wl_event_source_fd {
>   };
>
>   static int
> +wl_event_source_generic_destroy(struct wl_event_source *source)
> +{
> +	free(source);
> +	return 0;
> +}
> +
> +static int
>   wl_event_source_fd_dispatch(struct wl_event_source *source,
>   			    struct epoll_event *ep)
>   {
> @@ -82,17 +93,14 @@ wl_event_source_fd_remove(struct wl_event_source *source)
>   	struct wl_event_source_fd *fd_source =
>   		(struct wl_event_source_fd *) source;
>   	struct wl_event_loop *loop = source->loop;
> -	int fd;
> -
> -	fd = fd_source->fd;
> -	free(source);
>
> -	return epoll_ctl(loop->epoll_fd, EPOLL_CTL_DEL, fd, NULL);
> +	return epoll_ctl(loop->epoll_fd, EPOLL_CTL_DEL, fd_source->fd, NULL);
>   }
>
>   struct wl_event_source_interface fd_source_interface = {
>   	wl_event_source_fd_dispatch,
> -	wl_event_source_fd_remove
> +	wl_event_source_fd_remove,
> +	wl_event_source_generic_destroy
>   };
>
>   WL_EXPORT struct wl_event_source *
> @@ -104,7 +112,7 @@ wl_event_loop_add_fd(struct wl_event_loop *loop,
>   	struct wl_event_source_fd *source;
>   	struct epoll_event ep;
>
> -	source = malloc(sizeof *source);
> +	source = calloc(1, sizeof *source);
>   	if (source == NULL)
>   		return NULL;
>
> @@ -179,13 +187,13 @@ wl_event_source_timer_remove(struct wl_event_source *source)
>   		(struct wl_event_source_timer *) source;
>
>   	close(timer_source->fd);
> -	free(source);
>   	return 0;
>   }
>
>   struct wl_event_source_interface timer_source_interface = {
>   	wl_event_source_timer_dispatch,
> -	wl_event_source_timer_remove
> +	wl_event_source_timer_remove,
> +	wl_event_source_generic_destroy
>   };
>
>   WL_EXPORT struct wl_event_source *
> @@ -196,7 +204,7 @@ wl_event_loop_add_timer(struct wl_event_loop *loop,
>   	struct wl_event_source_timer *source;
>   	struct epoll_event ep;
>
> -	source = malloc(sizeof *source);
> +	source = calloc(1, sizeof *source);
>   	if (source == NULL)
>   		return NULL;
>
> @@ -278,13 +286,13 @@ wl_event_source_signal_remove(struct wl_event_source *source)
>   		(struct wl_event_source_signal *) source;
>
>   	close(signal_source->fd);
> -	free(source);
>   	return 0;
>   }
>
>   struct wl_event_source_interface signal_source_interface = {
>   	wl_event_source_signal_dispatch,
> -	wl_event_source_signal_remove
> +	wl_event_source_signal_remove,
> +	wl_event_source_generic_destroy
>   };
>
>   WL_EXPORT struct wl_event_source *
> @@ -297,7 +305,7 @@ wl_event_loop_add_signal(struct wl_event_loop *loop,
>   	struct epoll_event ep;
>   	sigset_t mask;
>
> -	source = malloc(sizeof *source);
> +	source = calloc(1, sizeof *source);
>   	if (source == NULL)
>   		return NULL;
>
> @@ -337,17 +345,10 @@ struct wl_event_source_idle {
>   	wl_event_loop_idle_func_t func;
>   };
>
> -static int
> -wl_event_source_idle_remove(struct wl_event_source *source)
> -{
> -	free(source);
> -
> -	return 0;
> -}
> -
>   struct wl_event_source_interface idle_source_interface = {
>   	NULL,
> -	wl_event_source_idle_remove
> +	NULL,
> +	wl_event_source_generic_destroy
>   };
>
>   WL_EXPORT struct wl_event_source *
> @@ -357,7 +358,7 @@ wl_event_loop_add_idle(struct wl_event_loop *loop,
>   {
>   	struct wl_event_source_idle *source;
>
> -	source = malloc(sizeof *source);
> +	source = calloc(1, sizeof *source);
>   	if (source == NULL)
>   		return NULL;
>
> @@ -381,11 +382,22 @@ wl_event_source_check(struct wl_event_source *source)
>   WL_EXPORT int
>   wl_event_source_remove(struct wl_event_source *source)
>   {
> +	struct wl_event_loop *loop = source->loop;
> +
>   	if (!wl_list_empty(&source->link))
>   		wl_list_remove(&source->link);
>
>   	source->interface->remove(source);
>
> +	if (loop->dispatching) {
> +		source->destroy = 1;
> +
> +		if (!wl_list_empty(&source->link))
> +			wl_list_remove(&source->link);
> +		wl_event_source_check(source);
> +	} else
> +		source->interface->destroy(source);
> +
>   	return 0;
>   }
>
> @@ -425,8 +437,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)
> -		n += source->interface->dispatch(source,&ep);
> +	wl_list_for_each_safe(source, next,&loop->check_list, link) {
> +		if (source->destroy) {
> +			wl_list_remove(&source->link);
> +			source->interface->destroy(source);
> +		} else
> +			n += source->interface->dispatch(source,&ep);
> +	}
>
>   	return n;
>   }
> @@ -456,11 +473,15 @@ wl_event_loop_dispatch(struct wl_event_loop *loop, int timeout)
>   	count = epoll_wait(loop->epoll_fd, ep, ARRAY_LENGTH(ep), timeout);
>   	if (count<  0)
>   		return -1;
> +
> +	loop->dispatching = 1;
>   	n = 0;
>   	for (i = 0; i<  count; i++) {
>   		source = ep[i].data.ptr;
> -		n += source->interface->dispatch(source,&ep[i]);
> +		if (!source->destroy)
> +			n += source->interface->dispatch(source,&ep[i]);
>   	}
> +	loop->dispatching = 0;
>
>   	while (n>  0)
>   		n = post_dispatch_check(loop);