From 30f07eb1d412fd53da70f350322034aae5b9c561 Mon Sep 17 00:00:00 2001 From: Mike Kaganski Date: Sun, 21 Jan 2018 22:10:09 +0300 Subject: tdf#38915: set cProcessed condition on any process outcome When application is initializing, initially request handler is not processing requests (its state is Starting). Requests processing is enabled in Desktop::OpenClients() after recovery had been processed. If another soffice process is started, it communicates over already established pipe, and sends a request to the first process. In IpcThread::process(), it is decided if the request needs to be checked for completion (e.g., if a file or specific module was requested to be open). After that, the prepared request is posted for processing. In case when the completion should be checked, PipeIpcThread::execute() then waits for Processed condition indefinitely. Request is processed in RequestHandler::ExecuteCmdLineRequests, which first checks that handler's state is RequestsEnabled, and if it isn't, then returns. Otherwise, after processing, Processed condition is set. The problem is, thus, in case when the request comes before requests processing is enabled (e.g., when recovery dialog is open): request handler thread waits indefinitely, but the processed condition will not be set. This will not allow to close the pipe to second process, and it will hang indefinitely. The IPC thread will be hung even after user closes recovery dialog, and continues working with program. So, subsequent attempts to open files from file manager (launching new process) will fail, and new zombie soffice processes will wait the first indefinitely. Also, when first process will be closed by user, the deinit sequence will attempt to wait for the IPC thread to finish (in RequestHandler::Disable(), after all visible windows had been closed), which will leave the first process hung, preventing all subsequent attempts to open LibreOffice. This patch ensures that the Processed condition is set at any outcome in RequestHandler::ExecuteCmdLineRequests. Also, it brings (possibly hidden) recovery dialog to front, making the reason why following attempts to open files fail apparent to user. Change-Id: Ibddf7483e5b1d6167ac7f307ea2442119f446129 Reviewed-on: https://gerrit.libreoffice.org/48280 Tested-by: Jenkins Reviewed-by: Aron Budea Reviewed-by: Mike Kaganski (cherry picked from commit cf333a878ceed18d0343520a2c65be69fc433b1f) Reviewed-on: https://gerrit.libreoffice.org/48289 Reviewed-by: Michael Stahl --- svx/source/inc/docrecovery.hxx | 1 + svx/source/unodraw/recoveryui.cxx | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+) (limited to 'svx/source') diff --git a/svx/source/inc/docrecovery.hxx b/svx/source/inc/docrecovery.hxx index 5ec8fab16e9c..6fce63ffd8a9 100644 --- a/svx/source/inc/docrecovery.hxx +++ b/svx/source/inc/docrecovery.hxx @@ -43,6 +43,7 @@ #define RECOVERY_CMDPART_DO_EMERGENCY_SAVE "/doEmergencySave" #define RECOVERY_CMDPART_DO_RECOVERY "/doAutoRecovery" +#define RECOVERY_CMDPART_DO_BRINGTOFRONT "/doBringToFront" #define RECOVERY_CMD_DO_PREPARE_EMERGENCY_SAVE "vnd.sun.star.autorecovery:/doPrepareEmergencySave" #define RECOVERY_CMD_DO_EMERGENCY_SAVE "vnd.sun.star.autorecovery:/doEmergencySave" diff --git a/svx/source/unodraw/recoveryui.cxx b/svx/source/unodraw/recoveryui.cxx index 2a352ddb473a..fa2c0dce6212 100644 --- a/svx/source/unodraw/recoveryui.cxx +++ b/svx/source/unodraw/recoveryui.cxx @@ -55,6 +55,7 @@ class RecoveryUI : public ::cppu::WeakImplHelper< css::lang::XServiceInfo E_JOB_UNKNOWN, E_DO_EMERGENCY_SAVE, E_DO_RECOVERY, + E_DO_BRINGTOFRONT, }; @@ -70,6 +71,9 @@ class RecoveryUI : public ::cppu::WeakImplHelper< css::lang::XServiceInfo /** @short TODO */ RecoveryUI::EJob m_eJob; + // Active dialog + VclPtr m_pDialog; + // interface public: @@ -101,6 +105,7 @@ class RecoveryUI : public ::cppu::WeakImplHelper< css::lang::XServiceInfo void impl_showAllRecoveredDocs(); + bool impl_doBringToFront(); }; RecoveryUI::RecoveryUI(const css::uno::Reference< css::uno::XComponentContext >& xContext) @@ -152,6 +157,13 @@ css::uno::Any SAL_CALL RecoveryUI::dispatchWithReturnValue(const css::util::URL& break; } + case RecoveryUI::E_DO_BRINGTOFRONT: + { + bool bRet = impl_doBringToFront(); + aRet <<= bRet; + break; + } + default: { aRet <<= false; @@ -211,11 +223,25 @@ RecoveryUI::EJob RecoveryUI::impl_classifyJob(const css::util::URL& aURL) m_eJob = RecoveryUI::E_DO_EMERGENCY_SAVE; else if (aURL.Path == RECOVERY_CMDPART_DO_RECOVERY) m_eJob = RecoveryUI::E_DO_RECOVERY; + else if (aURL.Path == RECOVERY_CMDPART_DO_BRINGTOFRONT) + m_eJob = RecoveryUI::E_DO_BRINGTOFRONT; } return m_eJob; } +struct DialogReleaseGuard +{ + VclPtr& m_rDialog; + template + DialogReleaseGuard(VclPtr& rDialog, DialogPtrClass& p) + : m_rDialog(rDialog) + { + m_rDialog.set(p.get()); + } + ~DialogReleaseGuard() { m_rDialog.reset(); } +}; + bool RecoveryUI::impl_doEmergencySave() { // create core service, which implements the real "emergency save" algorithm. @@ -223,6 +249,7 @@ bool RecoveryUI::impl_doEmergencySave() // create dialog for this operation and bind it to the used core service ScopedVclPtrInstance xDialog(m_pParentWindow, pCore.get()); + DialogReleaseGuard dialogReleaseGuard(m_pDialog, xDialog); // start the dialog short nRet = xDialog->Execute(); @@ -237,6 +264,7 @@ bool RecoveryUI::impl_doRecovery() // create all needed dialogs for this operation // and bind it to the used core service ScopedVclPtrInstance xDialog(m_pParentWindow, pCore.get()); + DialogReleaseGuard dialogReleaseGuard(m_pDialog, xDialog); // start the dialog short nRet = xDialog->Execute(); @@ -280,6 +308,16 @@ void RecoveryUI::impl_showAllRecoveredDocs() } } +bool RecoveryUI::impl_doBringToFront() +{ + VclPtr pDialog(m_pDialog); + if (!pDialog || !pDialog->IsVisible()) + return false; + + pDialog->ToTop(ToTopFlags::RestoreWhenMin | ToTopFlags::ForegroundTask); + return true; +} + } extern "C" SAL_DLLPUBLIC_EXPORT css::uno::XInterface * SAL_CALL -- cgit v1.2.3