Hi,
I've been looking at using guest control for some automation, and I've
encountered some problems that make the guest control part of the
VBoxService daemon stop working. It's easy to reproduce this by using
guestcontrol to execute /bin/true lots of times in a loop.
The root cause appears to be that after one request is handled by the
per-process thread, the main guest control thread in VBoxService can set
up another request before the per-process thread expects one. Two
problems appear because of this:
1. Occasionally the per-process thread will wipe out the request pointer
after it has already been updated to the new request, so the new request
is never processed. If you're logging you see a "IPC request is
invalid" message instead. This is easily solved by moving the
assignment before the code to wake up the main thread.
2. When the per-process thread realises it can shut down, if the main
thread produces another request it will deadlock when the per-process
thread attempts to enter a critical section. Fixing the deadlock isn't
enough because it still won't process the request. I've worked around
this by making the per-process thread set the shutdown flag and continue
iterating through the main loop until it's sure that there's no request.
I've attached a patch with these changes, although I think the way that
guest control is implemented in VBoxService may need to be revisited -
it seems a little confused and fragile, and my change for 2 is a bit of
a hack. Also, there's a memory leak that I haven't tried to diagnose.
Best regards,
Brian Campbell
diff -urp VirtualBox-4.1.18.orig/src/VBox/Additions/common/VBoxService/VBoxServiceControlThread.cpp VirtualBox-4.1.18/src/VBox/Additions/common/VBoxService/VBoxServiceControlThread.cpp
--- VirtualBox-4.1.18.orig/src/VBox/Additions/common/VBoxService/VBoxServiceControlThread.cpp 2012-06-20 14:09:03.000000000 +0100
+++ VirtualBox-4.1.18/src/VBox/Additions/common/VBoxService/VBoxServiceControlThread.cpp 2012-07-16 09:33:38.972009716 +0100
@@ -535,6 +535,9 @@ static int VBoxServiceControlThreadHandl
VBoxServiceVerbose(2, "[PID %u]: Handled req=%u, CID=%u, rc=%Rrc, cbData=%u\n",
pThread->uPID, pRequest->enmType, pRequest->uCID, pRequest->rc, pRequest->cbData);
+ /* We've completed the request, so drop access to it. */
+ pThread->pRequest = NULL;
+
/* In any case, regardless of the result, we notify
* the main guest control to unblock it. */
int rc2 = RTSemEventMultiSignal(pRequest->Event);
@@ -542,7 +545,7 @@ static int VBoxServiceControlThreadHandl
/* No access to pRequest here anymore -- could be out of scope
* or modified already! */
- pThread->pRequest = pRequest = NULL;
+ pRequest = NULL;
return rc;
}
@@ -690,8 +693,15 @@ static int VBoxServiceControlThreadProcL
*/
if ( !fProcessAlive
&& *phStdOutR == NIL_RTPIPE
- && *phStdErrR == NIL_RTPIPE)
- break;
+ && *phStdErrR == NIL_RTPIPE) {
+ /* Set shutdown and go around again until we're sure the main
+ * thread is finished its last request. */
+ if (ASMAtomicXchgBool(&pThread->fShutdown, true) &&
+ !RTCritSectIsOwned(&pThread->CritSect))
+ break;
+ else
+ continue;
+ }
/*
* Check for timed out, killing the process.
_______________________________________________
vbox-dev mailing list
[email protected]
https://www.virtualbox.org/mailman/listinfo/vbox-dev
