From: Daniel P. Berrange <berrange@redhat.com>
Date: Wed, 7 Dec 2011 14:46:39 +0000 (+0000)
Subject: Fix updating of haveTheBuck in RPC client to be race-free
X-Git-Tag: v0.9.8~3
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e970863746448a299533a5b3b4b2cecba361544c;p=thirdparty%2Flibvirt.git

Fix updating of haveTheBuck in RPC client to be race-free

When one thread passes the buck to another thread, it uses
virCondSignal to wake up the target thread. The variable
'haveTheBuck' is not updated in a race-free manner when
this occurs. The current thread sets it to false, and the
woken up thread sets it to true. There is a window where
a 3rd thread can come in and grab the buck.

Even if this didn't lead to crashes & deadlocks, this would
still result in unfairness in the buckpassing algorithm.

A better solution is to *never* set haveTheBuck to false
when we're passing the buck. Only set it to false when there
is no further thread waiting for the buck.

* src/rpc/virnetclient.c: Only set haveTheBuck to false
  if no thread is waiting
---

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index e35521d45a..469c6a5a4a 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1265,6 +1265,7 @@ static void virNetClientIOEventLoopPassTheBuck(virNetClientPtr client, virNetCli
         }
         tmp = tmp->next;
     }
+    client->haveTheBuck = false;
 
     VIR_DEBUG("No thread to pass the buck to");
     if (client->wantClose) {
@@ -1593,10 +1594,11 @@ static int virNetClientIO(virNetClientPtr client,
         }
 
         /* Grr, someone passed the buck onto us ... */
+    } else {
+        client->haveTheBuck = true;
     }
 
     VIR_DEBUG("We have the buck %p %p", client->waitDispatch, thiscall);
-    client->haveTheBuck = true;
 
     /*
      * The buck stops here!
@@ -1624,8 +1626,6 @@ static int virNetClientIO(virNetClientPtr client,
         virGetLastError())
         rv = -1;
 
-    client->haveTheBuck = false;
-
 cleanup:
     VIR_DEBUG("All done with our call %p %p %d", client->waitDispatch, thiscall, rv);
     return rv;