[libvirt] [PATCH] unlock eventLoop before calling callback function

[Daniel P. Berrange]

On Mon, Mar 07, 2011 at 02:06:49PM +0800, Wen Congyang wrote:
> 
> diff --git a/daemon/event.c b/daemon/event.c
> index 1a31717..0d45014 100644
> --- a/daemon/event.c
> +++ b/daemon/event.c
> @@ -493,8 +493,11 @@ static int virEventCleanupTimeouts(void) {
>  
>          EVENT_DEBUG("Purging timeout %d with id %d", i,
>                      eventLoop.timeouts[i].timer);
> -        if (eventLoop.timeouts[i].ff)
> +        if (eventLoop.timeouts[i].ff) {
> +            virMutexUnlock(&eventLoop.lock);
>              (eventLoop.timeouts[i].ff)(eventLoop.timeouts[i].opaque);
> +            virMutexLock(&eventLoop.lock);
> +        }
>  
>          if ((i+1) < eventLoop.timeoutsCount) {
>              memmove(eventLoop.timeouts+i,
> @@ -534,8 +537,11 @@ static int virEventCleanupHandles(void) {
>              continue;
>          }
>  
> -        if (eventLoop.handles[i].ff)
> +        if (eventLoop.handles[i].ff) {
> +            virMutexUnlock(&eventLoop.lock);
>              (eventLoop.handles[i].ff)(eventLoop.handles[i].opaque);
> +            virMutexLock(&eventLoop.lock);
> +        }
>  
>          if ((i+1) < eventLoop.handlesCount) {
>              memmove(eventLoop.handles+i,

I'm a little concerned as to whether the rest of the code in
virEventCleanupHandles/CleanupTimeouts is safe, if we release
the lock here. eg, if some other thread calls virEventAddTimeout
of AddHandle, is there any way this could cause problems for us
here. So far I think this is safe because AddTimeout/AddHandle
will simply append to the end of the array we're iterating over,
but would like a second opinion before ACK'ing

Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|