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

29 Jun 2011


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

    