Re: [libvirt] [PATCH] Fix race in ref counting when handling RPC jobs

2011-07-26 Thread Eric Blake

On 07/26/2011 08:56 AM, Daniel P. Berrange wrote:

From: "Daniel P. Berrange"

When an incoming RPC message is ready for processing,

   virNetServerClientDispatchRead()

will invoke the 'dispatchFunc' callback. This is set to

   virNetServerDispatchNewMessage

This function puts the message + client in a queue for processing by the thread
pool. The thread pool worker function is

   virNetServerHandleJob

The first thing this does is acquire an extra reference on the 'client'.

Unfortunately, between the time the message+client are put on the thread pool
queue, and the time the worker runs, the client object may have had its last
reference removed.

We clearly need to add the reference to the client object before putting the
client on the processing queue


ACK.

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

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


[libvirt] [PATCH] Fix race in ref counting when handling RPC jobs

2011-07-26 Thread Daniel P. Berrange
From: "Daniel P. Berrange" 

When an incoming RPC message is ready for processing,

  virNetServerClientDispatchRead()

will invoke the 'dispatchFunc' callback. This is set to

  virNetServerDispatchNewMessage

This function puts the message + client in a queue for processing by the thread
pool. The thread pool worker function is

  virNetServerHandleJob

The first thing this does is acquire an extra reference on the 'client'.

Unfortunately, between the time the message+client are put on the thread pool
queue, and the time the worker runs, the client object may have had its last
reference removed.

We clearly need to add the reference to the client object before putting the
client on the processing queue

* src/rpc/virnetserverclient.c: Add a reference to the client when
  invoking the dispatch function
* src/rpc/virnetserver.c: Don't acquire a reference to the client
  when in the worker thread
---
 src/rpc/virnetserver.c   |2 --
 src/rpc/virnetserverclient.c |2 ++
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c
index 2b9dd4d..c93c3f8 100644
--- a/src/rpc/virnetserver.c
+++ b/src/rpc/virnetserver.c
@@ -131,8 +131,6 @@ static void virNetServerHandleJob(void *jobOpaque, void 
*opaque)
 virNetServerProgramPtr prog = NULL;
 size_t i;
 
-virNetServerClientRef(job->client);
-
 virNetServerLock(srv);
 VIR_DEBUG("server=%p client=%p message=%p",
   srv, job->client, job->msg);
diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c
index 317d59c..3c0dba8 100644
--- a/src/rpc/virnetserverclient.c
+++ b/src/rpc/virnetserverclient.c
@@ -763,10 +763,12 @@ readmore:
 
 /* Send off to for normal dispatch to workers */
 if (msg) {
+client->refs++;
 if (!client->dispatchFunc ||
 client->dispatchFunc(client, msg, client->dispatchOpaque) < 0) 
{
 virNetMessageFree(msg);
 client->wantClose = true;
+client->refs--;
 return;
 }
 }
-- 
1.7.6

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