Opened 10 years ago

Closed 9 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


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.

Attachments (2)

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

Download all attachments as: .zip

Change History (9)

by a.urakov, 10 years ago

VBoxHeadless log

comment:1 by Frank Mehnert, 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 a.urakov, 10 years ago

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

comment:3 by Frank Mehnert, 10 years ago

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

comment:4 by a.urakov, 10 years ago

Thank you!

by Frank Mehnert, 10 years ago

Attachment: ConsoleImpl.diff added

comment:5 by Frank Mehnert, 10 years ago

Attached the diff against VBox 4.3.20.

comment:6 by a.urakov, 10 years ago

Thank you again!

comment:7 by Frank Mehnert, 9 years ago

Resolution: fixed
Status: newclosed

Fix is part of VBox 4.3.22.

Note: See TracTickets for help on using tickets.

© 2023 Oracle
ContactPrivacy policyTerms of Use