Opened 10 years ago
Closed 10 years ago
#13722 closed defect (fixed)
ConsoleImpl.cpp race condition
| Reported by: | a.urakov | Owned by: | |
|---|---|---|---|
| Component: | other | Version: | VirtualBox 4.3.20 |
| Keywords: | race condition, ConsoleImpl.cpp | Cc: | |
| Guest type: | Windows | Host type: | Linux |
Description
There is possible race condition in doMediumChange function of ConsoleImpl.cpp. It appears in code
/*
* Call worker in EMT, that's faster and safer than doing everything
* using VMR3ReqCall. Note that we separate VMR3ReqCall from VMR3ReqWait
* here to make requests from under the lock in order to serialize them.
*/
PVMREQ pReq;
int vrc = VMR3ReqCallU(pUVM, VMCPUID_ANY, &pReq, 0 /* no wait! */, VMREQFLAGS_VBOX_STATUS,
(PFNRT)changeRemovableMedium, 8,
this, pUVM, pszDevice, uInstance, enmBus, fUseHostIOCache, aMediumAttachment, fForce);
/* release the lock before waiting for a result (EMT will call us back!) */
alock.release();
if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc))
{
vrc = VMR3ReqWait(pReq, RT_INDEFINITE_WAIT);
AssertRC(vrc);
if (RT_SUCCESS(vrc))
vrc = pReq->iStatus;
}
VMR3ReqFree(pReq);
when EMT thread completes request earlier than VMR3ReqWait (with zero wait time) in VMR3ReqQueue function is called:
/*
* Notify EMT.
*/
if (pUVM->pVM)
VMCPU_FF_SET(pVCpu, VMCPU_FF_REQUEST);
VMR3NotifyCpuFFU(pUVCpu, fFlags & VMREQFLAGS_POKE ? VMNOTIFYFF_FLAGS_POKE : 0);
//
// !!! Request is already completed here !!!
//
/*
* Wait and return.
*/
if (!(fFlags & VMREQFLAGS_NO_WAIT))
rc = VMR3ReqWait(pReq, cMillies);
LogFlow(("VMR3ReqQueue: returns %Rrc\n", rc));
In this case VMR3ReqWait of VMR3ReqQueue function (and all its callers VMR3ReqCallVU and VMR3ReqCallU) returns VINF_SUCCESS instead of VERR_TIMEOUT and decreases pReq->EventSem. So VMR3ReqWait(pReq, RT_INDEFINITE_WAIT) of doMediumChange indefinitely waits.
Possible solution (works for us) is to call VMR3ReqWait of doMediumChange only when it was not completed:
if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc))
{
if (!RT_SUCCESS(vrc))
vrc = VMR3ReqWait(pReq, RT_INDEFINITE_WAIT);
AssertRC(vrc);
if (RT_SUCCESS(vrc))
vrc = pReq->iStatus;
}
A similar problem exists in doCPURemove, doCPUAdd, doStorageDeviceAttach, doStorageDeviceDetach etc.
Attachments (2)
Change History (9)
by , 10 years ago
| Attachment: | 2014-12-30-07-45-23.053-VBoxHeadless-14685.log.bz2 added |
|---|
comment:1 by , 10 years ago
I think you are right. Though passing VMREQFLAGS_NO_WAIT and adapting the caller code would be probably more correct.
comment:2 by , 10 years ago
But (if I'm not mistaken) function VMR3ReqCallU doesn't return pReq (needed for waiting) when passing VMREQFLAGS_NO_WAIT flag?
by , 10 years ago
| Attachment: | ConsoleImpl.diff added |
|---|


VBoxHeadless log