[libvirt] [PATCH 8/9] Fix locking wrt virNetClientStreamPtr object

[Daniel P. Berrange]

On Tue, Jun 28, 2011 at 12:12:53PM -0600, Eric Blake wrote:
> On 06/28/2011 11:01 AM, Daniel P. Berrange wrote:
> > The client stream object can be used independantly of the
> 
> s/independantly/independently/
> 
> > virNetClientPtr object, so must have full locking of its
> > own and not rely on any caller.
> > 
> > * src/remote/remote_driver.c: Remove locking around stream
> >   callback
> > * src/rpc/virnetclientstream.c: Add locking to all APIs
> >   and callbacks
> > ---
> >  src/remote/remote_driver.c   |    3 -
> >  src/rpc/virnetclientstream.c |  112 +++++++++++++++++++++++++++++++++---------
> >  2 files changed, 89 insertions(+), 26 deletions(-)
> > 
> > @@ -390,20 +435,23 @@ int virNetClientStreamEventAddCallback(virNetClientStreamPtr st,
> >                                         void *opaque,
> >                                         virFreeCallback ff)
> >  {
> > +    int ret = -1;
> > +
> > +    virMutexLock(&st->lock);
> >      if (st->cb) {
> >          virNetError(VIR_ERR_INTERNAL_ERROR,
> >                      "%s", _("multiple stream callbacks not supported"));
> > -        return 1;
> > +        goto cleanup;
> >      }
> >  
> > -    virNetClientStreamRef(st);
> > +    st->refs++;
> >      if ((st->cbTimer =
> >           virEventAddTimeout(-1,
> >                              virNetClientStreamEventTimer,
> >                              st,
> >                              virNetClientStreamEventTimerFree)) < 0) {
> > -        virNetClientStreamFree(st);
> > -        return -1;
> > +        st->refs--;
> > +        goto cleanup;
> >      }
> 
> This increments st->refs on success,
> 
> >  int virNetClientStreamEventRemoveCallback(virNetClientStreamPtr st)
> >  {
> > +    int ret = -1;
> > +
> > +    virMutexUnlock(&st->lock);
> >      if (!st->cb) {
> >          virNetError(VIR_ERR_INTERNAL_ERROR,
> >                      "%s", _("no stream callback registered"));
> > -        return -1;
> > +        goto cleanup;
> >      }
> >  
> >      if (!st->cbDispatch &&
> 
> but nothing here ever decrements it.

The decrement is done by the free callback, virNetClientStreamEventTimerFree
asynchronously.


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 :|