Index: /trunk/src/VBox/HostServices/GuestControl/service.cpp =================================================================== --- /trunk/src/VBox/HostServices/GuestControl/service.cpp (revision 42271) +++ /trunk/src/VBox/HostServices/GuestControl/service.cpp (revision 42272) @@ -5,5 +5,5 @@ /* - * Copyright (C) 2011 Oracle Corporation + * Copyright (C) 2011-2012 Oracle Corporation * * This file is part of VirtualBox Open Source Edition (OSE), as @@ -707,6 +707,6 @@ /* Remember which client processes which context (for * later reference & cleanup). */ - Assert(curCmd.mContextID > 0); /// @todo r=bird: check if already in the list. + /// @todo Use a map instead of a list? it->mContextList.push_back(curCmd.mContextID); @@ -894,5 +894,4 @@ */ newCmd.mParmBuf.pParms[0].getUInt32(&newCmd.mContextID); - Assert(newCmd.mContextID > 0); } else if (!cParms) Index: /trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h =================================================================== --- /trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h (revision 42271) +++ /trunk/src/VBox/Main/include/GuestCtrlImplPrivate.h (revision 42272) @@ -35,5 +35,4 @@ using namespace guestControl; #endif - /** Maximum number of guest sessions a VM can have. */ @@ -61,6 +60,7 @@ ((uContextID) & 0xffff) - +/** Vector holding a process' CPU affinity. */ typedef std::vector ProcessAffinity; +/** Vector holding process startup arguments. */ typedef std::vector ProcessArguments; @@ -94,4 +94,6 @@ Utf8Str GetMessage(void) { return mMessage; } + + int GetResultCode(void) { return mRC; } eVBoxGuestCtrlCallbackType GetType(void) { return mType; } @@ -107,4 +109,6 @@ /** Was the callback canceled? */ bool fCanceled; + /** Did the callback complete? */ + bool fCompleted; /** Pointer to user-supplied data. */ void *pvData; @@ -124,11 +128,6 @@ * Simple structure mantaining guest credentials. */ -class GuestCredentials -{ -public: - - -public: - +struct GuestCredentials +{ Utf8Str mUser; Utf8Str mPassword; Index: /trunk/src/VBox/Main/include/GuestProcessImpl.h =================================================================== --- /trunk/src/VBox/Main/include/GuestProcessImpl.h (revision 42271) +++ /trunk/src/VBox/Main/include/GuestProcessImpl.h (revision 42272) @@ -71,5 +71,5 @@ int callbackAdd(GuestCtrlCallback *pCallback, ULONG *puContextID); int callbackDispatcher(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData); - bool callbackExists(ULONG uContextID); + inline bool callbackExists(ULONG uContextID); bool isReady(void); ULONG getPID(void) { return mData.mPID; } @@ -81,5 +81,6 @@ int readData(ULONG uHandle, ULONG uSize, ULONG uTimeoutMS, BYTE *pbData, size_t cbData); int sendCommand(uint32_t uFunction, uint32_t uParms, PVBOXHGCMSVCPARM paParms); - int startProcess(void); + int signalWaiters(int rc, const Utf8Str strMessage = ""); + int startProcess(int *pRC = NULL, Utf8Str *pstrMessage = NULL); static DECLCALLBACK(int) startProcessThread(RTTHREAD Thread, void *pvUser); int terminateProcess(void); Index: /trunk/src/VBox/Main/include/GuestSessionImpl.h =================================================================== --- /trunk/src/VBox/Main/include/GuestSessionImpl.h (revision 42271) +++ /trunk/src/VBox/Main/include/GuestSessionImpl.h (revision 42272) @@ -126,5 +126,5 @@ const GuestEnvironment &getEnvironment(void); int processClose(ComObjPtr pProcess); - int processCreateExInteral(GuestProcessInfo &aProcInfo, IGuestProcess **aProcess); + int processCreateExInteral(const GuestProcessInfo &aProcInfo, IGuestProcess **aProcess); inline bool processExists(ULONG uProcessID, ComObjPtr *pProcess); inline int processGetByPID(ULONG uPID, ComObjPtr *pProcess); Index: /trunk/src/VBox/Main/src-client/GuestCtrlImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestCtrlImpl.cpp (revision 42271) +++ /trunk/src/VBox/Main/src-client/GuestCtrlImpl.cpp (revision 42272) @@ -700,4 +700,10 @@ AssertPtr(pHeader); +#ifdef DEBUG + LogFlowFunc(("uSession=%RU32, uProcess=%RU32, uCount=%RU32\n", + VBOX_GUESTCTRL_CONTEXTID_GET_SESSION(pHeader->u32ContextID), + VBOX_GUESTCTRL_CONTEXTID_GET_PROCESS(pHeader->u32ContextID), + VBOX_GUESTCTRL_CONTEXTID_GET_COUNT(pHeader->u32ContextID))); +#endif int rc = pGuest->dispatchToSession(pHeader->u32ContextID, u32Function, pvParms, cbParms); @@ -2675,4 +2681,6 @@ int Guest::dispatchToSession(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData) { + LogFlowFuncEnter(); + AssertPtrReturn(pvData, VERR_INVALID_POINTER); AssertReturn(cbData, VERR_INVALID_PARAMETER); @@ -2680,4 +2688,5 @@ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); + int rc; GuestSessions::const_iterator itSession = mData.mGuestSessions.find(VBOX_GUESTCTRL_CONTEXTID_GET_SESSION(uContextID)); @@ -2688,7 +2697,11 @@ alock.release(); - return pSession->dispatchToProcess(uContextID, uFunction, pvData, cbData); - } - return VERR_NOT_FOUND; + rc = pSession->dispatchToProcess(uContextID, uFunction, pvData, cbData); + } + else + rc = VERR_NOT_FOUND; + + LogFlowFuncLeaveRC(rc); + return rc; } @@ -2718,4 +2731,7 @@ int rc = VERR_MAX_PROCS_REACHED; + if (mData.mGuestSessions.size() >= VBOX_GUESTCTRL_MAX_SESSIONS) + return rc; + try { @@ -2726,10 +2742,11 @@ for (;;) { - /* Is the context ID already used? Try next ID ... */ - if (!sessionExists(++uNewSessionID)) + /* Is the context ID already used? */ + if (!sessionExists(uNewSessionID)) { rc = VINF_SUCCESS; break; } + uNewSessionID++; if (++uTries == UINT32_MAX) @@ -2763,6 +2780,4 @@ inline bool Guest::sessionExists(uint32_t uSessionID) { - AssertReturn(uSessionID, false); - GuestSessions::const_iterator itSessions = mData.mGuestSessions.find(uSessionID); return (itSessions == mData.mGuestSessions.end()) ? false : true; Index: /trunk/src/VBox/Main/src-client/GuestCtrlPrivate.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestCtrlPrivate.cpp (revision 42271) +++ /trunk/src/VBox/Main/src-client/GuestCtrlPrivate.cpp (revision 42272) @@ -37,4 +37,5 @@ uFlags(0), fCanceled(false), + fCompleted(false), pvData(NULL), cbData(0), @@ -47,4 +48,5 @@ uFlags(0), fCanceled(false), + fCompleted(false), pvData(NULL), cbData(0), @@ -62,7 +64,10 @@ int GuestCtrlCallback::Cancel(void) { + if (ASMAtomicReadBool(&fCompleted)) + return VINF_SUCCESS; if (!ASMAtomicReadBool(&fCanceled)) { - int rc = RTSemEventSignal(hEventSem); + int rc = hEventSem != NIL_RTSEMEVENT + ? RTSemEventSignal(hEventSem) : VINF_SUCCESS; if (RT_SUCCESS(rc)) ASMAtomicXchgBool(&fCanceled, true); @@ -128,4 +133,7 @@ void GuestCtrlCallback::Destroy(void) { + int rc = Cancel(); + AssertRC(rc); + mType = VBOXGUESTCTRLCALLBACKTYPE_UNKNOWN; if (pvData) @@ -152,12 +160,5 @@ if (!uTimeoutMS) msInterval = RT_INDEFINITE_WAIT; - int rc = RTSemEventWait(hEventSem, msInterval); - if (RT_SUCCESS(rc)) - { - /* Assign overall callback result. */ - rc = mRC; - } - - return rc; + return RTSemEventWait(hEventSem, msInterval); } Index: /trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp (revision 42271) +++ /trunk/src/VBox/Main/src-client/GuestProcessImpl.cpp (revision 42272) @@ -68,5 +68,5 @@ HRESULT GuestProcess::FinalConstruct(void) { - LogFlowThisFunc(("\n")); + LogFlowFuncEnter(); mData.mExitCode = 0; @@ -81,5 +81,7 @@ mData.mWaitEvent = NIL_RTSEMEVENT; - return BaseFinalConstruct(); + HRESULT hr = BaseFinalConstruct(); + LogFlowFuncLeaveRC(hr); + return hr; } @@ -108,4 +110,6 @@ mData.mParent = aSession; mData.mProcessID = aProcessID; + mData.mProcess = aProcInfo; + mData.mStatus = ProcessStatus_Starting; /* Everything else will be set by the actual starting routine. */ @@ -362,9 +366,13 @@ int GuestProcess::callbackDispatcher(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData) { +/* LogFlowFunc(("uPID=%RU32, uContextID=%RU32, uFunction=%RU32, pvData=%p, cbData=%z\n", + mData.mPID, uContextID, uFunction, pvData, cbData));*/ + AssertPtrReturn(pvData, VERR_INVALID_POINTER); AssertReturn(cbData, VERR_INVALID_PARAMETER); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - + AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); + + int rc; GuestCtrlCallbacks::const_iterator it = mData.mCallbacks.find(VBOX_GUESTCTRL_CONTEXTID_GET_COUNT(uContextID)); @@ -373,6 +381,4 @@ GuestCtrlCallback *pCallback = it->second; AssertPtr(pCallback); - - int rc; switch (uFunction) @@ -429,15 +435,14 @@ break; } - - return rc; - } - - return VERR_NOT_FOUND; -} - -bool GuestProcess::callbackExists(ULONG uContextID) -{ - AssertReturn(uContextID, false); - + } + else + rc = VERR_NOT_FOUND; + + //LogFlowFuncLeaveRC(rc); + return rc; +} + +inline bool GuestProcess::callbackExists(ULONG uContextID) +{ GuestCtrlCallbacks::const_iterator it = mData.mCallbacks.find(uContextID); return (it == mData.mCallbacks.end()) ? false : true; @@ -459,11 +464,11 @@ int GuestProcess::onGuestDisconnected(GuestCtrlCallback *pCallback, PCALLBACKDATACLIENTDISCONNECTED pData) { - LogFlowFunc(("uPID=%RU32, pCallback=%p, pData=%p", mData.mPID, pCallback, pData)); + LogFlowFunc(("uPID=%RU32, pCallback=%p, pData=%p\n", mData.mPID, pCallback, pData)); /* First, signal callback in every case. */ pCallback->Signal(); - Assert(mData.mWaitEvent != NIL_RTSEMEVENT); - int rc = RTSemEventSignal(mData.mWaitEvent); + /* Signal in any case. */ + int rc = signalWaiters(VERR_CANCELLED, tr("The guest got disconnected")); AssertRC(rc); @@ -486,9 +491,5 @@ /* Then do the WaitFor signalling stuff. */ if (mData.mWaitFlags & ProcessWaitForFlag_StdIn) - { - Assert(mData.mWaitEvent != NIL_RTSEMEVENT); - rc = RTSemEventSignal(mData.mWaitEvent); - AssertRC(rc); - } + rc = signalWaiters(VINF_SUCCESS); LogFlowFuncLeaveRC(rc); @@ -504,9 +505,10 @@ /* Get data from the callback payload. */ - mData.mStatus = (ProcessStatus_T)pData->u32Status; if (mData.mPID) Assert(mData.mPID == pData->u32PID); - Utf8Str strCallbackMsg; + Utf8Str callbackMsg; + int callbackRC = VINF_SUCCESS; + bool fSignal = false; switch (pData->u32Status) @@ -515,18 +517,21 @@ fSignal = ( (mData.mWaitFlags & ProcessWaitForFlag_Start) || (mData.mWaitFlags & ProcessWaitForFlag_Status)); + + mData.mStatus = ProcessStatus_Started; mData.mPID = pData->u32PID; mData.mStarted = true; - rc = pCallback->Signal(VINF_SUCCESS, - Utf8StrFmt(tr("Guest process \"%s\" was started (PID %RU32)"), - mData.mProcess.mCommand.c_str(), mData.mPID)); + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" was started (PID %RU32)"), + mData.mProcess.mCommand.c_str(), mData.mPID); + LogRel((callbackMsg.c_str())); break; case PROC_STS_TEN: { - Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated normally (exit code: %RU32)"), - pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags); - LogRel((strErrDetail.c_str())); - rc = pCallback->Signal(VINF_SUCCESS, strErrDetail); + mData.mStatus = ProcessStatus_TerminatedNormally; + + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated normally (exit code: %d)"), + mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags); + LogRel((callbackMsg.c_str())); break; } @@ -534,8 +539,10 @@ case PROC_STS_TES: { - Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated through signal (exit code: %RU32)"), - pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags); - LogRel((strErrDetail.c_str())); - rc = pCallback->Signal(VERR_GENERAL_FAILURE /** @todo */, strErrDetail); + mData.mStatus = ProcessStatus_TerminatedSignal; + + callbackRC = VERR_INTERRUPTED; + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated through signal (exit code: %d)"), + mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags); + LogRel((callbackMsg.c_str())); break; } @@ -543,8 +550,10 @@ case PROC_STS_TEA: { - Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated abnormally (exit code: %RU32)"), - pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags); - LogRel((strErrDetail.c_str())); - rc = pCallback->Signal(VERR_BROKEN_PIPE, strErrDetail); + mData.mStatus = ProcessStatus_TerminatedAbnormally; + + callbackRC = VERR_BROKEN_PIPE; + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) terminated abnormally (exit code: %d)"), + mData.mProcess.mCommand.c_str(), mData.mPID, pData->u32Flags); + LogRel((callbackMsg.c_str())); break; } @@ -552,8 +561,10 @@ case PROC_STS_TOK: { - Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and was killed"), - pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID); - LogRel((strErrDetail.c_str())); - rc = pCallback->Signal(VERR_TIMEOUT, strErrDetail); + mData.mStatus = ProcessStatus_TimedOutKilled; + + callbackRC = VERR_TIMEOUT; + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and was killed"), + mData.mProcess.mCommand.c_str(), mData.mPID); + LogRel((callbackMsg.c_str())); break; } @@ -561,8 +572,10 @@ case PROC_STS_TOA: { - Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and could not be killed\n"), - pData->u32PID, mData.mProcess.mCommand.c_str(), mData.mPID); - LogRel((strErrDetail.c_str())); - rc = pCallback->Signal(VERR_TIMEOUT, strErrDetail); + mData.mStatus = ProcessStatus_TimedOutAbnormally; + + callbackRC = VERR_TIMEOUT; + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) timed out and could not be killed\n"), + mData.mProcess.mCommand.c_str(), mData.mPID); + LogRel((callbackMsg.c_str())); break; } @@ -570,7 +583,6 @@ case PROC_STS_DWN: { - Utf8Str strErrDetail = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) was killed because system is shutting down\n"), - mData.mProcess.mCommand.c_str(), mData.mPID); - LogRel((strErrDetail.c_str())); + mData.mStatus = ProcessStatus_Down; + /* * If mFlags has CreateProcessFlag_IgnoreOrphanedProcesses set, we don't report an error to @@ -580,6 +592,9 @@ * In this case mFlags contains the actual execution flags reached in via Guest::ExecuteProcess(). */ - rc = pCallback->Signal( mData.mProcess.mFlags & ProcessCreateFlag_IgnoreOrphanedProcesses - ? VINF_SUCCESS : VERR_CANCELLED, strErrDetail); + callbackRC = mData.mProcess.mFlags & ProcessCreateFlag_IgnoreOrphanedProcesses + ? VINF_SUCCESS : VERR_OBJECT_DESTROYED; + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" (PID %u) was killed because system is shutting down\n"), + mData.mProcess.mCommand.c_str(), mData.mPID); + LogRel((callbackMsg.c_str())); break; } @@ -587,58 +602,58 @@ case PROC_STS_ERROR: { + mData.mStatus = ProcessStatus_Error; + + callbackRC = pData->u32Flags; + callbackMsg = Utf8StrFmt(tr("Guest process \"%s\" could not be started: ", mData.mProcess.mCommand.c_str())); + /* Note: It's not required that the process * has been started before. */ - Utf8Str strErrDetail; if (pData->u32PID) { - strErrDetail = Utf8StrFmt(tr("Guest process (PID %RU32) could not be started because of rc=%Rrc"), - pData->u32PID, pData->u32Flags); + callbackMsg += Utf8StrFmt(tr("Error rc=%Rrc occured"), pData->u32Flags); } else { - switch (pData->u32Flags) /* u32Flags member contains the IPRT error code from guest side. */ + switch (callbackRC) /* callbackRC contains the IPRT error code from guest side. */ { case VERR_FILE_NOT_FOUND: /* This is the most likely error. */ - strErrDetail = Utf8StrFmt(tr("The specified file was not found on guest")); + callbackMsg += Utf8StrFmt(tr("The specified file was not found on guest")); break; case VERR_PATH_NOT_FOUND: - strErrDetail = Utf8StrFmt(tr("Could not resolve path to specified file was not found on guest")); + callbackMsg += Utf8StrFmt(tr("Could not resolve path to specified file was not found on guest")); break; case VERR_BAD_EXE_FORMAT: - strErrDetail = Utf8StrFmt(tr("The specified file is not an executable format on guest")); + callbackMsg += Utf8StrFmt(tr("The specified file is not an executable format on guest")); break; case VERR_AUTHENTICATION_FAILURE: - strErrDetail = Utf8StrFmt(tr("The specified user was not able to logon on guest")); + callbackMsg += Utf8StrFmt(tr("The specified user was not able to logon on guest")); break; case VERR_TIMEOUT: - strErrDetail = Utf8StrFmt(tr("The guest did not respond within time")); + callbackMsg += Utf8StrFmt(tr("The guest did not respond within time")); break; case VERR_CANCELLED: - strErrDetail = Utf8StrFmt(tr("The execution operation was canceled")); + callbackMsg += Utf8StrFmt(tr("The execution operation was canceled")); break; case VERR_PERMISSION_DENIED: - strErrDetail = Utf8StrFmt(tr("Invalid user/password credentials")); + callbackMsg += Utf8StrFmt(tr("Invalid user/password credentials")); break; case VERR_MAX_PROCS_REACHED: - strErrDetail = Utf8StrFmt(tr("Guest process could not be started because maximum number of parallel guest processes has been reached")); + callbackMsg += Utf8StrFmt(tr("Maximum number of parallel guest processes has been reached")); break; default: - strErrDetail = Utf8StrFmt(tr("Guest process reported error %Rrc"), - pData->u32Flags); + callbackMsg += Utf8StrFmt(tr("Reported error %Rrc"), callbackRC); break; } - - Assert(!strErrDetail.isEmpty()); - rc = pCallback->Signal(pData->u32Flags /* rc from guest. */, strErrDetail); } - fSignal = mData.mWaitFlags & ProcessWaitForFlag_Status; + + LogRel((callbackMsg.c_str())); break; } @@ -646,9 +661,14 @@ case PROC_STS_UNDEFINED: default: - rc = pCallback->Signal(VERR_NOT_SUPPORTED, - Utf8StrFmt(tr("Got unsupported process status %RU32, skipping"), pData->u32Status)); + + /* Silently skip this request. */ fSignal = true; /* Signal in any case. */ break; } + + /* + * Now do the signalling stuff. + */ + rc = pCallback->Signal(callbackRC, callbackMsg); if (!fSignal) @@ -656,6 +676,5 @@ if (fSignal) { - Assert(mData.mWaitEvent != NIL_RTSEMEVENT); - int rc2 = RTSemEventSignal(mData.mWaitEvent); + int rc2 = signalWaiters(callbackRC, callbackMsg); if (RT_SUCCESS(rc)) rc = rc2; @@ -748,6 +767,35 @@ } -int GuestProcess::startProcess(void) -{ +int GuestProcess::signalWaiters(int rc, const Utf8Str strMessage) +{ + LogFlowFunc(("rc=%Rrc, strMessage=%s, mWaiting=%RTbool, mWaitEvent=%p\n", + rc, strMessage.c_str(), mData.mWaiting, mData.mWaitEvent)); + + /* Note: No locking here -- already done in the callback dispatcher. */ + + /* We have to set the error information here because the waiters are public + * Main clients which rely on it. */ + if (RT_FAILURE(rc)) + { + HRESULT hr = setError(VBOX_E_IPRT_ERROR, strMessage.c_str()); + ComAssertRC(hr); + } + + int rc2 = VINF_SUCCESS; + if ( mData.mWaiting + && mData.mWaitEvent != NIL_RTSEMEVENT) + { + rc2 = RTSemEventSignal(mData.mWaitEvent); + AssertRC(rc2); + } + + LogFlowFuncLeaveRC(rc2); + return rc2; +} + +int GuestProcess::startProcess(int *pRC, Utf8Str *pstrMessage) +{ + /* Parameters are optional. */ + LogFlowFunc(("aCmd=%s, aTimeoutMS=%RU32, fFlags=%x\n", mData.mProcess.mCommand.c_str(), mData.mProcess.mTimeoutMS, mData.mProcess.mFlags)); @@ -757,5 +805,5 @@ int rc; ULONG uContextID; - GuestCtrlCallback *pCallbackStart = (GuestCtrlCallback *)RTMemAlloc(sizeof(GuestCtrlCallback)); + GuestCtrlCallback *pCallbackStart = new GuestCtrlCallback(); if (!pCallbackStart) return VERR_NO_MEMORY; @@ -774,6 +822,4 @@ if (RT_SUCCESS(rc)) { - Assert(uContextID); - ComObjPtr pSession(mData.mParent); Assert(!pSession.isNull()); @@ -844,8 +890,18 @@ */ rc = pCallbackStart->Wait(mData.mProcess.mTimeoutMS); - } - } - - LogFlowFuncLeave(); + if (RT_SUCCESS(rc)) /* Wait was successful, check for supplied information. */ + { + if (pRC) + *pRC = pCallbackStart->GetResultCode(); + if (pstrMessage) + *pstrMessage = pCallbackStart->GetMessage(); + } + } + } + + if (pCallbackStart) + delete pCallbackStart; + + LogFlowFuncLeaveRC(rc); return rc; } @@ -862,6 +918,7 @@ int rc = pProcess->startProcess(); - - LogFlowFuncLeave(); + AssertRC(rc); + + LogFlowFuncLeaveRC(rc); return rc; } @@ -879,7 +936,4 @@ int GuestProcess::waitFor(uint32_t fFlags, ULONG uTimeoutMS, ProcessWaitReason_T *penmReason) { - LogFlowFunc(("fFlags=%x, uTimeoutMS=%RU32, penmReason=%p, mWaitEvent=%p\n", - fFlags, uTimeoutMS, penmReason, &mData.mWaitEvent)); - Assert(mData.mWaitEvent != NIL_RTSEMEVENT); @@ -887,4 +941,5 @@ return VERR_ALREADY_EXISTS; + /* At the moment we only support one waiter at a time. */ int rc = RTSemMutexRequest(mData.mWaitMutex, uTimeoutMS); if (RT_SUCCESS(rc)) @@ -902,5 +957,4 @@ } - LogFlowFuncLeaveRC(rc); return rc; } @@ -981,6 +1035,7 @@ if (FAILED(autoCaller.rc())) return autoCaller.rc(); - AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); - + /* + * Note: Do not hold any locks here while waiting! + */ uint32_t fWaitFor = ProcessWaitForFlag_None; com::SafeArray flags(ComSafeArrayInArg(aFlags)); Index: /trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp (revision 42271) +++ /trunk/src/VBox/Main/src-client/GuestSessionImpl.cpp (revision 42272) @@ -61,5 +61,4 @@ AssertPtrReturn(aGuest, VERR_INVALID_POINTER); - AssertReturn(aSessionID, VERR_INVALID_PARAMETER); /* Enclose the state transition NotReady->InInit->Ready. */ @@ -67,4 +66,5 @@ AssertReturn(autoInitSpan.isOk(), VERR_OBJECT_DESTROYED); + mData.mTimeout = 30 * 60 * 1000; /* Session timeout is 30 mins by default. */ mData.mParent = aGuest; mData.mId = aSessionID; @@ -352,4 +352,6 @@ int GuestSession::dispatchToProcess(uint32_t uContextID, uint32_t uFunction, void *pvData, size_t cbData) { + LogFlowFuncEnter(); + AssertPtrReturn(pvData, VERR_INVALID_POINTER); AssertReturn(cbData, VERR_INVALID_PARAMETER); @@ -357,4 +359,5 @@ AutoReadLock alock(this COMMA_LOCKVAL_SRC_POS); + int rc; SessionProcesses::const_iterator itProc = mData.mProcesses.find(VBOX_GUESTCTRL_CONTEXTID_GET_PROCESS(uContextID)); @@ -365,7 +368,11 @@ alock.release(); - return pProcess->callbackDispatcher(uContextID, uFunction, pvData, cbData); - } - return VERR_NOT_FOUND; + rc = pProcess->callbackDispatcher(uContextID, uFunction, pvData, cbData); + } + else + rc = VERR_NOT_FOUND; + + LogFlowFuncLeaveRC(rc); + return rc; } @@ -414,8 +421,6 @@ } -int GuestSession::processCreateExInteral(GuestProcessInfo &aProcInfo, IGuestProcess **aProcess) -{ - AssertPtrReturn(aProcess, VERR_INVALID_POINTER); - +int GuestSession::processCreateExInteral(const GuestProcessInfo &aProcInfo, IGuestProcess **aProcess) +{ /* Validate flags. */ if (aProcInfo.mFlags) @@ -430,12 +435,16 @@ } + GuestProcessInfo procInfo = aProcInfo; + /* Adjust timeout. If set to 0, we define * an infinite timeout. */ - if (aProcInfo.mTimeoutMS == 0) - aProcInfo.mTimeoutMS = UINT32_MAX; + if (procInfo.mTimeoutMS == 0) + procInfo.mTimeoutMS = UINT32_MAX; /** @tood Implement process priority + affinity. */ - int rc = VERR_MAX_THRDS_REACHED; + int rc = VERR_MAX_PROCS_REACHED; + if (mData.mProcesses.size() >= VBOX_GUESTCTRL_MAX_PROCESSES) + return rc; AutoWriteLock alock(this COMMA_LOCKVAL_SRC_POS); @@ -447,6 +456,6 @@ for (;;) { - /* Is the context ID already used? Try next ID ... */ - if (!processExists(uNewProcessID++, NULL /* pProgress */)) + /* Is the context ID already used? */ + if (!processExists(uNewProcessID, NULL /* pProgress */)) { /* Callback with context ID was not found. This means @@ -456,4 +465,5 @@ break; } + uNewProcessID++; if (++uTries == UINT32_MAX) @@ -470,5 +480,5 @@ rc = pGuestProcess->init(mData.mParent->getConsole() /* Console */, this /* Session */, - uNewProcessID, aProcInfo); + uNewProcessID, procInfo); if (RT_FAILURE(rc)) throw rc; @@ -489,6 +499,4 @@ inline bool GuestSession::processExists(ULONG uProcessID, ComObjPtr *pProcess) { - AssertReturn(uProcessID, false); - SessionProcesses::const_iterator it = mData.mProcesses.find(uProcessID); if (it != mData.mProcesses.end()) @@ -932,6 +940,8 @@ com::SafeArray affinity; - return ProcessCreateEx(aCommand, ComSafeArrayInArg(aArguments), ComSafeArrayInArg(aEnvironment), - ComSafeArrayInArg(aFlags), aTimeoutMS, ProcessPriority_Default, ComSafeArrayAsInParam(affinity), aProcess); + HRESULT hr = ProcessCreateEx(aCommand, ComSafeArrayInArg(aArguments), ComSafeArrayInArg(aEnvironment), + ComSafeArrayInArg(aFlags), aTimeoutMS, ProcessPriority_Default, ComSafeArrayAsInParam(affinity), aProcess); + LogFlowFuncLeaveRC(hr); + return hr; #endif /* VBOX_WITH_GUEST_CONTROL */ } @@ -957,34 +967,48 @@ procInfo.mCommand = Utf8Str(aCommand); - - com::SafeArray arguments(ComSafeArrayInArg(aArguments)); - procInfo.mArguments.reserve(arguments.size()); - for (size_t i = 0; i < arguments.size(); i++) - procInfo.mArguments[i] = Utf8Str(Bstr(arguments[i])); + procInfo.mFlags = ProcessCreateFlag_None; + + if (aArguments) + { + com::SafeArray arguments(ComSafeArrayInArg(aArguments)); + procInfo.mArguments.reserve(arguments.size()); + for (size_t i = 0; i < arguments.size(); i++) + procInfo.mArguments[i] = Utf8Str(Bstr(arguments[i])); + } int rc = VINF_SUCCESS; - /* Create the process environment: + /* + * Create the process environment: * - Apply the session environment in a first step, and * - Apply environment variables specified by this call to * have the chance of overwriting/deleting session entries. */ - procInfo.mEnvironment = mData.mEnvironment; - com::SafeArray environment(ComSafeArrayInArg(aEnvironment)); - for (size_t i = 0; i < environment.size() && RT_SUCCESS(rc); i++) - rc = mData.mEnvironment.Set(Utf8Str(environment[i])); + procInfo.mEnvironment = mData.mEnvironment; /* Apply original session environment. */ + if (aEnvironment) /* Apply/overwrite environment, if set. */ + { + com::SafeArray environment(ComSafeArrayInArg(aEnvironment)); + for (size_t i = 0; i < environment.size() && RT_SUCCESS(rc); i++) + rc = mData.mEnvironment.Set(Utf8Str(environment[i])); + } if (RT_SUCCESS(rc)) { - com::SafeArray flags(ComSafeArrayInArg(aFlags)); - for (size_t i = 0; i < flags.size(); i++) - procInfo.mFlags |= flags[i]; + if (aFlags) + { + com::SafeArray flags(ComSafeArrayInArg(aFlags)); + for (size_t i = 0; i < flags.size(); i++) + procInfo.mFlags |= flags[i]; + } procInfo.mTimeoutMS = aTimeoutMS; - com::SafeArray affinity(ComSafeArrayInArg(aAffinity)); - procInfo.mAffinity.reserve(affinity.size()); - for (size_t i = 0; i < affinity.size(); i++) - procInfo.mAffinity[i] = affinity[i]; /** @todo Really necessary? Later. */ + if (aAffinity) + { + com::SafeArray affinity(ComSafeArrayInArg(aAffinity)); + procInfo.mAffinity.reserve(affinity.size()); + for (size_t i = 0; i < affinity.size(); i++) + procInfo.mAffinity[i] = affinity[i]; /** @todo Really necessary? Later. */ + } procInfo.mPriority = aPriority;