[sheepdog] [PATCH v2 1/3] sheep: make {register, unregister}_event thread safe

[Hitoshi Mitake]

At Tue, 17 Dec 2013 14:31:56 +0800,
Liu Yuan wrote:
> 
> This allow us to call even handling functions in worker thread
> 
> Signed-off-by: Liu Yuan <namei.unix at gmail.com>
> ---

I think this change is dangerous. This permits worker threads to
unregister events even if these events are processed in the main
thread. Making a new work queue and delegate it to register/unregister
events would be safer.

Thanks,
Hitoshi

>  lib/event.c          |   14 ++++++++++++--
>  shepherd/Makefile.am |    2 +-
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/lib/event.c b/lib/event.c
> index 88078f4..16aa921 100644
> --- a/lib/event.c
> +++ b/lib/event.c
> @@ -21,6 +21,7 @@
>  
>  static int efd;
>  static struct rb_root events_tree = RB_ROOT;
> +static struct sd_lock events_lock = SD_LOCK_INITIALIZER;
>  
>  static void timer_handler(int fd, int events, void *data)
>  {
> @@ -92,8 +93,12 @@ int init_event(int nr)
>  static struct event_info *lookup_event(int fd)
>  {
>  	struct event_info key = { .fd = fd };
> +	struct event_info *ret;
>  
> -	return rb_search(&events_tree, &key, rb, event_cmp);
> +	sd_read_lock(&events_lock);
> +	ret = rb_search(&events_tree, &key, rb, event_cmp);
> +	sd_unlock(&events_lock);
> +	return ret;
>  }
>  
>  int register_event_prio(int fd, event_handler_t h, void *data, int prio)
> @@ -116,8 +121,11 @@ int register_event_prio(int fd, event_handler_t h, void *data, int prio)
>  	if (ret) {
>  		sd_err("failed to add epoll event: %m");
>  		free(ei);
> -	} else
> +	} else {
> +		sd_write_lock(&events_lock);
>  		rb_insert(&events_tree, ei, rb, event_cmp);
> +		sd_unlock(&events_lock);
> +	}
>  
>  	return ret;
>  }
> @@ -135,7 +143,9 @@ void unregister_event(int fd)
>  	if (ret)
>  		sd_err("failed to delete epoll event for fd %d: %m", fd);
>  
> +	sd_write_lock(&events_lock);
>  	rb_erase(&ei->rb, &events_tree);
> +	sd_unlock(&events_lock);
>  	free(ei);
>  
>  	/*
> diff --git a/shepherd/Makefile.am b/shepherd/Makefile.am
> index 33a3da6..86f45be 100644
> --- a/shepherd/Makefile.am
> +++ b/shepherd/Makefile.am
> @@ -25,7 +25,7 @@ sbin_PROGRAMS		= shepherd
>  
>  shepherd_SOURCES		= shepherd.c
>  
> -shepherd_LDADD	  	= ../lib/libsheepdog.a
> +shepherd_LDADD	  	= ../lib/libsheepdog.a -lpthread
>  shepherd_DEPENDENCIES	= ../lib/libsheepdog.a
>  
>  EXTRA_DIST		=
> -- 
> 1.7.9.5
> 
> -- 
> sheepdog mailing list
> sheepdog at lists.wpkg.org
> http://lists.wpkg.org/mailman/listinfo/sheepdog