Ticket #13722 (closed defect: fixed)

Opened 8 years ago

Last modified 7 years ago

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


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!) */

    if (vrc == VERR_TIMEOUT || RT_SUCCESS(vrc))
        vrc = VMR3ReqWait(pReq, RT_INDEFINITE_WAIT);
        if (RT_SUCCESS(vrc))
            vrc = pReq->iStatus;

when EMT thread completes request earlier than VMR3ReqWait (with zero wait time) in VMR3ReqQueue function is called:

         * Notify EMT.
        if (pUVM->pVM)

        // !!! 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);
        if (RT_SUCCESS(vrc))
            vrc = pReq->iStatus;

A similar problem exists in doCPURemove, doCPUAdd, doStorageDeviceAttach, doStorageDeviceDetach etc.


2014-12-30-07-45-23.053-VBoxHeadless-14685.log.bz2 Download (90.2 KB) - added by a.urakov 8 years ago.
VBoxHeadless log
ConsoleImpl.diff Download (7.7 KB) - added by frank 8 years ago.

Change History

Changed 8 years ago by a.urakov

VBoxHeadless log

comment:1 Changed 8 years ago by frank

I think you are right. Though passing VMREQFLAGS_NO_WAIT and adapting the caller code would be probably more correct.

comment:2 Changed 8 years ago by a.urakov

But (if I'm not mistaken) function VMR3ReqCallU doesn't return pReq (needed for waiting) when passing VMREQFLAGS_NO_WAIT flag?

comment:3 Changed 8 years ago by frank

Thanks again, you are right! We will discuss how to fix this.

comment:4 Changed 8 years ago by a.urakov

Thank you!

Changed 8 years ago by frank

comment:5 Changed 8 years ago by frank

Attached the diff against VBox 4.3.20.

comment:6 Changed 8 years ago by a.urakov

Thank you again!

comment:7 Changed 7 years ago by frank

  • Status changed from new to closed
  • Resolution set to fixed

Fix is part of VBox 4.3.22.

Note: See TracTickets for help on using tickets.
ContactPrivacy policyTerms of Use