desktop/source/app/app.cxx | 34 ++++++++++++++++++++++++++--- desktop/source/app/officeipcthread.cxx | 19 +++++++++++++--- svx/source/inc/docrecovery.hxx | 1 svx/source/unodraw/recoveryui.cxx | 38 +++++++++++++++++++++++++++++++++ vcl/win/app/salinst.cxx | 3 +- 5 files changed, 87 insertions(+), 8 deletions(-)
New commits: commit 5ef05e2b54983a3937c0ecae09ef5808117596b1 Author: Mike Kaganski <mike.kagan...@collabora.com> Date: Sun Jan 21 22:10:09 2018 +0300 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 <c...@libreoffice.org> Reviewed-by: Aron Budea <aron.bu...@collabora.com> Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Reviewed-on: https://gerrit.libreoffice.org/48292 Tested-by: Aron Budea <aron.bu...@collabora.com> (cherry picked from commit 88a37944f55949c122fb4d5b7e504e40f25ed3a8) diff --git a/desktop/source/app/app.cxx b/desktop/source/app/app.cxx index 7c70eca66101..10fadb86626b 100644 --- a/desktop/source/app/app.cxx +++ b/desktop/source/app/app.cxx @@ -1134,6 +1134,15 @@ void impl_checkRecoveryState(bool& bCrashed , bSessionDataExists = elements && session; } +Reference< css::frame::XSynchronousDispatch > g_xRecoveryUI; + +template <class Ref> +struct RefClearGuard +{ + Ref& m_Ref; + RefClearGuard(Ref& ref) : m_Ref(ref) {} + ~RefClearGuard() { m_Ref.clear(); } +}; /* @short start the recovery wizard. @@ -1148,12 +1157,13 @@ bool impl_callRecoveryUI(bool bEmergencySave , css::uno::Reference< css::uno::XComponentContext > xContext = ::comphelper::getProcessComponentContext(); - Reference< css::frame::XSynchronousDispatch > xRecoveryUI( + g_xRecoveryUI.set( xContext->getServiceManager()->createInstanceWithContext("com.sun.star.comp.svx.RecoveryUI", xContext), css::uno::UNO_QUERY_THROW); + RefClearGuard<Reference< css::frame::XSynchronousDispatch >> refClearGuard(g_xRecoveryUI); Reference< css::util::XURLTransformer > xURLParser = - css::util::URLTransformer::create(::comphelper::getProcessComponentContext()); + css::util::URLTransformer::create(xContext); css::util::URL aURL; if (bEmergencySave) @@ -1165,6 +1175,24 @@ bool impl_callRecoveryUI(bool bEmergencySave , xURLParser->parseStrict(aURL); + css::uno::Any aRet = g_xRecoveryUI->dispatchWithReturnValue(aURL, css::uno::Sequence< css::beans::PropertyValue >()); + bool bRet = false; + aRet >>= bRet; + return bRet; +} + +bool impl_bringToFrontRecoveryUI() +{ + Reference< css::frame::XSynchronousDispatch > xRecoveryUI(g_xRecoveryUI); + if (!xRecoveryUI.is()) + return false; + + css::util::URL aURL; + aURL.Complete = "vnd.sun.star.autorecovery:/doBringToFront"; + Reference< css::util::XURLTransformer > xURLParser = + css::util::URLTransformer::create(::comphelper::getProcessComponentContext()); + xURLParser->parseStrict(aURL); + css::uno::Any aRet = xRecoveryUI->dispatchWithReturnValue(aURL, css::uno::Sequence< css::beans::PropertyValue >()); bool bRet = false; aRet >>= bRet; @@ -2370,7 +2398,7 @@ void Desktop::HandleAppEvent( const ApplicationEvent& rAppEvent ) createAcceptor(rAppEvent.GetStringData()); break; case ApplicationEvent::Type::Appear: - if ( !GetCommandLineArgs().IsInvisible() ) + if ( !GetCommandLineArgs().IsInvisible() && !impl_bringToFrontRecoveryUI() ) { Reference< css::uno::XComponentContext > xContext = ::comphelper::getProcessComponentContext(); diff --git a/desktop/source/app/officeipcthread.cxx b/desktop/source/app/officeipcthread.cxx index 1df2e06c722e..e36a9c9ba863 100644 --- a/desktop/source/app/officeipcthread.cxx +++ b/desktop/source/app/officeipcthread.cxx @@ -1349,6 +1349,12 @@ static void AddConversionsToDispatchList( } } +struct ConditionSetGuard +{ + osl::Condition* m_pCondition; + ConditionSetGuard(osl::Condition* pCondition) : m_pCondition(pCondition) {} + ~ConditionSetGuard() { if (m_pCondition) m_pCondition->set(); } +}; bool RequestHandler::ExecuteCmdLineRequests( ProcessDocumentsRequest& aRequest, bool noTerminate) @@ -1356,6 +1362,9 @@ bool RequestHandler::ExecuteCmdLineRequests( // protect the dispatch list osl::ClearableMutexGuard aGuard( GetMutex() ); + // ensure that Processed flag (if exists) is signaled in any outcome + ConditionSetGuard(aRequest.pcProcessed); + static std::vector<DispatchWatcher::DispatchRequest> aDispatchList; // Create dispatch list for dispatch watcher @@ -1373,7 +1382,13 @@ bool RequestHandler::ExecuteCmdLineRequests( if ( pGlobal.is() ) { if( ! pGlobal->AreRequestsEnabled() ) + { + // Either starting, or downing - do not process the request, just try to bring Office to front + ApplicationEvent* pAppEvent = + new ApplicationEvent(ApplicationEvent::Type::Appear); + ImplPostForeignAppEvent(pAppEvent); return bShutdown; + } pGlobal->mnPendingRequests += aDispatchList.size(); if ( !pGlobal->mpDispatchWatcher.is() ) @@ -1391,10 +1406,6 @@ bool RequestHandler::ExecuteCmdLineRequests( // Execute dispatch requests bShutdown = dispatchWatcher->executeDispatchRequests( aTempList, noTerminate); - - // set processed flag - if (aRequest.pcProcessed != nullptr) - aRequest.pcProcessed->set(); } return bShutdown; diff --git a/svx/source/inc/docrecovery.hxx b/svx/source/inc/docrecovery.hxx index 244c56983439..7cdc907a2aa7 100644 --- a/svx/source/inc/docrecovery.hxx +++ b/svx/source/inc/docrecovery.hxx @@ -42,6 +42,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 0404e397fcc6..3a02a34957fd 100644 --- a/svx/source/unodraw/recoveryui.cxx +++ b/svx/source/unodraw/recoveryui.cxx @@ -56,6 +56,7 @@ class RecoveryUI : public ::cppu::WeakImplHelper< css::lang::XServiceInfo E_JOB_UNKNOWN, E_DO_EMERGENCY_SAVE, E_DO_RECOVERY, + E_DO_BRINGTOFRONT, }; @@ -71,6 +72,9 @@ class RecoveryUI : public ::cppu::WeakImplHelper< css::lang::XServiceInfo /** @short TODO */ RecoveryUI::EJob m_eJob; + // Active dialog + VclPtr<Dialog> m_pDialog; + // interface public: @@ -111,6 +115,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) @@ -171,6 +176,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; @@ -230,11 +242,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<Dialog>& m_rDialog; + template <class DialogPtrClass> + DialogReleaseGuard(VclPtr<Dialog>& 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. @@ -243,6 +269,7 @@ bool RecoveryUI::impl_doEmergencySave() // create dialog for this operation and bind it to the used core service ScopedVclPtrInstance<svxdr::SaveDialog> xDialog(m_pParentWindow, pCore); + DialogReleaseGuard dialogReleaseGuard(m_pDialog, xDialog); // start the dialog short nRet = xDialog->Execute(); @@ -258,6 +285,7 @@ bool RecoveryUI::impl_doRecovery() // create all needed dialogs for this operation // and bind it to the used core service ScopedVclPtrInstance<svxdr::RecoveryDialog> xDialog(m_pParentWindow, pCore); + DialogReleaseGuard dialogReleaseGuard(m_pDialog, xDialog); // start the dialog short nRet = xDialog->Execute(); @@ -301,6 +329,16 @@ void RecoveryUI::impl_showAllRecoveredDocs() } } +bool RecoveryUI::impl_doBringToFront() +{ + VclPtr<Dialog> 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 commit 7f7dd572d4fa4a46daff11c5a90655f689641758 Author: Mike Kaganski <mike.kagan...@collabora.com> Date: Sat Jul 15 11:37:22 2017 +0200 tdf#38915: don't wait on message queue if application already has quit. Despite precautions in Application::Execute() and ImplYield(), in my testing I sometimes see that soffice is waiting in ImplSalYield()'s GetMessageW() when ImplGetSVData()->maAppData.mbAppQuit is true, so that soffice.bin hangs in the background. I suspect that this is related to the bug. Some obscure code path seems to be able to get here after the flag is already set. So, test also in ImplSalYield() right before GetMessageW() to make sure. Another possibility is that we get here when the flag is not set yet, and gets set while already waiting, but that would mean this happens in a different thread. Change-Id: Idb19eabcca8b5c24eac0ca76950edc1bf1e5bccb Reviewed-on: https://gerrit.libreoffice.org/39996 Reviewed-by: Mike Kaganski <mike.kagan...@collabora.com> Tested-by: Mike Kaganski <mike.kagan...@collabora.com> (cherry picked from commit f054b9187155bc32b7d06808aea87127cb0a3a4f) Reviewed-on: https://gerrit.libreoffice.org/48219 Reviewed-by: Aron Budea <aron.bu...@collabora.com> Tested-by: Aron Budea <aron.bu...@collabora.com> (cherry picked from commit 1952332148ff351c21a198945fa76154d7e1f209) diff --git a/vcl/win/app/salinst.cxx b/vcl/win/app/salinst.cxx index da67ca44582f..293ce2c2cc0b 100644 --- a/vcl/win/app/salinst.cxx +++ b/vcl/win/app/salinst.cxx @@ -598,7 +598,8 @@ ImplSalYield( bool bWait, bool bHandleAllCurrentEvents ) bOneEvent = false; } while( --nMaxEvents && bOneEvent ); - if ( bWait && ! bWasMsg ) + // Also check that we don't wait when application already has quit + if ( bWait && !bWasMsg && !ImplGetSVData()->maAppData.mbAppQuit ) { if ( GetMessageW( &aMsg, nullptr, 0, 0 ) ) { _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits