Index: /trunk/src/VBox/Main/include/ExtPackManagerImpl.h =================================================================== --- /trunk/src/VBox/Main/include/ExtPackManagerImpl.h (revision 59318) +++ /trunk/src/VBox/Main/include/ExtPackManagerImpl.h (revision 59319) @@ -193,7 +193,5 @@ * @{ */ #if !defined(VBOX_COM_INPROC) - static DECLCALLBACK(int) i_doInstallThreadProc(RTTHREAD hThread, void *pvJob); HRESULT i_doInstall(ExtPackFile *a_pExtPackFile, bool a_fReplace, Utf8Str const *a_pstrDisplayInfo); - static DECLCALLBACK(int) i_doUninstallThreadProc(RTTHREAD hThread, void *pvJob); HRESULT i_doUninstall(const Utf8Str *a_pstrName, bool a_fForcedRemoval, const Utf8Str *a_pstrDisplayInfo); #endif @@ -213,5 +211,4 @@ private: - bool isThereAnyRunningVM() const; // wrapped IExtPackManager properties HRESULT getInstalledExtPacks(std::vector > &aInstalledExtPacks); @@ -232,4 +229,5 @@ BOOL *aUsable); + bool i_areThereAnyRunningVMs(void) const; HRESULT i_runSetUidToRootHelper(Utf8Str const *a_pstrDisplayInfo, const char *a_pszCommand, ...); ExtPack *i_findExtPack(const char *a_pszName); Index: /trunk/src/VBox/Main/src-all/ExtPackManagerImpl.cpp =================================================================== --- /trunk/src/VBox/Main/src-all/ExtPackManagerImpl.cpp (revision 59318) +++ /trunk/src/VBox/Main/src-all/ExtPackManagerImpl.cpp (revision 59319) @@ -172,4 +172,5 @@ #if !defined(VBOX_COM_INPROC) + /** * Extension pack installation job. @@ -177,38 +178,27 @@ class ExtPackInstallTask : public ThreadTask { - public: - explicit ExtPackInstallTask() - { - m_strTaskName = "ExtPackInst"; - } - - ~ExtPackInstallTask(){}; + explicit ExtPackInstallTask() : ThreadTask("ExtPackInst") { } + ~ExtPackInstallTask() { } void handler() { - int vrc = ExtPackManager::i_doInstallThreadProc(NULL, this); - } - - HRESULT Init(const ComPtr& extPackFile, - bool f, - const Utf8Str& dispInfo, - const ComPtr& extPackMgr) - { - HRESULT hrc = S_OK; - - ptrExtPackFile = extPackFile; - fReplace = f; - strDisplayInfo = dispInfo; - ptrExtPackMgr = extPackMgr; - - hrc = ptrProgress.createObject(); + HRESULT hrc = ptrExtPackMgr->i_doInstall(ptrExtPackFile, fReplace, &strDisplayInfo); + ptrProgress->i_notifyComplete(hrc); + } + + HRESULT Init(const ComPtr &a_strExtPackFile, bool a_fReplace, + const Utf8Str &strDispInfo, const ComPtr &a_ptrExtPackMgr) + { + ptrExtPackFile = a_strExtPackFile; + fReplace = a_fReplace; + strDisplayInfo = strDispInfo; + ptrExtPackMgr = a_ptrExtPackMgr; + + HRESULT hrc = ptrProgress.createObject(); if (SUCCEEDED(hrc)) { Bstr bstrDescription("Installing extension pack"); - hrc = ptrProgress->init( -#ifndef VBOX_COM_INPROC - ptrExtPackFile->m->pVirtualBox, -#endif + hrc = ptrProgress->init(ptrExtPackFile->m->pVirtualBox, static_cast(ptrExtPackFile), bstrDescription.raw(), @@ -219,10 +209,6 @@ } - const ComPtr& GetProgressObject() const {return ptrProgress;} - const ComPtr& GetExtPackFileObject() const {return ptrExtPackFile;} - const ComPtr& GetExtPackManagerObject() const {return ptrExtPackMgr;} - bool IsReplaced() const {return fReplace;} - const Utf8Str& GetDisplayInfo(){return strDisplayInfo;} - + /** Smart pointer to the progress object for this job. */ + ComObjPtr ptrProgress; private: /** Smart pointer to the extension pack file. */ @@ -234,9 +220,5 @@ /** Smart pointer to the extension manager. */ ComPtr ptrExtPackMgr; - /** Smart pointer to the progress object for this job. */ - ComObjPtr ptrProgress; }; -/** Pointer to an extension pack installation job. */ -//typedef EXTPACKINSTALLJOB *PEXTPACKINSTALLJOB; /** @@ -246,33 +228,26 @@ { public: - explicit ExtPackUninstallTask() - { - m_strTaskName = "ExtPackUninst"; - } - - ~ExtPackUninstallTask(){} + explicit ExtPackUninstallTask() : ThreadTask("ExtPackUninst") { } + ~ExtPackUninstallTask() { } void handler() { - int vrc = ExtPackManager::i_doUninstallThreadProc(NULL, this); - } - - HRESULT Init(const ComPtr& extPackMgr, const Utf8Str& name, bool f, const Utf8Str& dispInfo) - { - HRESULT hrc = S_OK; - - ptrExtPackMgr = extPackMgr; - strName = name; - fForcedRemoval = f; - strDisplayInfo = dispInfo; - - hrc = ptrProgress.createObject(); + HRESULT hrc = ptrExtPackMgr->i_doUninstall(&strName, fForcedRemoval, &strDisplayInfo); + ptrProgress->i_notifyComplete(hrc); + } + + HRESULT Init(const ComPtr &a_ptrExtPackMgr, const Utf8Str &a_strName, + bool a_fForcedRemoval, const Utf8Str &a_strDisplayInfo) + { + ptrExtPackMgr = a_ptrExtPackMgr; + strName = a_strName; + fForcedRemoval = a_fForcedRemoval; + strDisplayInfo = a_strDisplayInfo; + + HRESULT hrc = ptrProgress.createObject(); if (SUCCEEDED(hrc)) { Bstr bstrDescription("Uninstalling extension pack"); - hrc = ptrProgress->init( -#ifndef VBOX_COM_INPROC - ptrExtPackMgr->m->pVirtualBox, -#endif + hrc = ptrProgress->init(ptrExtPackMgr->m->pVirtualBox, static_cast(ptrExtPackMgr), bstrDescription.raw(), @@ -283,10 +258,6 @@ } - const ComPtr& GetProgressObject() const {return ptrProgress;} - const ComPtr& GetExtPackManagerObject() const {return ptrExtPackMgr;} - const Utf8Str& GetName() const {return strName;} - bool IsForcedRemoval() const {return fForcedRemoval;} - const Utf8Str& GetDisplayInfo(){return strDisplayInfo;} - + /** Smart pointer to the progress object for this job. */ + ComObjPtr ptrProgress; private: /** Smart pointer to the extension manager. */ @@ -298,9 +269,5 @@ /** The display info argument. */ Utf8Str strDisplayInfo; - /** Smart pointer to the progress object for this job. */ - ComObjPtr ptrProgress; - }; -/** Pointer to an extension pack uninstallation job. */ DEFINE_EMPTY_CTOR_DTOR(ExtPackFile) @@ -682,44 +649,38 @@ HRESULT ExtPackFile::install(BOOL aReplace, const com::Utf8Str &aDisplayInfo, ComPtr &aProgress) { - HRESULT hrc = S_OK; - + HRESULT hrc; if (m->fUsable) { - ExtPackInstallTask* pTask = NULL; - ComPtr ptrProgress; + ExtPackInstallTask *pTask = NULL; try { pTask = new ExtPackInstallTask(); - hrc = pTask->Init(this, aReplace != FALSE, aDisplayInfo, m->ptrExtPackMgr); - - if (FAILED(hrc)) - { - delete pTask; - hrc = setError(VBOX_E_IPRT_ERROR, - tr("Looks like creating a progress object for ExtraPackInstallTask object failed")); - throw hrc; - } - - ptrProgress = pTask->GetProgressObject(); - hrc = pTask->createThread(NULL, RTTHREADTYPE_DEFAULT); - if (SUCCEEDED(hrc)) { - hrc = ptrProgress.queryInterfaceTo(aProgress.asOutParam()); + ComPtr ptrProgress = pTask->ptrProgress; + hrc = pTask->createThread(NULL, RTTHREADTYPE_DEFAULT); + pTask = NULL; /* The _completely_ _undocumented_ createThread method always consumes pTask. */ + if (SUCCEEDED(hrc)) + hrc = ptrProgress.queryInterfaceTo(aProgress.asOutParam()); + else + hrc = setError(VBOX_E_IPRT_ERROR, + tr("Starting thread for an extension pack installation failed with %Rrc"), hrc); } else hrc = setError(VBOX_E_IPRT_ERROR, - tr("Starting thread for an extension pack installation failed with %Rrc"), hrc); - } - catch(std::bad_alloc &) + tr("Looks like creating a progress object for ExtraPackInstallTask object failed")); + } + catch (std::bad_alloc &) { hrc = E_OUTOFMEMORY; } - catch(HRESULT eHR) + catch (HRESULT hrcXcpt) { LogFlowThisFunc(("Exception was caught in the function ExtPackFile::install() \n")); - } - + hrc = hrcXcpt; + } + if (pTask) + delete pTask; } else @@ -728,5 +689,5 @@ } -#endif +#endif /* !VBOX_COM_INPROC */ @@ -2112,46 +2073,40 @@ const com::Utf8Str &aDisplayInfo, ComPtr &aProgress) { - HRESULT hrc = S_OK; - Assert(m->enmContext == VBOXEXTPACKCTX_PER_USER_DAEMON); #if !defined(VBOX_COM_INPROC) - ExtPackUninstallTask* pTask = NULL; - ComPtr ptrProgress; + HRESULT hrc; + ExtPackUninstallTask *pTask = NULL; try { pTask = new ExtPackUninstallTask(); - hrc = pTask->Init(this, aName, aForcedRemoval != FALSE, aDisplayInfo); - - if (FAILED(hrc)) - { - delete pTask; + if (SUCCEEDED(hrc)) + { + ComPtr ptrProgress = pTask->ptrProgress; + hrc = pTask->createThread(NULL, RTTHREADTYPE_DEFAULT); + pTask = NULL; /* always consumed by createThread */ + if (SUCCEEDED(hrc)) + hrc = ptrProgress.queryInterfaceTo(aProgress.asOutParam()); + else + hrc = setError(VBOX_E_IPRT_ERROR, + tr("Starting thread for an extension pack uninstallation failed with %Rrc"), hrc); + } + else hrc = setError(VBOX_E_IPRT_ERROR, tr("Looks like creating a progress object for ExtraPackUninstallTask object failed")); - throw hrc; - } - - ptrProgress = pTask->GetProgressObject(); - hrc = pTask->createThread(NULL, RTTHREADTYPE_DEFAULT); - - if (SUCCEEDED(hrc)) - { - hrc = ptrProgress.queryInterfaceTo(aProgress.asOutParam()); - } - else - hrc = setError(VBOX_E_IPRT_ERROR, - tr("Starting thread for an extension pack uninstallation failed with %Rrc"), hrc); - } - catch(std::bad_alloc &) + } + catch (std::bad_alloc &) { hrc = E_OUTOFMEMORY; } - catch(HRESULT eHR) - { - LogFlowThisFunc(("Exception was caught in the function ExtPackManager::uninstall() \n")); - } - + catch (HRESULT hrcXcpt) + { + LogFlowThisFunc(("Exception was caught in the function ExtPackManager::uninstall()\n")); + hrc = hrcXcpt; + } + if (pTask) + delete pTask; return hrc; #else @@ -2612,59 +2567,36 @@ } -bool ExtPackManager::isThereAnyRunningVM() const -{ - Assert(m->pVirtualBox); /* Only called from VBoxSVC */ +/** + * Checks if there are any running VMs. + * + * This is called when uninstalling or replacing an extension pack. + * + * @returns true / false + */ +bool ExtPackManager::i_areThereAnyRunningVMs(void) const +{ + Assert(m->pVirtualBox != NULL); /* Only called from VBoxSVC. */ /* - * Get the list of all _running_ VMs - */ - bool fRunning = false; - com::SafeIfaceArray machines; - com::SafeArray states; - int rc = m->pVirtualBox->COMGETTER(Machines)(ComSafeArrayAsOutParam(machines)); - if (SUCCEEDED(rc)) - rc = m->pVirtualBox->GetMachineStates(ComSafeArrayAsInParam(machines), ComSafeArrayAsOutParam(states)); - if (SUCCEEDED(rc)) - { - for (size_t i = 0; i < machines.size(); ++i) - { - if (machines[i]) - { - MachineState_T machineState = states[i]; - switch (machineState) - { - case MachineState_Running: - case MachineState_Teleporting: - case MachineState_LiveSnapshotting: - case MachineState_Paused: - case MachineState_TeleportingPausedVM: - fRunning = true; - break; - } - } - if (fRunning) - break; - } - } - return fRunning; -} - -/** - * Thread wrapper around doInstall. - * - * @returns VINF_SUCCESS (ignored) - * @param hThread The thread handle (ignored). - * @param pvJob The job structure. - */ -/*static*/ DECLCALLBACK(int) ExtPackManager::i_doInstallThreadProc(RTTHREAD hThread, void *pvJob) -{ - ExtPackInstallTask* pTask = (ExtPackInstallTask*)pvJob; - HRESULT hrc = pTask->GetExtPackManagerObject()->i_doInstall(pTask->GetExtPackFileObject(), - pTask->IsReplaced(), - &pTask->GetDisplayInfo()); - pTask->GetProgressObject()->i_notifyComplete(hrc); - - NOREF(hThread); - return VINF_SUCCESS; + * Get list of machines and their states. + */ + com::SafeIfaceArray SaMachines; + HRESULT hrc = m->pVirtualBox->COMGETTER(Machines)(ComSafeArrayAsOutParam(SaMachines)); + if (SUCCEEDED(hrc)) + { + com::SafeArray SaStates; + hrc = m->pVirtualBox->GetMachineStates(ComSafeArrayAsInParam(SaMachines), ComSafeArrayAsOutParam(SaStates)); + if (SUCCEEDED(hrc)) + { + /* + * Scan the two parallel arrays for machines in the running state. + */ + Assert(SaStates.size() == SaMachines.size()); + for (size_t i = 0; i < SaMachines.size(); ++i) + if (SaMachines[i] && Global::IsOnline(SaStates[i])) + return true; + } + } + return false; } @@ -2695,26 +2627,24 @@ { AutoWriteLock autoLock(this COMMA_LOCKVAL_SRC_POS); + + /* + * Refresh the data we have on the extension pack as it + * may be made stale by direct meddling or some other user. + */ ExtPack *pExtPack; - - bool fRunning = isThereAnyRunningVM(); - if (fRunning) - { - LogRel(("Install extension pack '%s' failed because at least one VM is still running.", pStrName->c_str())); - hrc = setError(E_FAIL, tr("Install extension pack '%s' failed because at least one VM is still running"), - pStrName->c_str()); - } - else - { - /* - * Refresh the data we have on the extension pack as it - * may be made stale by direct meddling or some other user. - */ - hrc = i_refreshExtPack(pStrName->c_str(), false /*a_fUnusableIsError*/, &pExtPack); - } - + hrc = i_refreshExtPack(pStrName->c_str(), false /*a_fUnusableIsError*/, &pExtPack); if (SUCCEEDED(hrc)) { if (pExtPack && a_fReplace) - hrc = pExtPack->i_callUninstallHookAndClose(m->pVirtualBox, false /*a_ForcedRemoval*/); + { + if (!i_areThereAnyRunningVMs()) + hrc = pExtPack->i_callUninstallHookAndClose(m->pVirtualBox, false /*a_ForcedRemoval*/); + else + { + LogRel(("Install extension pack '%s' failed because at least one VM is still running.", pStrName->c_str())); + hrc = setError(E_FAIL, tr("Install extension pack '%s' failed because at least one VM is still running"), + pStrName->c_str()); + } + } else if (pExtPack) hrc = setError(E_FAIL, @@ -2794,23 +2724,4 @@ /** - * Thread wrapper around doUninstall. - * - * @returns VINF_SUCCESS (ignored) - * @param hThread The thread handle (ignored). - * @param pvJob The job structure. - */ -/*static*/ DECLCALLBACK(int) ExtPackManager::i_doUninstallThreadProc(RTTHREAD hThread, void *pvJob) -{ - ExtPackUninstallTask* pTask = (ExtPackUninstallTask*)pvJob; - HRESULT hrc = pTask->GetExtPackManagerObject()->i_doUninstall(&pTask->GetName(), - pTask->IsForcedRemoval(), - &pTask->GetDisplayInfo()); - pTask->GetProgressObject()->i_notifyComplete(hrc); - - NOREF(hThread); - return VINF_SUCCESS; -} - -/** * Worker for IExtPackManager::Uninstall. * @@ -2834,14 +2745,5 @@ { AutoWriteLock autoLock(this COMMA_LOCKVAL_SRC_POS); - ExtPack *pExtPack; - - bool fRunning = isThereAnyRunningVM(); - if (fRunning) - { - LogRel(("Uninstall extension pack '%s' failed because at least one VM is still running.", a_pstrName->c_str())); - hrc = setError(E_FAIL, tr("Uninstall extension pack '%s' failed because at least one VM is still running"), - a_pstrName->c_str()); - } - else + if (a_fForcedRemoval || !i_areThereAnyRunningVMs()) { /* @@ -2849,56 +2751,62 @@ * stale by direct meddling or some other user. */ + ExtPack *pExtPack; hrc = i_refreshExtPack(a_pstrName->c_str(), false /*a_fUnusableIsError*/, &pExtPack); - } - - if (SUCCEEDED(hrc)) - { - if (!pExtPack) + if (SUCCEEDED(hrc)) { - LogRel(("ExtPackManager: Extension pack '%s' is not installed, so nothing to uninstall.\n", a_pstrName->c_str())); - hrc = S_OK; /* nothing to uninstall */ - } - else - { - /* - * Call the uninstall hook and unload the main dll. - */ - hrc = pExtPack->i_callUninstallHookAndClose(m->pVirtualBox, a_fForcedRemoval); - if (SUCCEEDED(hrc)) + if (!pExtPack) + { + LogRel(("ExtPackManager: Extension pack '%s' is not installed, so nothing to uninstall.\n", a_pstrName->c_str())); + hrc = S_OK; /* nothing to uninstall */ + } + else { /* - * Run the set-uid-to-root binary that performs the - * uninstallation. Then refresh the object. - * - * This refresh is theorically subject to races, but it's of - * the don't-do-that variety. + * Call the uninstall hook and unload the main dll. */ - const char *pszForcedOpt = a_fForcedRemoval ? "--forced" : NULL; - hrc = i_runSetUidToRootHelper(a_pstrDisplayInfo, - "uninstall", - "--base-dir", m->strBaseDir.c_str(), - "--name", a_pstrName->c_str(), - pszForcedOpt, /* Last as it may be NULL. */ - (const char *)NULL); + hrc = pExtPack->i_callUninstallHookAndClose(m->pVirtualBox, a_fForcedRemoval); if (SUCCEEDED(hrc)) { - hrc = i_refreshExtPack(a_pstrName->c_str(), false /*a_fUnusableIsError*/, &pExtPack); + /* + * Run the set-uid-to-root binary that performs the + * uninstallation. Then refresh the object. + * + * This refresh is theorically subject to races, but it's of + * the don't-do-that variety. + */ + const char *pszForcedOpt = a_fForcedRemoval ? "--forced" : NULL; + hrc = i_runSetUidToRootHelper(a_pstrDisplayInfo, + "uninstall", + "--base-dir", m->strBaseDir.c_str(), + "--name", a_pstrName->c_str(), + pszForcedOpt, /* Last as it may be NULL. */ + (const char *)NULL); if (SUCCEEDED(hrc)) { - if (!pExtPack) - LogRel(("ExtPackManager: Successfully uninstalled extension pack '%s'.\n", a_pstrName->c_str())); - else - hrc = setError(E_FAIL, - tr("Uninstall extension pack '%s' failed under mysterious circumstances"), - a_pstrName->c_str()); + hrc = i_refreshExtPack(a_pstrName->c_str(), false /*a_fUnusableIsError*/, &pExtPack); + if (SUCCEEDED(hrc)) + { + if (!pExtPack) + LogRel(("ExtPackManager: Successfully uninstalled extension pack '%s'.\n", a_pstrName->c_str())); + else + hrc = setError(E_FAIL, + tr("Uninstall extension pack '%s' failed under mysterious circumstances"), + a_pstrName->c_str()); + } } - } - else - { - ErrorInfoKeeper Eik; - i_refreshExtPack(a_pstrName->c_str(), false /*a_fUnusableIsError*/, NULL); + else + { + ErrorInfoKeeper Eik; + i_refreshExtPack(a_pstrName->c_str(), false /*a_fUnusableIsError*/, NULL); + } } } } + } + else + { + LogRel(("Uninstall extension pack '%s' failed because at least one VM is still running.", a_pstrName->c_str())); + hrc = setError(E_FAIL, tr("Uninstall extension pack '%s' failed because at least one VM is still running"), + a_pstrName->c_str()); }