Index: /trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp =================================================================== --- /trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp (revision 30019) +++ /trunk/src/VBox/Frontends/VBoxManage/VBoxManageGuestCtrl.cpp (revision 30020) @@ -388,35 +388,56 @@ * Some data left to output? */ - if ( fWaitForStdOut || fWaitForStdErr) { - CHECK_ERROR_BREAK(guest, GetProcessOutput(uPID, 0 /* aFlags */, - u32TimeoutMS, _64K, ComSafeArrayAsOutParam(aOutputData))); - cbOutputData = aOutputData.size(); - if (cbOutputData > 0) + + rc = guest->GetProcessOutput(uPID, 0 /* aFlags */, + u32TimeoutMS, _64K, ComSafeArrayAsOutParam(aOutputData)); + if (FAILED(rc)) { - /* aOutputData has a platform dependent line ending, standardize on - * Unix style, as RTStrmWrite does the LF -> CR/LF replacement on - * Windows. Otherwise we end up with CR/CR/LF on Windows. */ - ULONG cbOutputDataPrint = cbOutputData; - for (BYTE *s = aOutputData.raw(), *d = s; - s - aOutputData.raw() < (ssize_t)cbOutputData; - s++, d++) + /* If we got a VBOX_E_IPRT error we handle the error in a more gentle way + * because it contains more accurate info about what went wrong. */ + ErrorInfo info(guest); + if (info.isFullAvailable()) { - if (*s == '\r') + if (rc == VBOX_E_IPRT_ERROR) { - /* skip over CR, adjust destination */ - d--; - cbOutputDataPrint--; + RTPrintf("%ls.\n", info.getText().raw()); } - else if (s != d) - *d = *s; + else + { + RTPrintf("ERROR: %ls (%Rhrc).\n", info.getText().raw(), info.getResultCode()); + } } - RTStrmWrite(g_pStdOut, aOutputData.raw(), cbOutputDataPrint); + cbOutputData = 0; } - } - - if (cbOutputData <= 0) + else + { + cbOutputData = aOutputData.size(); + if (cbOutputData > 0) + { + /* aOutputData has a platform dependent line ending, standardize on + * Unix style, as RTStrmWrite does the LF -> CR/LF replacement on + * Windows. Otherwise we end up with CR/CR/LF on Windows. */ + ULONG cbOutputDataPrint = cbOutputData; + for (BYTE *s = aOutputData.raw(), *d = s; + s - aOutputData.raw() < (ssize_t)cbOutputData; + s++, d++) + { + if (*s == '\r') + { + /* skip over CR, adjust destination */ + d--; + cbOutputDataPrint--; + } + else if (s != d) + *d = *s; + } + RTStrmWrite(g_pStdOut, aOutputData.raw(), cbOutputDataPrint); + } + } + } + + if (cbOutputData <= 0) /* No more output data left? */ { if (fCompleted) @@ -431,5 +452,5 @@ } - /* process async cancelation */ + /* Process async cancelation */ if (g_fExecCanceled && !fCanceledAlready) { @@ -441,5 +462,5 @@ } - /* progress canceled by Main API? */ + /* Progress canceled by Main API? */ if ( SUCCEEDED(progress->COMGETTER(Canceled(&fCanceled))) && fCanceled) @@ -447,9 +468,7 @@ break; } - - progress->WaitForCompletion(100); - } - - /* undo signal handling */ + } + + /* Undo signal handling */ if (fCancelable) { @@ -465,5 +484,6 @@ RTPrintf("Process execution canceled!\n"); } - else if (fCompleted) + else if ( fCompleted + && SUCCEEDED(rc)) { LONG iRc = false; @@ -474,16 +494,31 @@ rc = progress->COMGETTER(ErrorInfo)(execError.asOutParam()); com::ErrorInfo info (execError); - RTPrintf("\n\nProcess error details:\n"); - GluePrintErrorInfo(info); - RTPrintf("\n"); - } - if (fVerbose) + if (SUCCEEDED(rc) && info.isFullAvailable()) + { + /* If we got a VBOX_E_IPRT error we handle the error in a more gentle way + * because it contains more accurate info about what went wrong. */ + if (info.getResultCode() == VBOX_E_IPRT_ERROR) + { + RTPrintf("%ls.\n", info.getText().raw()); + } + else + { + RTPrintf("\n\nProcess error details:\n"); + GluePrintErrorInfo(info); + RTPrintf("\n"); + } + } + else + AssertMsgFailed(("Full error description is missing!\n")); + } + else if (fVerbose) { ULONG uRetStatus, uRetExitCode, uRetFlags; - CHECK_ERROR_BREAK(guest, GetProcessStatus(uPID, &uRetExitCode, &uRetFlags, &uRetStatus)); - RTPrintf("Exit code=%u (Status=%u [%s], Flags=%u)\n", uRetExitCode, uRetStatus, getStatus(uRetStatus), uRetFlags); - } - } - else /* If neither canceled nor completed we got a hard abort (shouldn't happen). */ + rc = guest->GetProcessStatus(uPID, &uRetExitCode, &uRetFlags, &uRetStatus); + if (SUCCEEDED(rc)) + RTPrintf("Exit code=%u (Status=%u [%s], Flags=%u)\n", uRetExitCode, uRetStatus, getStatus(uRetStatus), uRetFlags); + } + } + else { if (fVerbose) Index: /trunk/src/VBox/Main/GuestImpl.cpp =================================================================== --- /trunk/src/VBox/Main/GuestImpl.cpp (revision 30019) +++ /trunk/src/VBox/Main/GuestImpl.cpp (revision 30020) @@ -114,18 +114,21 @@ #ifdef VBOX_WITH_GUEST_CONTROL - /* - * Cleanup must be done *before* AutoUninitSpan to cancel all - * all outstanding waits in API functions (which hold AutoCaller - * ref counts). - */ - AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); - - /* Clean up callback data. */ - CallbackListIter it; - for (it = mCallbackList.begin(); it != mCallbackList.end(); it++) - destroyCtrlCallbackContext(it); - - /* Clear process list. */ - mGuestProcessList.clear(); + /* Scope write lock as much as possible. */ + { + /* + * Cleanup must be done *before* AutoUninitSpan to cancel all + * all outstanding waits in API functions (which hold AutoCaller + * ref counts). + */ + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + + /* Clean up callback data. */ + CallbackListIter it; + for (it = mCallbackList.begin(); it != mCallbackList.end(); it++) + destroyCtrlCallbackContext(it); + + /* Clear process list. */ + mGuestProcessList.clear(); + } #endif @@ -493,6 +496,8 @@ * changes to the object state. */ +#ifdef DEBUG_andy LogFlowFunc(("pvExtension = %p, u32Function = %d, pvParms = %p, cbParms = %d\n", pvExtension, u32Function, pvParms, cbParms)); +#endif ComObjPtr pGuest = reinterpret_cast(pvExtension); @@ -540,5 +545,4 @@ PCALLBACKDATAEXECSTATUS pData) { - LogFlowFuncEnter(); int rc = VINF_SUCCESS; @@ -560,91 +564,99 @@ /* Was progress canceled before? */ - BOOL fCancelled; - it->pProgress->COMGETTER(Canceled)(&fCancelled); - - /* Do progress handling. */ + BOOL fCanceled; + ComAssert(it->pProgress.isNotNull()); + it->pProgress->COMGETTER(Canceled)(&fCanceled); + Utf8Str errMsg; - HRESULT rc2 = S_OK; - switch (pData->u32Status) - { - case PROC_STS_STARTED: - break; - - case PROC_STS_TEN: /* Terminated normally. */ - if ( !it->pProgress->getCompleted() - && !fCancelled) + if (!fCanceled) + { + /* Do progress handling. */ + switch (pData->u32Status) + { + case PROC_STS_STARTED: + rc = it->pProgress->SetNextOperation(BstrFmt(tr("Waiting for process to exit ...")), 1 /* Weight */); + if (FAILED(rc)) + errMsg = Utf8StrFmt(Guest::tr("Cannot enter waiting for process exit stage! rc=%u"), + rc); + break; + + case PROC_STS_TEN: /* Terminated normally. */ + it->pProgress->notifyComplete(S_OK); + LogFlowFunc(("Proccess (context ID=%u, status=%u) terminated successfully\n", + pData->hdr.u32ContextID, pData->u32Status)); + break; + + case PROC_STS_TEA: /* Terminated abnormally. */ + errMsg = Utf8StrFmt(Guest::tr("Process terminated abnormally with status '%u'"), + pCBData->u32Flags); + break; + + case PROC_STS_TES: /* Terminated through signal. */ + errMsg = Utf8StrFmt(Guest::tr("Process terminated via signal with status '%u'"), + pCBData->u32Flags); + break; + + case PROC_STS_TOK: + errMsg = Utf8StrFmt(Guest::tr("Process timed out and was killed")); + break; + + case PROC_STS_TOA: + errMsg = Utf8StrFmt(Guest::tr("Process timed out and could not be killed")); + break; + + case PROC_STS_DWN: + errMsg = Utf8StrFmt(Guest::tr("Process exited because system is shutting down")); + break; + + case PROC_STS_ERROR: + errMsg = Utf8StrFmt(Guest::tr("Process execution failed with rc=%Rrc"), pCBData->u32Flags); + break; + + default: + break; + } + + /* Handle process list. */ + /** @todo What happens on/deal with PID reuse? */ + /** @todo How to deal with multiple updates at once? */ + if (pCBData->u32PID > 0) + { + GuestProcessIter it_proc = getProcessByPID(pCBData->u32PID); + if (it_proc == mGuestProcessList.end()) { - it->pProgress->notifyComplete(S_OK); + /* Not found, add to list. */ + GuestProcess p; + p.mPID = pCBData->u32PID; + p.mStatus = pCBData->u32Status; + p.mExitCode = pCBData->u32Flags; /* Contains exit code. */ + p.mFlags = 0; + + mGuestProcessList.push_back(p); } - break; - - case PROC_STS_TEA: /* Terminated abnormally. */ - errMsg = Utf8StrFmt(Guest::tr("Process terminated abnormally with status '%u'"), - pCBData->u32Flags); - break; - - case PROC_STS_TES: /* Terminated through signal. */ - errMsg = Utf8StrFmt(Guest::tr("Process terminated via signal with status '%u'"), - pCBData->u32Flags); - break; - - case PROC_STS_TOK: - errMsg = Utf8StrFmt(Guest::tr("Process timed out and was killed")); - break; - - case PROC_STS_TOA: - errMsg = Utf8StrFmt(Guest::tr("Process timed out and could not be killed")); - break; - - case PROC_STS_DWN: - errMsg = Utf8StrFmt(Guest::tr("Process exited because system is shutting down")); - break; - - default: - break; - } - - /* Handle process list. */ - /** @todo What happens on/deal with PID reuse? */ - /** @todo How to deal with multiple updates at once? */ - GuestProcessIter it_proc = getProcessByPID(pCBData->u32PID); - if (it_proc == mGuestProcessList.end()) - { - /* Not found, add to list. */ - GuestProcess p; - p.mPID = pCBData->u32PID; - p.mStatus = pCBData->u32Status; - p.mExitCode = pCBData->u32Flags; /* Contains exit code. */ - p.mFlags = 0; - - mGuestProcessList.push_back(p); - } - else /* Update list. */ - { - it_proc->mStatus = pCBData->u32Status; - it_proc->mExitCode = pCBData->u32Flags; /* Contains exit code. */ - it_proc->mFlags = 0; - } - - if ( !it->pProgress.isNull() - && errMsg.length()) - { - if ( !it->pProgress->getCompleted() - && !fCancelled) + else /* Update list. */ + { + it_proc->mStatus = pCBData->u32Status; + it_proc->mExitCode = pCBData->u32Flags; /* Contains exit code. */ + it_proc->mFlags = 0; + } + } + } + else + errMsg = Utf8StrFmt(Guest::tr("Process execution canceled")); + + if (!it->pProgress->getCompleted()) + { + if ( errMsg.length() + || fCanceled) /* If cancelled we have to report E_FAIL! */ { - it->pProgress->notifyComplete(E_FAIL /** @todo Find a better rc! */, COM_IIDOF(IGuest), + it->pProgress->notifyComplete(VBOX_E_IPRT_ERROR, COM_IIDOF(IGuest), (CBSTR)Guest::getComponentName(), errMsg.c_str()); - LogFlowFunc(("Callback (context ID=%u, status=%u) progress marked as completed\n", - pData->hdr.u32ContextID, pData->u32Status)); + LogFlowFunc(("Process (context ID=%u, status=%u) reported error: %s\n", + pData->hdr.u32ContextID, pData->u32Status, errMsg.c_str())); } - else - LogFlowFunc(("Callback (context ID=%u, status=%u) progress already marked as completed\n", - pData->hdr.u32ContextID, pData->u32Status)); - } - ASMAtomicWriteBool(&it->fCalled, true); + } } else LogFlowFunc(("Unexpected callback (magic=%u, context ID=%u) arrived\n", pData->hdr.u32Magic, pData->hdr.u32ContextID)); - LogFlowFuncLeave(); return rc; } @@ -654,5 +666,4 @@ PCALLBACKDATAEXECOUT pData) { - LogFlowFuncEnter(); int rc = VINF_SUCCESS; @@ -663,5 +674,4 @@ if (it != mCallbackList.end()) { - Assert(!it->fCalled); PCALLBACKDATAEXECOUT pCBData = (CALLBACKDATAEXECOUT*)it->pvData; AssertPtr(pCBData); @@ -687,9 +697,18 @@ pCBData->cbData = 0; } - ASMAtomicWriteBool(&it->fCalled, true); + + /* Was progress canceled before? */ + BOOL fCanceled; + ComAssert(it->pProgress.isNotNull()); + it->pProgress->COMGETTER(Canceled)(&fCanceled); + + if (!fCanceled) + it->pProgress->notifyComplete(S_OK); + else + it->pProgress->notifyComplete(VBOX_E_IPRT_ERROR, COM_IIDOF(IGuest), + (CBSTR)Guest::getComponentName(), Guest::tr("The output operation was cancelled")); } else LogFlowFunc(("Unexpected callback (magic=%u, context ID=%u) arrived\n", pData->hdr.u32Magic, pData->hdr.u32ContextID)); - LogFlowFuncLeave(); return rc; } @@ -698,5 +717,4 @@ PCALLBACKDATACLIENTDISCONNECTED pData) { - LogFlowFuncEnter(); int rc = VINF_SUCCESS; @@ -708,8 +726,9 @@ { if (it->mContextID == pData->hdr.u32ContextID) + { + LogFlowFunc(("Client with context ID=%u disconnected\n", it->mContextID)); destroyCtrlCallbackContext(it); - } - - LogFlowFuncLeave(); + } + } return rc; } @@ -746,7 +765,8 @@ void Guest::destroyCtrlCallbackContext(Guest::CallbackListIter it) { - LogFlowFuncEnter(); if (it->pvData) { + LogFlowFunc(("Destroying callback with context ID=%u ...\n", it->mContextID)); + RTMemFree(it->pvData); it->pvData = NULL; @@ -755,10 +775,21 @@ /* Notify outstanding waits for progress ... */ - if (it->pProgress && !it->pProgress.isNull()) - { - /* Only cancel if not canceled before! */ - BOOL fCancelled; - if (SUCCEEDED(it->pProgress->COMGETTER(Canceled)(&fCancelled)) && !fCancelled) - it->pProgress->Cancel(); + if (it->pProgress && it->pProgress.isNotNull()) + { + LogFlowFunc(("Handling progress of context ID=%u ...\n", it->mContextID)); + + BOOL fCompleted; + it->pProgress->COMGETTER(Completed)(&fCompleted); + if (!fCompleted) + { + /* Only cancel if not canceled before! */ + BOOL fCanceled; + if (SUCCEEDED(it->pProgress->COMGETTER(Canceled)(&fCanceled)) && !fCanceled) + it->pProgress->Cancel(); + + /* To get waitForCompletion notified we have to notify it if necessary. */ + it->pProgress->notifyComplete(VBOX_E_IPRT_ERROR, COM_IIDOF(IGuest), + (CBSTR)Guest::getComponentName(), Guest::tr("The operation was canceled during shutdown")); + } /* * Do *not NULL pProgress here, because waiting function like executeProcess() @@ -766,5 +797,4 @@ */ } - LogFlowFuncLeave(); } @@ -774,5 +804,5 @@ uint32_t Guest::addCtrlCallbackContext(eVBoxGuestCtrlCallbackType enmType, void *pvData, uint32_t cbData, Progress *pProgress) { - LogFlowFuncEnter(); + AssertPtr(pProgress); uint32_t uNewContext = ASMAtomicIncU32(&mNextContextID); if (uNewContext == UINT32_MAX) @@ -783,5 +813,4 @@ context.mContextID = uNewContext; context.mType = enmType; - context.fCalled = false; context.pvData = pvData; context.cbData = cbData; @@ -807,6 +836,4 @@ } #endif - - LogFlowFuncLeave(); return uNewContext; } @@ -844,5 +871,11 @@ { /* - * Create progress object. + * Create progress object. Note that this is a multi operation + * object to perform the following steps: + * - Operation 1 (0): Create/start process. + * - Operation 2 (1): Wait for process to exit. + * If this progress completed successfully (S_OK), the process + * started and exited normally. In any other case an error/exception + * occured. */ ComObjPtr progress; @@ -852,5 +885,7 @@ rc = progress->init(static_cast(this), BstrFmt(tr("Executing process")), - TRUE); + TRUE, + 2, /* Number of operations. */ + BstrFmt(tr("Starting process ..."))); /* Description of first stage. */ } if (FAILED(rc)) return rc; @@ -919,4 +954,5 @@ PCALLBACKDATAEXECSTATUS pData = (PCALLBACKDATAEXECSTATUS)RTMemAlloc(sizeof(CALLBACKDATAEXECSTATUS)); AssertReturn(pData, VBOX_E_IPRT_ERROR); + RT_ZERO(*pData); uContextID = addCtrlCallbackContext(VBOXGUESTCTRLCALLBACKTYPE_EXEC_START, pData, sizeof(CALLBACKDATAEXECSTATUS), progress); @@ -974,81 +1010,68 @@ if (it != mCallbackList.end()) { - uint64_t u64Started = RTTimeMilliTS(); - while (!it->fCalled) + ComAssert(it->pProgress.isNotNull()); + + /* + * Wait for the first stage (=0) to complete (that is starting the process). + */ + PCALLBACKDATAEXECSTATUS pData = NULL; + rc = it->pProgress->WaitForOperationCompletion(0, aTimeoutMS); + if (SUCCEEDED(rc)) { - /* Check for timeout. */ - unsigned cMsWait; - if (aTimeoutMS == RT_INDEFINITE_WAIT) - cMsWait = 10; - else + /* Was the operation canceled by one of the parties? */ + rc = it->pProgress->COMGETTER(Canceled)(&fCanceled); + if (FAILED(rc)) throw rc; + + if (!fCanceled) { - uint64_t cMsElapsed = RTTimeMilliTS() - u64Started; - if (cMsElapsed >= aTimeoutMS) - break; /* Timed out. */ - cMsWait = RT_MIN(10, aTimeoutMS - (uint32_t)cMsElapsed); + AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); + + pData = (PCALLBACKDATAEXECSTATUS)it->pvData; + Assert(it->cbData == sizeof(CALLBACKDATAEXECSTATUS)); + AssertPtr(pData); + + /* Did we get some status? */ + switch (pData->u32Status) + { + case PROC_STS_STARTED: + /* Process is (still) running; get PID. */ + *aPID = pData->u32PID; + break; + + /* In any other case the process either already + * terminated or something else went wrong, so no PID ... */ + case PROC_STS_TEN: /* Terminated normally. */ + case PROC_STS_TEA: /* Terminated abnormally. */ + case PROC_STS_TES: /* Terminated through signal. */ + case PROC_STS_TOK: + case PROC_STS_TOA: + case PROC_STS_DWN: + /* + * Process (already) ended, but we want to get the + * PID anyway to retrieve the output in a later call. + */ + *aPID = pData->u32PID; + break; + + case PROC_STS_ERROR: + vrc = pData->u32Flags; /* u32Flags member contains IPRT error code. */ + break; + + case PROC_STS_UNDEFINED: + vrc = VERR_TIMEOUT; /* Operation did not complete within time. */ + break; + + default: + vrc = VERR_INVALID_PARAMETER; /* Unknown status, should never happen! */ + break; + } } - - /* Check for manual stop. */ - if (!it->pProgress.isNull()) - { - rc = it->pProgress->COMGETTER(Canceled)(&fCanceled); - if (FAILED(rc)) throw rc; - if (fCanceled) - break; /* HGCM/guest wants to abort because of status change. */ - - } - /// @todo r=bird: two operation progress object and wait first operation. - /// IProgress::WaitForOperationCompletion. - RTThreadSleep(cMsWait); + else /* Operation was canceled. */ + vrc = VERR_CANCELLED; } - } - - /* Was the whole thing canceled? */ - if (!fCanceled) - { - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - - PCALLBACKDATAEXECSTATUS pData = (PCALLBACKDATAEXECSTATUS)it->pvData; - Assert(it->cbData == sizeof(CALLBACKDATAEXECSTATUS)); - AssertPtr(pData); - - if (it->fCalled) - { - /* Did we get some status? */ - switch (pData->u32Status) - { - case PROC_STS_STARTED: - /* Process is (still) running; get PID. */ - *aPID = pData->u32PID; - break; - - /* In any other case the process either already - * terminated or something else went wrong, so no PID ... */ - case PROC_STS_TEN: /* Terminated normally. */ - case PROC_STS_TEA: /* Terminated abnormally. */ - case PROC_STS_TES: /* Terminated through signal. */ - case PROC_STS_TOK: - case PROC_STS_TOA: - case PROC_STS_DWN: - /* - * Process (already) ended, but we want to get the - * PID anyway to retrieve the output in a later call. - */ - *aPID = pData->u32PID; - break; - - case PROC_STS_ERROR: - vrc = pData->u32Flags; /* u32Flags member contains IPRT error code. */ - break; - - default: - vrc = VERR_INVALID_PARAMETER; /* Unknown status, should never happen! */ - break; - } - } - else /* If callback not called within time ... well, that's a timeout! */ + else /* Operation did not complete within time. */ vrc = VERR_TIMEOUT; - - /* + + /* * Do *not* remove the callback yet - we might wait with the IProgress object on something * else (like end of process) ... @@ -1081,4 +1104,9 @@ tr("The guest did not respond within time (%ums)"), aTimeoutMS); } + else if (vrc == VERR_CANCELLED) + { + rc = setError(VBOX_E_IPRT_ERROR, + tr("The execution operation was canceled")); + } else if (vrc == VERR_PERMISSION_DENIED) { @@ -1088,5 +1116,5 @@ else { - if (pData->u32Status == PROC_STS_ERROR) + if (pData && pData->u32Status == PROC_STS_ERROR) rc = setError(VBOX_E_IPRT_ERROR, tr("Process could not be started: %Rrc"), pData->u32Flags); @@ -1094,5 +1122,5 @@ rc = setError(E_UNEXPECTED, tr("The service call failed with error %Rrc"), vrc); - } + } } else /* Execution went fine. */ @@ -1102,9 +1130,6 @@ } } - else /* Operation was canceled. */ - { - rc = setError(VBOX_E_IPRT_ERROR, - tr("The operation was canceled")); - } + else /* Callback context not found; should never happen! */ + AssertMsg(it != mCallbackList.end(), ("Callback context with ID %u not found!", uContextID)); } else /* HGCM related error codes .*/ @@ -1163,6 +1188,7 @@ /* * Create progress object. - * Note that we need at least a local progress object here in order - * to get notified when someone cancels the operation. + * This progress object, compared to the one in executeProgress() above, + * is only local and is used to determine whether the operation finished + * or got cancelled. */ ComObjPtr progress; @@ -1183,4 +1209,5 @@ PCALLBACKDATAEXECOUT pData = (CALLBACKDATAEXECOUT*)RTMemAlloc(sizeof(CALLBACKDATAEXECOUT)); AssertReturn(pData, VBOX_E_IPRT_ERROR); + RT_ZERO(*pData); uint32_t uContextID = addCtrlCallbackContext(VBOXGUESTCTRLCALLBACKTYPE_EXEC_OUTPUT, pData, sizeof(CALLBACKDATAEXECOUT), progress); @@ -1232,34 +1259,19 @@ if (it != mCallbackList.end()) { - uint64_t u64Started = RTTimeMilliTS(); - while (!it->fCalled) - { - /* Check for timeout. */ - unsigned cMsWait; - if (aTimeoutMS == RT_INDEFINITE_WAIT) - cMsWait = 10; - else - { - uint64_t cMsElapsed = RTTimeMilliTS() - u64Started; - if (cMsElapsed >= aTimeoutMS) - break; /* Timed out. */ - cMsWait = RT_MIN(10, aTimeoutMS - (uint32_t)cMsElapsed); - } - - /* Check for manual stop. */ - if (!it->pProgress.isNull()) - { - rc = it->pProgress->COMGETTER(Canceled)(&fCanceled); - if (FAILED(rc)) throw rc; - if (fCanceled) - break; /* Client wants to abort. */ - } - RTThreadSleep(cMsWait); - } - - /* Was the whole thing canceled? */ + ComAssert(it->pProgress.isNotNull()); + + /* Wait until operation completed. */ + rc = it->pProgress->WaitForCompletion(aTimeoutMS); + if (FAILED(rc)) throw rc; + + /* Was the operation canceled by one of the parties? */ + rc = it->pProgress->COMGETTER(Canceled)(&fCanceled); + if (FAILED(rc)) throw rc; + if (!fCanceled) { - if (it->fCalled) + BOOL fCompleted; + if ( SUCCEEDED(it->pProgress->COMGETTER(Completed)(&fCompleted)) + && fCompleted) { AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); @@ -1287,5 +1299,7 @@ } else /* Operation was canceled. */ + { vrc = VERR_CANCELLED; + } if (RT_FAILURE(vrc)) @@ -1294,4 +1308,5 @@ { /* This is not an error we want to report to COM. */ + rc = NO_ERROR; } else if (vrc == VERR_TIMEOUT) @@ -1303,5 +1318,5 @@ { rc = setError(VBOX_E_IPRT_ERROR, - tr("The operation was canceled")); + tr("The output operation was canceled")); } else @@ -1316,4 +1331,7 @@ destroyCtrlCallbackContext(it); } + + /* Remove callback context (not used anymore). */ + mCallbackList.erase(it); } else /* PID lookup failed. */ Index: /trunk/src/VBox/Main/include/GuestImpl.h =================================================================== --- /trunk/src/VBox/Main/include/GuestImpl.h (revision 30019) +++ /trunk/src/VBox/Main/include/GuestImpl.h (revision 30020) @@ -122,10 +122,11 @@ struct CallbackContext { + /** Associated context ID. */ uint32_t mContextID; eVBoxGuestCtrlCallbackType mType; + /** Pointer to user-supplied data. */ void *pvData; + /** Size of user-supplied data. */ uint32_t cbData; - /** Atomic flag whether callback was called. */ - volatile bool fCalled; /** Pointer to user-supplied IProgress. */ ComObjPtr pProgress;