[Libreoffice-commits] online.git: wsd/ClientSession.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp

[Ashod Nakashian]

 wsd/ClientSession.cpp  |    2 +-
 wsd/DocumentBroker.cpp |   30 ++++++++++++++++++------------
 wsd/DocumentBroker.hpp |    2 +-
 wsd/LOOLWSD.cpp        |    4 ++--
 4 files changed, 22 insertions(+), 16 deletions(-)

New commits:
commit b5baf3672fab052e5d7457fdb3a8c60c8fa70107
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Sun Jan 14 20:49:11 2018 -0500

    wsd: stop DocBroker using only stop() member
    
    Now always given a proper reason too.
    
    Also, stop polling thread and cleanup when
    failing to acquire/spawn a child process.
    
    Change-Id: I7ddee01dd47b8ee72f2d9134c0f1b264634d8611
    Reviewed-on: https://gerrit.libreoffice.org/47886
    Reviewed-by: Michael Meeks <michael.meeks at collabora.com>
    Tested-by: Michael Meeks <michael.meeks at collabora.com>

diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 061504c3..f644ae2c 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -716,7 +716,7 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
             docBroker->removeSession(getId());
 
             // Now terminate.
-            docBroker->stop();
+            docBroker->stop("Finished saveas handler.");
         }
 
         return true;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 44574aab..28a4920d 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -198,7 +198,7 @@ void DocumentBroker::pollThread()
                                                                   _threadStart).count() > timeoutMs)
             break;
 
-        // Nominal time between retries, lest we  busy-loop. getNewChild could also wait, so don't double that here.
+        // Nominal time between retries, lest we busy-loop. getNewChild could also wait, so don't double that here.
         std::this_thread::sleep_for(std::chrono::milliseconds(CHILD_REBALANCE_INTERVAL_MS / 10));
     }
     while (!_stop && _poll->continuePolling() && !TerminationFlag && !ShutdownRequestFlag);
@@ -209,14 +209,21 @@ void DocumentBroker::pollThread()
         LOG_ERR("Failed to get new child.");
 
         // FIXME: need to notify all clients and shut this down ...
+        // FIXME: return something good down the websocket ...
 #if 0
         const std::string msg = SERVICE_UNAVAILABLE_INTERNAL_ERROR;
         ws.sendMessage(msg);
         // abnormal close frame handshake
         ws.shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY);
 #endif
-        // FIXME: return something good down the websocket ...
-        _stop = true;
+        stop("Failed to get new child.");
+
+        // Stop to mark it done and cleanup.
+        _poll->stop();
+        _poll->removeSockets();
+
+        // Async cleanup.
+        LOOLWSD::doHousekeeping();
 
         LOG_INF("Finished docBroker polling thread for docKey [" << _docKey << "].");
         return;
@@ -269,8 +276,7 @@ void DocumentBroker::pollThread()
         if (ShutdownRequestFlag)
         {
             autoSave(true);
-            _closeReason = "recycling";
-            _stop = true;
+            stop("recycling");
         }
         else if (AutoSaveEnabled && !_stop &&
                  std::chrono::duration_cast<std::chrono::seconds>(now - last30SecCheckTime).count() >= 30)
@@ -288,8 +294,7 @@ void DocumentBroker::pollThread()
         {
             LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
                     " DocumentBroker for docKey [" << getDocKey() << "].");
-            _closeReason = (idle ? "idle" : "dead");
-            _stop = true;
+            stop(idle ? "idle" : "dead");
         }
     }
 
@@ -374,8 +379,10 @@ void DocumentBroker::joinThread()
     _poll->joinThread();
 }
 
-void DocumentBroker::stop()
+void DocumentBroker::stop(const std::string& reason)
 {
+    LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with reason: " << reason);
+    _closeReason = reason; // used later in the polling loop
     _stop = true;
     _poll->wakeup();
 }
@@ -1553,7 +1560,7 @@ void DocumentBroker::terminateChild(const std::string& closeReason)
 
     LOG_INF("Terminating doc [" << _docKey << "] with reason: " << closeReason);
 
-    // Close all running sessions
+    // Close all running sessions first.
     shutdownClients(closeReason);
 
     if (_childProcess)
@@ -1566,7 +1573,7 @@ void DocumentBroker::terminateChild(const std::string& closeReason)
         _childProcess->close(false);
     }
 
-    _stop = true;
+    stop(closeReason);
 }
 
 void DocumentBroker::closeDocument(const std::string& reason)
@@ -1574,8 +1581,7 @@ void DocumentBroker::closeDocument(const std::string& reason)
     assertCorrectThread();
 
     LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with reason: " << reason);
-    _closeReason = reason; // used later in the polling loop
-    stop();
+    stop(reason);
 }
 
 void DocumentBroker::broadcastMessage(const std::string& message)
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 0b8153ad..1e8e0746 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -227,7 +227,7 @@ public:
     void startThread();
 
     /// Flag for termination.
-    void stop();
+    void stop(const std::string& reason);
 
     /// Thread safe termination of this broker if it has a lingering thread
     void joinThread();
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 0750a0b2..94b4c0d3 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1528,7 +1528,7 @@ public:
         if (docBroker)
         {
             // FIXME: No need to notify if asked to stop.
-            docBroker->stop();
+            docBroker->stop("Request dispatcher destroyed.");
         }
     }
 
@@ -1556,7 +1556,7 @@ private:
             auto lock = docBroker->getLock();
             docBroker->assertCorrectThread();
             docBroker->setCloseReason("docdisconnected");
-            docBroker->stop();
+            docBroker->stop("Lost connection with LOKit.");
         }
     }