Re: [libvirt] [PATCH 2/4] daemon: call free callbacks synchronously in event loop impl
On 22.09.2016 11:44, Daniel P. Berrange wrote: > On Thu, Sep 22, 2016 at 11:38:50AM +0300, Nikolay Shirokovskiy wrote: >> Default event loop impl delays deleting registered handles and >> timeouts so that deleting is safe from event handlers. But >> this means deletions after event loop is finished will never >> occur and particularly assosiated callback objects will not >> be freed. For this reason network clients that exist at >> the moment of libvirtd daemon exit are never get disposed >> and daemon object itself too if there are admin server clients. >> >> Let's call free callbacks immediately we don't need to delay >> this operation, only removing handles from the list. >> >> This breaks virnetsocket.c immediately as socket is locked >> in freeing callback and callback is called synchronously >> from virNetSocketRemoveIOCallback which holds the lock already. >> So fix it to pass intermediate callback object to the loop >> instead of socket itself. I've checked other users of >> virEventAddHandle, looks like they don't have such problems. > > That is impossible to guarantee. The event loop impl is exposed > via the public API, so arbitrary external applications get to > call it. > > So I don't think it is safe to make this change. > Ok, the other I option I think of is to add call to indicate event loop is finished so that there is no need to defer cleanups. Nikolay -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH 2/4] daemon: call free callbacks synchronously in event loop impl
On Thu, Sep 22, 2016 at 11:38:50AM +0300, Nikolay Shirokovskiy wrote: > Default event loop impl delays deleting registered handles and > timeouts so that deleting is safe from event handlers. But > this means deletions after event loop is finished will never > occur and particularly assosiated callback objects will not > be freed. For this reason network clients that exist at > the moment of libvirtd daemon exit are never get disposed > and daemon object itself too if there are admin server clients. > > Let's call free callbacks immediately we don't need to delay > this operation, only removing handles from the list. > > This breaks virnetsocket.c immediately as socket is locked > in freeing callback and callback is called synchronously > from virNetSocketRemoveIOCallback which holds the lock already. > So fix it to pass intermediate callback object to the loop > instead of socket itself. I've checked other users of > virEventAddHandle, looks like they don't have such problems. That is impossible to guarantee. The event loop impl is exposed via the public API, so arbitrary external applications get to call it. So I don't think it is safe to make this change. Regards, 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 :| -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
