Re: [PATCH 1/5] rpc: push ref acquisition into RPC dispatch function

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/06/2018 10:58 AM, Daniel P. Berrangé wrote:
There's no reason why the virNetServerClientDispatchRead method needs to
acquire an extra reference on the "client" object. An extra reference is
only needed if the registered dispatch callback is going to keep hold of
the "client" for work in the background. Thus we can push reference
acquisition into virNetServerDispatchNewMessage.

Signed-off-by: Daniel P. Berrangé <berrange@xxxxxxxxxx>
---
  src/rpc/virnetserver.c       | 2 ++
  src/rpc/virnetserverclient.c | 2 --
  2 files changed, 2 insertions(+), 2 deletions(-)

I haven't worked on this code much, but this change seems sane to me regardless of the lost locks issue.

Reviewed-by: Jim Fehlig <jfehlig@xxxxxxxx>

Regards,
Jim


diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 204f425264..28afe54e49 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -217,9 +217,11 @@ static int virNetServerDispatchNewMessage(virNetServerClientPtr client,
              priority = virNetServerProgramGetPriority(prog, msg->header.proc);
          }
+ virObjectRef(client);
          ret = virThreadPoolSendJob(srv->workers, priority, job);
if (ret < 0) {
+            virObjectUnref(client);
              VIR_FREE(job);
              virObjectUnref(prog);
          }
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 388514946b..00459d17ba 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -1315,12 +1315,10 @@ static void virNetServerClientDispatchRead(virNetServerClientPtr client)
/* Send off to for normal dispatch to workers */
          if (msg) {
-            virObjectRef(client);
              if (!client->dispatchFunc ||
                  client->dispatchFunc(client, msg, client->dispatchOpaque) < 0) {
                  virNetMessageFree(msg);
                  client->wantClose = true;
-                virObjectUnref(client);
                  return;
              }
          }


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux