On Fri, Dec 02, 2011 at 14:06:06 +0100, Peter Krempa wrote:
Originaly, the code checked if another client is the queue and
infered
ownership of the buck from that. Commit fa9595003d043df9f2efe95521c008
added a separate variable to track the buck. That caused, that a new
call might enter claiming it has the buck, while another thread was
signalled to take the buck. This ends in two threads claiming they hold
the buck and entering poll(). This happens due to a race on waking up
threads on the client lock mutex.
This caused multi-threaded clients to hang, most prominently visible and
reproducible on python based clients, like virt-manager.
This patch causes threads, that have been signalled to take the buck to
re-check if buck is held by another thread.
---
src/rpc/virnetclient.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 5165c8d..07d8a60 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1545,6 +1545,7 @@ static int virNetClientIO(virNetClientPtr client,
virNetClientCallQueue(&client->waitDispatch, thiscall);
/* Check to see if another thread is dispatching */
+recheck:
if (client->haveTheBuck) {
char ignore = 1;
@@ -1592,7 +1593,13 @@ static int virNetClientIO(virNetClientPtr client,
goto cleanup;
}
- /* Grr, someone passed the buck onto us ... */
+ /* Grr, someone might have passed the buck onto us ... */
+
+ /* We need to re-check if the buck has been passed to this thread
+ * as this thread might have been signalled to wake up, but the another
Just remove "the" from "but the another".
+ * call might acquire the lock before this thread manages to
wake up.
+ * This could cause that two threads claim they have the buck */
+ goto recheck;
}
VIR_DEBUG("We have the buck %p %p", client->waitDispatch, thiscall);
ACK
Jirka