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

[Ashod Nakashian]

 loolwsd/LOOLWSD.cpp         |   30 ++++++++++++++++++++++--------
 loolwsd/common/FileUtil.cpp |    9 +++++----
 loolwsd/common/FileUtil.hpp |    2 +-
 3 files changed, 28 insertions(+), 13 deletions(-)

New commits:
commit 51c88c5fb79c5db146a08f5ef2d9a4fe6d095caa
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date:   Tue Nov 22 22:06:50 2016 -0500

    loolwsd: avoid deadlocking when alerting all users
    
    Alerting all users is done from different contexts.
    One such is when loading a new document.
    
    Since both alerting all users and loading documents
    need to hold the same lock, this would deadlock.
    
    The solution here is to differentiate between
    external alerts and internal ones (to WSD).
    
    The internal one expects to be invoked while holding
    the lock, while the external one always takes the lock.
    
    Care should be taking when alerting from within WSD to
    avoid this deadlock.
    
    Change-Id: Idf0e952db1216a3d161f22c7da51af16701f685b
    Reviewed-on: https://gerrit.libreoffice.org/31102
    Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
    Tested-by: Ashod Nakashian <ashnakash at gmail.com>

diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 1f8644f..c253f51 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -228,6 +228,20 @@ void shutdownLimitReached(LOOLWebSocket& ws)
     }
 }
 
+/// Internal implementation to alert all clients
+/// connected to any document.
+void alertAllUsersInternal(const std::string& msg)
+{
+    Util::assertIsLocked(DocBrokersMutex);
+
+    LOG_INF("Alerting all users: [" << msg << "]");
+
+    for (auto& brokerIt : DocBrokers)
+    {
+        auto lock = brokerIt.second->getLock();
+        brokerIt.second->alertAllUsers(msg);
+    }
+}
 }
 
 /// Remove dead DocBrokers.
@@ -260,7 +274,13 @@ static void forkChildren(const int number)
 
     if (number > 0)
     {
-        FileUtil::checkDiskSpaceOnRegisteredFileSystems();
+        const std::string fs = FileUtil::checkDiskSpaceOnRegisteredFileSystems();
+        if (!fs.empty())
+        {
+            LOG_WRN("File system of " << fs << " dangerously low on disk space");
+            alertAllUsersInternal("error: cmd=internal kind=diskfull");
+        }
+
         const std::string aMessage = "spawn " + std::to_string(number) + "\n";
         LOG_DBG("MasterToForKit: " << aMessage.substr(0, aMessage.length() - 1));
 
@@ -2155,13 +2175,7 @@ void alertAllUsers(const std::string& msg)
 {
     std::lock_guard<std::mutex> DocBrokersLock(DocBrokersMutex);
 
-    LOG_INF("Alerting all users: [" << msg << "]");
-
-    for (auto& brokerIt : DocBrokers)
-    {
-        auto lock = brokerIt.second->getLock();
-        brokerIt.second->alertAllUsers(msg);
-    }
+    alertAllUsersInternal(msg);
 }
 
 }
diff --git a/loolwsd/common/FileUtil.cpp b/loolwsd/common/FileUtil.cpp
index 19e8e00..ea55548 100644
--- a/loolwsd/common/FileUtil.cpp
+++ b/loolwsd/common/FileUtil.cpp
@@ -163,7 +163,7 @@ namespace FileUtil
         }
     }
 
-    void checkDiskSpaceOnRegisteredFileSystems()
+    std::string checkDiskSpaceOnRegisteredFileSystems()
     {
         std::lock_guard<std::mutex> lock(fsmutex);
 
@@ -172,7 +172,7 @@ namespace FileUtil
 
         // Don't check more often that once a minute
         if (std::chrono::duration_cast<std::chrono::seconds>(now - lastCheck).count() < 60)
-            return;
+            return std::string();
 
         lastCheck = now;
 
@@ -180,10 +180,11 @@ namespace FileUtil
         {
             if (!checkDiskSpace(i.path))
             {
-                alertAllUsersAndLog("File system of " + i.path + " dangerously low on disk space", "internal", "diskfull");
-                break;
+                return i.path;
             }
         }
+
+        return std::string();
     }
 
     bool checkDiskSpace(const std::string& path)
diff --git a/loolwsd/common/FileUtil.hpp b/loolwsd/common/FileUtil.hpp
index aa6bba6..e0e9487 100644
--- a/loolwsd/common/FileUtil.hpp
+++ b/loolwsd/common/FileUtil.hpp
@@ -51,7 +51,7 @@ namespace FileUtil
     // Perform the check. If the free space on any of the registered file systems is below 5%, call
     // 'alertAllUsers("internal", "diskfull")'. The check will be made no more often than once a
     // minute.
-    void checkDiskSpaceOnRegisteredFileSystems();
+    std::string checkDiskSpaceOnRegisteredFileSystems();
 
     // Check disk space on a specific file system, the one where 'path' is located. This does not
     // add that file system to the list used by 'registerFileSystemForDiskSpaceChecks'. If the free