rpc: client stream: dispose private data on stream dispose

If we call virStreamFinish and virStreamAbort from 2 distinct
threads for example we can have access to freed memory.
Because when virStreamFinish finishes for example virStreamAbort
yet to be finished and it access virNetClientStreamPtr object
in stream->privateData.

Also it does not make sense to clear @driver field. After
stream is finished/aborted it is better to have appropriate
error message instead of "unsupported error".

This commit reverts [1] or virNetClientStreamPtr and
virStreamPtr will never be unrefed due to cyclic dependency.
Before this patch we don't have leaks because all execution
paths we call virStreamFinish or virStreamAbort.

[1] 8b6ffe40 : virNetClientStreamNew: Track origin stream

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>

diff --git a/src/datatypes.c b/src/datatypes.c
index 09f63d9e151eb9f2068e04efba00c6db435da308..be9b5286aa9f8582d57c9acd90c81c48f393c8b2 100644 (file)
--- a/src/datatypes.c
+++ b/src/datatypes.c
@@ -763,6 +763,8 @@ virStreamDispose(void *obj)
     virStreamPtr st = obj;
     VIR_DEBUG("release dev %p", st);
 
+    if (st->ff)
+        st->ff(st->privateData);
     virObjectUnref(st->conn);
 }
 

diff --git a/src/datatypes.h b/src/datatypes.h
index 529b3405874b83323745b29082b7f6a8a4b44d5b..12015679f3b4ae7f821d6e5cd41cbd0843f90eaa 100644 (file)
--- a/src/datatypes.h
+++ b/src/datatypes.h
@@ -665,6 +665,7 @@ struct _virStream {
 
     virStreamDriverPtr driver;
     void *privateData;
+    virFreeCallback ff;
 };
 
 /**

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 1ff55e241ac5ba921951603d9afcdb475cdeba17..2861ee68e20946a8ed543a8c78ece296e003e64b 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -5835,9 +5835,6 @@ remoteStreamCloseInt(virStreamPtr st, bool streamAbort)
     priv->localUses--;
 
     virNetClientRemoveStream(priv->client, privst);
-    virObjectUnref(privst);
-    st->privateData = NULL;
-    st->driver = NULL;
 
     remoteDriverUnlock(priv);
     return ret;
@@ -6177,8 +6174,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn,
     memset(&args, 0, sizeof(args));
     memset(&ret, 0, sizeof(ret));
 
-    if (!(netst = virNetClientStreamNew(st,
-                                        priv->remoteProgram,
+    if (!(netst = virNetClientStreamNew(priv->remoteProgram,
                                         REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3,
                                         priv->counter,
                                         false)))
@@ -6191,6 +6187,7 @@ remoteDomainMigratePrepareTunnel3(virConnectPtr dconn,
 
     st->driver = &remoteStreamDrv;
     st->privateData = netst;
+    st->ff = virObjectFreeCallback;
 
     args.cookie_in.cookie_in_val = (char *)cookiein;
     args.cookie_in.cookie_in_len = cookieinlen;
@@ -7142,8 +7139,7 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
         goto cleanup;
     }
 
-    if (!(netst = virNetClientStreamNew(st,
-                                        priv->remoteProgram,
+    if (!(netst = virNetClientStreamNew(priv->remoteProgram,
                                         REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3_PARAMS,
                                         priv->counter,
                                         false)))
@@ -7156,6 +7152,7 @@ remoteDomainMigratePrepareTunnel3Params(virConnectPtr dconn,
 
     st->driver = &remoteStreamDrv;
     st->privateData = netst;
+    st->ff = virObjectFreeCallback;
 
     if (call(dconn, priv, 0, REMOTE_PROC_DOMAIN_MIGRATE_PREPARE_TUNNEL3_PARAMS,
              (xdrproc_t) xdr_remote_domain_migrate_prepare_tunnel3_params_args,

diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index ce4db5d7b79a348192456adcb5279dc4ba33ef9e..ae3a42c4c1b00a850c20ca78db7707fd9a4ee438 100755 (executable)
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1804,7 +1804,7 @@ elsif ($mode eq "client") {
 
         if ($call->{streamflag} ne "none") {
             print "\n";
-            print "    if (!(netst = virNetClientStreamNew(st, priv->remoteProgram, $call->{constname}, priv->counter, sparse)))\n";
+            print "    if (!(netst = virNetClientStreamNew(priv->remoteProgram, $call->{constname}, priv->counter, sparse)))\n";
             print "        goto done;\n";
             print "\n";
             print "    if (virNetClientAddStream(priv->client, netst) < 0) {\n";
@@ -1814,6 +1814,7 @@ elsif ($mode eq "client") {
             print "\n";
             print "    st->driver = &remoteStreamDrv;\n";
             print "    st->privateData = netst;\n";
+            print "    st->ff = virObjectFreeCallback;\n";
         }
 
         if ($call->{ProcName} eq "SupportsFeature") {

diff --git a/src/rpc/virnetclientstream.c b/src/rpc/virnetclientstream.c
index 32349f0d42bee996737edc0912e56a898930291f..f27dcbfea79095bbf73f033930e1742510f015ac 100644 (file)
--- a/src/rpc/virnetclientstream.c
+++ b/src/rpc/virnetclientstream.c
@@ -34,8 +34,6 @@ VIR_LOG_INIT("rpc.netclientstream");
 struct _virNetClientStream {
     virObjectLockable parent;
 
-    virStreamPtr stream; /* Reverse pointer to parent stream */
-
     virNetClientProgramPtr prog;
     int proc;
     unsigned serial;
@@ -133,8 +131,7 @@ virNetClientStreamEventTimer(int timer ATTRIBUTE_UNUSED, void *opaque)
 }
 
 
-virNetClientStreamPtr virNetClientStreamNew(virStreamPtr stream,
-                                            virNetClientProgramPtr prog,
+virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog,
                                             int proc,
                                             unsigned serial,
                                             bool allowSkip)
@@ -147,7 +144,6 @@ virNetClientStreamPtr virNetClientStreamNew(virStreamPtr stream,
     if (!(st = virObjectLockableNew(virNetClientStreamClass)))
         return NULL;
 
-    st->stream = virObjectRef(stream);
     st->prog = virObjectRef(prog);
     st->proc = proc;
     st->serial = serial;
@@ -167,7 +163,6 @@ void virNetClientStreamDispose(void *obj)
         virNetMessageFree(msg);
     }
     virObjectUnref(st->prog);
-    virObjectUnref(st->stream);
 }
 
 bool virNetClientStreamMatches(virNetClientStreamPtr st,

diff --git a/src/rpc/virnetclientstream.h b/src/rpc/virnetclientstream.h
index d81ec60a48250f2f88af39d470a1419a889373b1..49b74bcc41b252268e78823c833d210106b6fbbd 100644 (file)
--- a/src/rpc/virnetclientstream.h
+++ b/src/rpc/virnetclientstream.h
@@ -30,8 +30,7 @@ typedef virNetClientStream *virNetClientStreamPtr;
 typedef void (*virNetClientStreamEventCallback)(virNetClientStreamPtr stream,
                                                 int events, void *opaque);
 
-virNetClientStreamPtr virNetClientStreamNew(virStreamPtr stream,
-                                            virNetClientProgramPtr prog,
+virNetClientStreamPtr virNetClientStreamNew(virNetClientProgramPtr prog,
                                             int proc,
                                             unsigned serial,
                                             bool allowSkip);