From e970863746448a299533a5b3b4b2cecba361544c Mon Sep 17 00:00:00 2001 From: "Daniel P. Berrange" Date: Wed, 7 Dec 2011 14:46:39 +0000 Subject: [PATCH] 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 --- src/rpc/virnetclient.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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;