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
This commit is contained in:
Daniel P. Berrange 2011-12-07 14:46:39 +00:00 committed by Daniel Veillard
parent 50a4f49c19
commit e970863746

View File

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