diff options
author | Joachim Lingner <jl@openoffice.org> | 2010-10-07 17:20:55 +0200 |
---|---|---|
committer | Joachim Lingner <jl@openoffice.org> | 2010-10-07 17:20:55 +0200 |
commit | 8c1f6c9b46671bc9d3a7dc16d1343cd12a9d0411 (patch) | |
tree | e680b86b9964e47f2b3377af7e7c52f98dbe8a86 /desktop | |
parent | 8281fcfc6399d8f9d77bf848fc2ed2bf44321d24 (diff) |
jl161 #i114933# solve deadlock problem when adding an extension
Diffstat (limited to 'desktop')
4 files changed, 358 insertions, 132 deletions
diff --git a/desktop/source/deployment/manager/dp_commandenvironments.cxx b/desktop/source/deployment/manager/dp_commandenvironments.cxx index c2801ba1d965..0de1f9e96e91 100644 --- a/desktop/source/deployment/manager/dp_commandenvironments.cxx +++ b/desktop/source/deployment/manager/dp_commandenvironments.cxx @@ -31,6 +31,8 @@ #include "com/sun/star/deployment/VersionException.hpp" #include "com/sun/star/deployment/LicenseException.hpp" #include "com/sun/star/deployment/InstallException.hpp" +#include "com/sun/star/deployment/DependencyException.hpp" +#include "com/sun/star/deployment/PlatformException.hpp" #include "com/sun/star/task/XInteractionApprove.hpp" #include "com/sun/star/task/XInteractionAbort.hpp" #include "com/sun/star/task/XInteractionHandler.hpp" @@ -250,7 +252,43 @@ void NoLicenseCommandEnv::handle( handle_(approve, abort, xRequest); } +// SilentCheckPrerequisitesCommandEnv::SilentCheckPrerequisitesCommandEnv( +// css::uno::Reference< css::task::XInteractionHandler> const & handler): +// BaseCommandEnv(handler) +// { +// } +SilentCheckPrerequisitesCommandEnv::SilentCheckPrerequisitesCommandEnv() +{ +} + +void SilentCheckPrerequisitesCommandEnv::handle( + Reference< task::XInteractionRequest> const & xRequest ) + throw (uno::RuntimeException) +{ + uno::Any request( xRequest->getRequest() ); + OSL_ASSERT( request.getValueTypeClass() == uno::TypeClass_EXCEPTION ); + deployment::LicenseException licExc; + deployment::PlatformException platformExc; + deployment::DependencyException depExc; + bool approve = false; + bool abort = false; + + if (request >>= licExc) + { + approve = true; + handle_(approve, abort, xRequest); + } + else if ((request >>= platformExc) + || (request >>= depExc)) + { + m_Exception = request; + } + else + { + m_UnknownException = request; + } +} // NoExceptionCommandEnv::NoExceptionCommandEnv( // css::uno::Reference< css::task::XInteractionHandler> const & handler, // css::uno::Type const & type): diff --git a/desktop/source/deployment/manager/dp_commandenvironments.hxx b/desktop/source/deployment/manager/dp_commandenvironments.hxx index aa21f8281c72..bea11586d462 100644 --- a/desktop/source/deployment/manager/dp_commandenvironments.hxx +++ b/desktop/source/deployment/manager/dp_commandenvironments.hxx @@ -135,6 +135,29 @@ public: }; +/* For use in XExtensionManager::addExtension in the call to + XPackage::checkPrerequisites + It prevents all user interactions. The license is always accepted. + It remembers if there was a platform or a dependency exception in + the member m_bException. if there was any other exception then m_bUnknownException + is set. + + */ +class SilentCheckPrerequisitesCommandEnv : public BaseCommandEnv +{ +public: + SilentCheckPrerequisitesCommandEnv(); + // XInteractionHandler + virtual void SAL_CALL handle( + css::uno::Reference<css::task::XInteractionRequest > const & xRequest ) + throw (css::uno::RuntimeException); + + // Set to true if a PlatformException or a DependencyException were handled. + css::uno::Any m_Exception; + // Set to true if an unknown exception was handled. + css::uno::Any m_UnknownException; +}; + // class NoExceptionCommandEnv : public BaseCommandEnv // { // css::uno::Type m_type; diff --git a/desktop/source/deployment/manager/dp_extensionmanager.cxx b/desktop/source/deployment/manager/dp_extensionmanager.cxx index c82973f1b680..23fb36ed6a43 100644 --- a/desktop/source/deployment/manager/dp_extensionmanager.cxx +++ b/desktop/source/deployment/manager/dp_extensionmanager.cxx @@ -139,6 +139,37 @@ void writeLastModified(OUString & url, Reference<ucb::XCommandEnvironment> const OUSTR("Failed to update") + url, 0, exc); } } + +class ExtensionRemoveGuard +{ + css::uno::Reference<css::deployment::XPackage> m_extension; + css::uno::Reference<css::deployment::XPackageManager> m_xPackageManager; + +public: + ExtensionRemoveGuard( + css::uno::Reference<css::deployment::XPackage> const & extension, + css::uno::Reference<css::deployment::XPackageManager> const & xPackageManager): + m_extension(extension), m_xPackageManager(xPackageManager) {} + ~ExtensionRemoveGuard(); + + void reset(css::uno::Reference<css::deployment::XPackage> const & extension) { + m_extension = extension; + } +}; + +ExtensionRemoveGuard::~ExtensionRemoveGuard() +{ + try { + if (m_xPackageManager.is() && m_extension.is()) + m_xPackageManager->removePackage( + dp_misc::getIdentifier(m_extension), ::rtl::OUString(), + css::uno::Reference<css::task::XAbortChannel>(), + css::uno::Reference<css::ucb::XCommandEnvironment>()); + } catch (...) { + OSL_ASSERT(0); + } +} + } //end namespace namespace dp_manager { @@ -501,6 +532,103 @@ ExtensionManager::getSupportedPackageTypes() return m_userRepository->getSupportedPackageTypes(); } +bool ExtensionManager::doChecksForAddExtension( + Reference<deploy::XPackageManager> const & xPackageMgr, + uno::Sequence<beans::NamedValue> const & properties, + css::uno::Reference<css::deployment::XPackage> const & xTmpExtension, + Reference<task::XAbortChannel> const & xAbortChannel, + Reference<ucb::XCommandEnvironment> const & xCmdEnv, + Reference<deploy::XPackage> & out_existingExtension ) + throw (deploy::DeploymentException, + ucb::CommandFailedException, + ucb::CommandAbortedException, + lang::IllegalArgumentException, + uno::RuntimeException) +{ + try + { + Reference<deploy::XPackage> xOldExtension; + const OUString sIdentifier = dp_misc::getIdentifier(xTmpExtension); + const OUString sFileName = xTmpExtension->getName(); + const OUString sDisplayName = xTmpExtension->getDisplayName(); + const OUString sVersion = xTmpExtension->getVersion(); + + try + { + xOldExtension = xPackageMgr->getDeployedPackage( + sIdentifier, sFileName, xCmdEnv); + out_existingExtension = xOldExtension; + } + catch (lang::IllegalArgumentException &) + { + } + bool bCanInstall = false; + + //This part is not guarded against other threads removing, adding, disabling ... + //etc. the same extension. + //checkInstall is safe because it notifies the user if the extension is not yet + //installed in the same repository. Because addExtension has its own guard + //(m_addMutex), another thread cannot add the extension in the meantime. + //checkUpdate is called if the same extension exists in the same + //repository. The user is asked if they want to replace it. Another + //thread + //could already remove the extension. So asking the user was not + //necessary. No harm is done. The other thread may also ask the user + //if he wants to remove the extension. This depends on the + //XCommandEnvironment which it passes to removeExtension. + if (xOldExtension.is()) + { + //throws a CommandFailedException if the user cancels + //the action. + checkUpdate(sVersion, sDisplayName,xOldExtension, xCmdEnv); + } + else + { + //throws a CommandFailedException if the user cancels + //the action. + checkInstall(sDisplayName, xCmdEnv); + } + //Prevent showing the license if requested. + Reference<ucb::XCommandEnvironment> _xCmdEnv(xCmdEnv); + ExtensionProperties props(OUString(), properties, Reference<ucb::XCommandEnvironment>()); + + dp_misc::DescriptionInfoset info(dp_misc::getDescriptionInfoset(xTmpExtension->getURL())); + const ::boost::optional<dp_misc::SimpleLicenseAttributes> licenseAttributes = + info.getSimpleLicenseAttributes(); + + if (licenseAttributes && licenseAttributes->suppressIfRequired + && props.isSuppressedLicense()) + _xCmdEnv = Reference<ucb::XCommandEnvironment>( + new NoLicenseCommandEnv(xCmdEnv->getInteractionHandler())); + + bCanInstall = xTmpExtension->checkPrerequisites( + xAbortChannel, _xCmdEnv, xOldExtension.is() || props.isExtensionUpdate()) == 0 ? true : false; + + return bCanInstall; + } + catch (deploy::DeploymentException& ) { + throw; + } catch (ucb::CommandFailedException & ) { + throw; + } catch (ucb::CommandAbortedException & ) { + throw; + } catch (lang::IllegalArgumentException &) { + throw; + } catch (uno::RuntimeException &) { + throw; + } catch (uno::Exception &) { + uno::Any excOccurred = ::cppu::getCaughtException(); + deploy::DeploymentException exc( + OUSTR("Extension Manager: exception in doChecksForAddExtension"), + static_cast<OWeakObject*>(this), excOccurred); + throw exc; + } catch (...) { + throw uno::RuntimeException( + OUSTR("Extension Manager: unexpected exception in doChecksForAddExtension"), + static_cast<OWeakObject*>(this)); + } +} + // Only add to shared and user repository Reference<deploy::XPackage> ExtensionManager::addExtension( OUString const & url, uno::Sequence<beans::NamedValue> const & properties, @@ -524,165 +652,182 @@ Reference<deploy::XPackage> ExtensionManager::addExtension( throw lang::IllegalArgumentException( OUSTR("No valid repository name provided."), static_cast<cppu::OWeakObject*>(this), 0); - ::osl::MutexGuard guard(getMutex()); + //We must make sure that the xTmpExtension is not create twice, because this + //would remove the first one. + ::osl::MutexGuard addGuard(m_addMutex); + Reference<deploy::XPackage> xTmpExtension = getTempExtension(url, xAbortChannel, xCmdEnv); + //Make sure the extension is removed from the tmp repository in case + //of an exception + ExtensionRemoveGuard tmpExtensionRemoveGuard(xTmpExtension, m_tmpRepository); const OUString sIdentifier = dp_misc::getIdentifier(xTmpExtension); const OUString sFileName = xTmpExtension->getName(); - const OUString sDisplayName = xTmpExtension->getDisplayName(); - const OUString sVersion = xTmpExtension->getVersion(); - dp_misc::DescriptionInfoset info(dp_misc::getDescriptionInfoset(xTmpExtension->getURL())); - const ::boost::optional<dp_misc::SimpleLicenseAttributes> licenseAttributes = - info.getSimpleLicenseAttributes(); Reference<deploy::XPackage> xOldExtension; Reference<deploy::XPackage> xExtensionBackup; - uno::Any excOccurred1; uno::Any excOccurred2; bool bUserDisabled = false; - try + bool bCanInstall = doChecksForAddExtension( + xPackageManager, + properties, + xTmpExtension, + xAbortChannel, + xCmdEnv, + xOldExtension ); + { - bUserDisabled = isUserDisabled(sIdentifier, sFileName); - try - { - xOldExtension = xPackageManager->getDeployedPackage( - sIdentifier, sFileName, xCmdEnv); - } - catch (lang::IllegalArgumentException &) - { - } - bool bCanInstall = false; - try + // In this garded section (getMutex) we must not use the argument xCmdEnv + // because it may bring up dialogs (XInteractionHandler::handle) this + //may potententially deadlock. See issue + //http://qa.openoffice.org/issues/show_bug.cgi?id=114933 + //By not providing xCmdEnv the underlying APIs will throw an exception if + //the XInteractionRequest cannot be handled + ::osl::MutexGuard guard(getMutex()); + + if (bCanInstall) { - if (xOldExtension.is()) + try { - //throws a CommandFailedException if the user cancels - //the action. - checkUpdate(sVersion, sDisplayName,xOldExtension, xCmdEnv); + bUserDisabled = isUserDisabled(sIdentifier, sFileName); + if (xOldExtension.is()) + { + try + { + xOldExtension->revokePackage( + xAbortChannel, Reference<ucb::XCommandEnvironment>()); + //save the old user extension in case the user aborts + //store the extension in the tmp repository, this will overwrite + //xTmpPackage (same identifier). Do not let the user abort or + //interact + //importing the old extension in the tmp repository will remove + //the xTmpExtension + //no command environment supplied, only this class shall interact + //with the user! + xExtensionBackup = m_tmpRepository->importExtension( + xOldExtension, Reference<task::XAbortChannel>(), + Reference<ucb::XCommandEnvironment>()); + tmpExtensionRemoveGuard.reset(xExtensionBackup); + xTmpExtension = xExtensionBackup; + OSL_ASSERT(xTmpExtension.is()); + } + catch (lang::DisposedException &) + { + //Another thread might have removed the extension meanwhile + } + } + //check again dependencies but prevent user interaction, + //We can disregard the license, because the user must have already + //accepted it, whe we called checkPrerequisites the first time + SilentCheckPrerequisitesCommandEnv * pSilentCommandEnv = + new SilentCheckPrerequisitesCommandEnv(); + Reference<ucb::XCommandEnvironment> silentCommandEnv(pSilentCommandEnv); + + sal_Int32 failedPrereq = xTmpExtension->checkPrerequisites( + xAbortChannel, silentCommandEnv, true); + if (failedPrereq == 0) + { + xNewExtension = xPackageManager->addPackage( + url, properties, OUString(), xAbortChannel, + Reference<ucb::XCommandEnvironment>()); + //If we add a user extension and there is already one which was + //disabled by a user, then the newly installed one is enabled. If we + //add to another repository then the user extension remains + //disabled. + bool bUserDisabled2 = bUserDisabled; + if (repository.equals(OUSTR("user"))) + bUserDisabled2 = false; + + activateExtension( + dp_misc::getIdentifier(xNewExtension), + xNewExtension->getName(), bUserDisabled2, false, xAbortChannel, + Reference<ucb::XCommandEnvironment>()); + } + else + { + if (pSilentCommandEnv->m_Exception.hasValue()) + ::cppu::throwException(pSilentCommandEnv->m_Exception); + else if ( pSilentCommandEnv->m_UnknownException.hasValue()) + ::cppu::throwException(pSilentCommandEnv->m_UnknownException); + else + throw deploy::DeploymentException ( + OUSTR("Extension Manager: exception during addExtension, ckeckPrerequisites failed"), + static_cast<OWeakObject*>(this), uno::Any()); + } } - else - { - //throws a CommandFailedException if the user cancels - //the action. - checkInstall(sDisplayName, xCmdEnv); + catch (deploy::DeploymentException& ) { + excOccurred2 = ::cppu::getCaughtException(); + } catch (ucb::CommandFailedException & ) { + excOccurred2 = ::cppu::getCaughtException(); + } catch (ucb::CommandAbortedException & ) { + excOccurred2 = ::cppu::getCaughtException(); + } catch (lang::IllegalArgumentException &) { + excOccurred2 = ::cppu::getCaughtException(); + } catch (uno::RuntimeException &) { + excOccurred2 = ::cppu::getCaughtException(); + } catch (...) { + excOccurred2 = ::cppu::getCaughtException(); + deploy::DeploymentException exc( + OUSTR("Extension Manager: exception during addExtension, url: ") + + url, static_cast<OWeakObject*>(this), excOccurred2); + excOccurred2 <<= exc; } - //Prevent showing the license if requested. - Reference<ucb::XCommandEnvironment> _xCmdEnv(xCmdEnv); - ExtensionProperties props(OUString(), properties, Reference<ucb::XCommandEnvironment>()); - if (licenseAttributes && licenseAttributes->suppressIfRequired - && props.isSuppressedLicense()) - _xCmdEnv = Reference<ucb::XCommandEnvironment>( - new NoLicenseCommandEnv(xCmdEnv->getInteractionHandler())); - - bCanInstall = xTmpExtension->checkPrerequisites( - xAbortChannel, _xCmdEnv, xOldExtension.is() || props.isExtensionUpdate()) == 0 ? true : false; - } - catch (deploy::DeploymentException& ) { - excOccurred1 = ::cppu::getCaughtException(); - } catch (ucb::CommandFailedException & ) { - excOccurred1 = ::cppu::getCaughtException(); - } catch (ucb::CommandAbortedException & ) { - excOccurred1 = ::cppu::getCaughtException(); - } catch (lang::IllegalArgumentException &) { - excOccurred1 = ::cppu::getCaughtException(); - } catch (uno::RuntimeException &) { - excOccurred1 = ::cppu::getCaughtException(); - } catch (...) { - excOccurred1 = ::cppu::getCaughtException(); - deploy::DeploymentException exc( - OUSTR("Extension Manager: exception during addExtension, url: ") - + url, static_cast<OWeakObject*>(this), excOccurred1); - excOccurred1 <<= exc; } - if (bCanInstall) + if (excOccurred2.hasValue()) { - if (xOldExtension.is()) + //It does not matter what exception is thrown. We try to + //recover the original status. + //If the user aborted installation then a ucb::CommandAbortedException + //is thrown. + //Use a private AbortChannel so the user cannot interrupt. + try { - xOldExtension->revokePackage(xAbortChannel, xCmdEnv); - //save the old user extension in case the user aborts - //store the extension in the tmp repository, this will overwrite - //xTmpPackage (same identifier). Do not let the user abort or - //interact Reference<ucb::XCommandEnvironment> tmpCmdEnv( - new TmpRepositoryCommandEnv(xCmdEnv->getInteractionHandler())); - //importing the old extension in the tmp repository will remove - //the xTmpExtension - xTmpExtension = 0; - xExtensionBackup = m_tmpRepository->importExtension( - xOldExtension, Reference<task::XAbortChannel>(), - tmpCmdEnv); + new TmpRepositoryCommandEnv()); + if (xExtensionBackup.is()) + { + Reference<deploy::XPackage> xRestored = + xPackageManager->importExtension( + xExtensionBackup, Reference<task::XAbortChannel>(), + tmpCmdEnv); + } + activateExtension( + sIdentifier, sFileName, bUserDisabled, false, + Reference<task::XAbortChannel>(), tmpCmdEnv); } - xNewExtension = xPackageManager->addPackage( - url, properties, OUString(), xAbortChannel, xCmdEnv); - //If we add a user extension and there is already one which was - //disabled by a user, then the newly installed one is enabled. If we - //add to another repository then the user extension remains - //disabled. - bool bUserDisabled2 = bUserDisabled; - if (repository.equals(OUSTR("user"))) - bUserDisabled2 = false; - activateExtension( - dp_misc::getIdentifier(xNewExtension), - xNewExtension->getName(), bUserDisabled2, false, xAbortChannel, xCmdEnv); - fireModified(); + catch (...) + { + } + ::cppu::throwException(excOccurred2); } - } - catch (deploy::DeploymentException& ) { - excOccurred2 = ::cppu::getCaughtException(); + } // leaving the garded section (getMutex()) + + try + { + fireModified(); + + }catch (deploy::DeploymentException& ) { + throw; } catch (ucb::CommandFailedException & ) { - excOccurred2 = ::cppu::getCaughtException(); + throw; } catch (ucb::CommandAbortedException & ) { - excOccurred2 = ::cppu::getCaughtException(); + throw; } catch (lang::IllegalArgumentException &) { - excOccurred2 = ::cppu::getCaughtException(); + throw; } catch (uno::RuntimeException &) { - excOccurred2 = ::cppu::getCaughtException(); - } catch (...) { - excOccurred2 = ::cppu::getCaughtException(); + throw; + } catch (uno::Exception &) { + uno::Any excOccurred = ::cppu::getCaughtException(); deploy::DeploymentException exc( - OUSTR("Extension Manager: exception during addExtension, url: ") - + url, static_cast<OWeakObject*>(this), excOccurred2); - excOccurred2 <<= exc; - } - - if (excOccurred2.hasValue()) - { - //It does not matter what exception is thrown. We try to - //recover the original status. - //If the user aborted installation then a ucb::CommandAbortedException - //is thrown. - //Use a private AbortChannel so the user cannot interrupt. - try - { - Reference<ucb::XCommandEnvironment> tmpCmdEnv( - new TmpRepositoryCommandEnv(xCmdEnv->getInteractionHandler())); - if (xExtensionBackup.is()) - { - Reference<deploy::XPackage> xRestored = - xPackageManager->importExtension( - xExtensionBackup, Reference<task::XAbortChannel>(), - tmpCmdEnv); - } - activateExtension( - sIdentifier, sFileName, bUserDisabled, false, - Reference<task::XAbortChannel>(), tmpCmdEnv); - if (xTmpExtension.is() || xExtensionBackup.is()) - m_tmpRepository->removePackage( - sIdentifier, OUString(), xAbortChannel, xCmdEnv); - fireModified(); - } - catch (...) - { - } - ::cppu::throwException(excOccurred2); + OUSTR("Extension Manager: exception in doChecksForAddExtension"), + static_cast<OWeakObject*>(this), excOccurred); + throw exc; + } catch (...) { + throw uno::RuntimeException( + OUSTR("Extension Manager: unexpected exception in doChecksForAddExtension"), + static_cast<OWeakObject*>(this)); } - if (xTmpExtension.is() || xExtensionBackup.is()) - m_tmpRepository->removePackage( - sIdentifier,OUString(), xAbortChannel, xCmdEnv); - - if (excOccurred1.hasValue()) - ::cppu::throwException(excOccurred1); return xNewExtension; } diff --git a/desktop/source/deployment/manager/dp_extensionmanager.hxx b/desktop/source/deployment/manager/dp_extensionmanager.hxx index 64cada7da3ac..d0cb4bb8339f 100644 --- a/desktop/source/deployment/manager/dp_extensionmanager.hxx +++ b/desktop/source/deployment/manager/dp_extensionmanager.hxx @@ -235,6 +235,8 @@ private: css::uno::Reference<css::deployment::XPackageManager> m_bundledRepository; css::uno::Reference<css::deployment::XPackageManager> m_tmpRepository; + //only to be used within addExtension + ::osl::Mutex m_addMutex; /* contains the names of all repositories (except tmp) in order of there priority. That is, the first element is "user" follod by "shared" and then "bundled" @@ -296,6 +298,24 @@ private: css::uno::Reference<css::deployment::XPackageManager> getPackageManager(::rtl::OUString const & repository) throw (css::lang::IllegalArgumentException); + + //Do some necessary checks and user interaction. This function does not + //aquire the extension manager mutex. + //Returns true if the extension can be installed. + bool doChecksForAddExtension( + css::uno::Reference<css::deployment::XPackageManager> const & xPackageMgr, + css::uno::Sequence<css::beans::NamedValue> const & properties, + css::uno::Reference<css::deployment::XPackage> const & xTmpExtension, + css::uno::Reference<css::task::XAbortChannel> const & xAbortChannel, + css::uno::Reference<css::ucb::XCommandEnvironment> const & xCmdEnv, + css::uno::Reference<css::deployment::XPackage> & out_existingExtension ) + throw (css::deployment::DeploymentException, + css::ucb::CommandFailedException, + css::ucb::CommandAbortedException, + css::lang::IllegalArgumentException, + css::uno::RuntimeException); + + }; } |