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.nakash...@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 <ashnak...@gmail.com> Tested-by: Ashod Nakashian <ashnak...@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 _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits