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 <ci@libreoffice.org>
Reviewed-by: Aron Budea <aron.budea@collabora.com>
Reviewed-by: Mike Kaganski <mike.kaganski@collabora.com>

4 files changed, 85 insertions, 7 deletions

diff --git a/desktop/source/app/app.cxx b/desktop/source/app/app.cxx
index f75fcac00ab8..c95674d50211 100644
--- a/desktop/source/app/app.cxx
+++ b/desktop/source/app/app.cxx
@@ -982,6 +982,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.
 
@@ -996,12 +1005,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)
@@ -1013,6 +1023,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;
@@ -2274,7 +2302,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 5bfd5ed80a23..031e027f9473 100644
--- a/desktop/source/app/officeipcthread.cxx
+++ b/desktop/source/app/officeipcthread.cxx
@@ -1310,6 +1310,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)
@@ -1317,6 +1323,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
@@ -1334,7 +1343,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() )
@@ -1352,10 +1367,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 c74e20cce9db..a3f15b15f9ed 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 afbf6456c3d8..062ca7eb9ed9 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<Dialog> 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<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.
@@ -223,6 +249,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.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<svxdr::RecoveryDialog> 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<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 *