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

[Ashod Nakashian]

 wsd/LOOLWSD.cpp |   23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

New commits:
commit a652e6b583b2779d83745637bea0b989945917f6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Thu Dec 1 22:04:43 2016 -0500

    loolwsd: guarantee DocBrokersMutex locked when alerting
    
    We now guarantee that forkChildren will be invoked
    under DocBrokersMutex lock.
    
    This eliminates the case when alertAllUsersInternal
    is invoked when this mutex isn't locked.
    
    Change-Id: Ibb259bbb4f380300a90ad2fc7affe6013dd71fef

diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 3e56c67..1305576 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -283,6 +283,7 @@ bool cleanupDocBrokers()
 
 static void forkChildren(const int number)
 {
+    Util::assertIsLocked(DocBrokersMutex);
     Util::assertIsLocked(NewChildrenMutex);
 
     if (number > 0)
@@ -345,12 +346,8 @@ static void preForkChildren()
 static void prespawnChildren()
 {
     // First remove dead DocBrokers, if possible.
-    std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex, std::defer_lock);
-    if (docBrokersLock.try_lock())
-    {
-        cleanupDocBrokers();
-        docBrokersLock.unlock();
-    }
+    std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
+    cleanupDocBrokers();
 
     std::unique_lock<std::mutex> lock(NewChildrenMutex, std::defer_lock);
     if (!lock.try_lock())
@@ -397,6 +394,7 @@ static size_t addNewChild(const std::shared_ptr<ChildProcess>& child)
 
 static std::shared_ptr<ChildProcess> getNewChild()
 {
+    Util::assertIsLocked(DocBrokersMutex);
     std::unique_lock<std::mutex> lock(NewChildrenMutex);
 
     namespace chrono = std::chrono;
@@ -531,6 +529,13 @@ private:
                 {
                     LOG_INF("Conversion request for URI [" << fromPath << "].");
 
+                    auto uriPublic = DocumentBroker::sanitizeURI(fromPath);
+                    const auto docKey = DocumentBroker::getDocKey(uriPublic);
+
+                    // This lock could become a bottleneck.
+                    // In that case, we can use a pool and index by publicPath.
+                    std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
+
                     // Request a kit process for this doc.
                     auto child = getNewChild();
                     if (!child)
@@ -539,16 +544,10 @@ private:
                         throw std::runtime_error("Failed to spawn lokit child.");
                     }
 
-                    auto uriPublic = DocumentBroker::sanitizeURI(fromPath);
-                    const auto docKey = DocumentBroker::getDocKey(uriPublic);
                     LOG_DBG("New DocumentBroker for docKey [" << docKey << "].");
                     auto docBroker = std::make_shared<DocumentBroker>(uriPublic, docKey, LOOLWSD::ChildRoot, child);
                     child->setDocumentBroker(docBroker);
 
-                    // This lock could become a bottleneck.
-                    // In that case, we can use a pool and index by publicPath.
-                    std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
-
                     cleanupDocBrokers();
 
                     // FIXME: What if the same document is already open? Need a fake dockey here?