loolwsd/DocumentBroker.cpp | 23 +++++++++++++++++++++++ loolwsd/DocumentBroker.hpp | 7 +++++++ loolwsd/LOOLWSD.cpp | 25 ++++++++++++++++++------- 3 files changed, 48 insertions(+), 7 deletions(-)
New commits: commit 800e711321e19b02f5e15f496525d42edcba4376 Author: Ashod Nakashian <[email protected]> Date: Sat Nov 5 23:00:16 2016 -0400 loolwsd: proper ChildProcess cleanup Change-Id: If9be827aa50471b7d1d922402414d028ccdcff8b Reviewed-on: https://gerrit.libreoffice.org/30629 Reviewed-by: Ashod Nakashian <[email protected]> Tested-by: Ashod Nakashian <[email protected]> diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp index 19dbb97..507362c 100644 --- a/loolwsd/DocumentBroker.cpp +++ b/loolwsd/DocumentBroker.cpp @@ -180,6 +180,10 @@ DocumentBroker::~DocumentBroker() { LOG_WRN("DocumentBroker still has unremoved sessions."); } + + // Need to first make sure the child exited, socket closed, + // and thread finished before we are destroyed. + _childProcess.reset(); } bool DocumentBroker::load(const std::string& sessionId, const std::string& jailId) @@ -922,4 +926,23 @@ void DocumentBroker::childSocketTerminated() } } +void DocumentBroker::terminateChild(std::unique_lock<std::mutex>& lock) +{ + Util::assertIsLocked(_mutex); + Util::assertIsLocked(lock); + + Log::info() << "Terminating child [" << getPid() << "] of doc [" << _docKey << "]." << Log::end; + + assert(_sessions.empty() && "DocumentBroker still has unremoved sessions!"); + + // First flag to stop as it might be waiting on our lock + // to process some incoming message. + _childProcess->stop(); + + // Release the lock and wait for the thread to finish. + lock.unlock(); + + _childProcess->close(false); +} + /* vim:set shiftwidth=4 softtabstop=4 expandtab: */ diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp index e58b642..0075fb6 100644 --- a/loolwsd/DocumentBroker.hpp +++ b/loolwsd/DocumentBroker.hpp @@ -270,6 +270,13 @@ public: /// or upon failing to process an incoming message. void childSocketTerminated(); + /// This gracefully terminates the connection + /// with the child and cleans up ChildProcess etc. + /// We must be called under lock and it must be + /// passed to us so we unlock before waiting on + /// the ChildProcess thread, which can take our lock. + void terminateChild(std::unique_lock<std::mutex>& lock); + /// Get the PID of the associated child process Poco::Process::PID getPid() const { return _childProcess->getPid(); } diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp index 7ec35a1..ed5810a 100644 --- a/loolwsd/LOOLWSD.cpp +++ b/loolwsd/LOOLWSD.cpp @@ -565,11 +565,9 @@ private: if (sessionsCount == 0) { // At this point we're done. - // We can't save if we hadn't, just kill. - Log::debug("Closing child for docKey [" + docKey + "]."); - child->close(true); - Log::debug("Removing DocumentBroker for docKey [" + docKey + "]."); + LOG_DBG("Removing DocumentBroker for docKey [" << docKey << "]."); DocBrokers.erase(docKey); + docBroker->terminateChild(docLock); } else { @@ -928,9 +926,22 @@ private: if (sessionsCount == 0) { - std::unique_lock<std::mutex> DocBrokersLock(DocBrokersMutex); - Log::debug("Removing DocumentBroker for docKey [" + docKey + "]."); - DocBrokers.erase(docKey); + // We've supposedly destroyed the last session and can do away with + // DocBroker. But first we need to take both locks in the correct + // order and check again. We can't take the DocBrokersMutex while + // holding the docBroker lock as that can deadlock with autoSave below. + std::unique_lock<std::mutex> docBrokersLock2(DocBrokersMutex); + it = DocBrokers.find(docKey); + if (it != DocBrokers.end() && it->second) + { + auto lock = it->second->getLock(); + if (it->second->getSessionsCount() == 0) + { + Log::info("Removing DocumentBroker for docKey [" + docKey + "]."); + DocBrokers.erase(docKey); + docBroker->terminateChild(lock); + } + } } LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "EndSession: " + uri); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
