[Libreoffice-commits] online.git: loolwsd/DocumentBroker.cpp loolwsd/Util.hpp

[Michael Meeks]

 loolwsd/DocumentBroker.cpp |   39 ++++++++++++++++++++-------------------
 loolwsd/Util.hpp           |    7 +++++++
 2 files changed, 27 insertions(+), 19 deletions(-)

New commits:
commit 8d4c5d4e68eaf196a9e6c7478fbee15285beac04
Author: Michael Meeks <michael.meeks at collabora.com>
Date:   Tue Oct 18 21:51:11 2016 +0100

    Avoid deadlock when notifying users of document load failure.

diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 624b2be..3adc1ad 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -410,30 +410,30 @@ size_t DocumentBroker::addSession(std::shared_ptr<ClientSession>& session)
     const auto id = session->getId();
     const std::string aMessage = "session " + id + " " + _docKey + "\n";
 
-    std::lock_guard<std::mutex> lock(_mutex);
+    try
+    {
+        std::lock_guard<std::mutex> lock(_mutex);
 
-    // Request a new session from the child kit.
-    Log::debug("DocBroker to Child: " + aMessage.substr(0, aMessage.length() - 1));
-    _childProcess->sendTextFrame(aMessage);
+        // Request a new session from the child kit.
+        Log::debug("DocBroker to Child: " + aMessage.substr(0, aMessage.length() - 1));
+        _childProcess->sendTextFrame(aMessage);
 
-    auto ret = _sessions.emplace(id, session);
-    if (!ret.second)
-    {
-        Log::warn("DocumentBroker: Trying to add already existing session.");
-    }
+        auto ret = _sessions.emplace(id, session);
+        if (!ret.second)
+        {
+            Log::warn("DocumentBroker: Trying to add already existing session.");
+        }
 
-    if (session->isReadOnly())
-    {
-        Log::debug("Adding a readonly session [" + id + "]");
-    }
+        if (session->isReadOnly())
+        {
+            Log::debug("Adding a readonly session [" + id + "]");
+        }
 
-    // Below values are recalculated when startDestroy() is called (before destroying the
-    // document). It is safe to reset their values to their defaults whenever a new session is added.
-    _lastEditableSession = false;
-    _markToDestroy = false;
+        // Below values are recalculated when startDestroy() is called (before destroying the
+        // document). It is safe to reset their values to their defaults whenever a new session is added.
+        _lastEditableSession = false;
+        _markToDestroy = false;
 
-    try
-    {
         load(id, std::to_string(_childProcess->getPid()));
     }
     catch (const StorageSpaceLowException&)
@@ -491,6 +491,7 @@ size_t DocumentBroker::removeSession(const std::string& id)
 
 void DocumentBroker::alertAllUsersOfDocument(const std::string& cmd, const std::string& kind)
 {
+    Util::assertIsNotLocked(_mutex);
     std::lock_guard<std::mutex> lock(_mutex);
 
     std::stringstream ss;
diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp
index a55c70e..d99fd36 100644
--- a/loolwsd/Util.hpp
+++ b/loolwsd/Util.hpp
@@ -106,6 +106,13 @@ namespace Util
         assert(!mtx.try_lock());
     }
 
+    inline
+    void assertIsNotLocked(std::mutex& mtx)
+    {
+        assert(mtx.try_lock());
+        mtx.unlock();
+    }
+
     /// Safely remove a file or directory.
     /// Supresses exception when the file is already removed.
     /// This can happen when there is a race (unavoidable) or when