wsd/DocumentBroker.cpp | 2 - wsd/LOOLWSD.cpp | 63 +++++++++++++++++++++++++++++++------------------ 2 files changed, 41 insertions(+), 24 deletions(-)
New commits: commit 5ce8d84049d87dbbce2c7197e7319e81053758f5 Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk> AuthorDate: Sun Nov 3 15:03:11 2019 -0500 Commit: Andras Timar <andras.ti...@collabora.com> CommitDate: Mon Nov 4 09:36:10 2019 +0100 wsd: improved shutdown cleanup Change-Id: Ibdb822575c376af6065080070bf6b89c240ce67b Reviewed-on: https://gerrit.libreoffice.org/81968 Tested-by: Jenkins CollaboraOffice <jenkinscollaboraoff...@gmail.com> Reviewed-by: Andras Timar <andras.ti...@collabora.com> diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 6650593b7..32ae37ef4 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -451,7 +451,7 @@ void DocumentBroker::joinThread() void DocumentBroker::stop(const std::string& reason) { - LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with reason: " << reason); + LOG_DBG("Stopping DocumentBroker for docKey [" << _docKey << "] with reason: " << reason); _closeReason = reason; // used later in the polling loop _stop = true; _poll->wakeup(); diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index ba9a6f94f..35fc79c48 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -3265,8 +3265,8 @@ int LOOLWSD::innerMain() #if ENABLE_DEBUG if (careerSpanMs > 0 && timeSinceStartMs > careerSpanMs) { - LOG_INF(timeSinceStartMs << " milliseconds gone, finishing as requested."); - break; + LOG_INF(timeSinceStartMs << " milliseconds gone, finishing as requested. Setting ShutdownRequestFlag."); + ShutdownRequestFlag = true; } #endif } @@ -3279,36 +3279,53 @@ int LOOLWSD::innerMain() srv.stop(); // atexit handlers tend to free Admin before Documents - LOG_INF("Cleaning up lingering documents."); - if (ShutdownRequestFlag || TerminationFlag) + LOG_INF("Exiting. Cleaning up lingering documents."); + if (!ShutdownRequestFlag) { - // Don't stop the DocBroker, they will exit. - const size_t sleepMs = 300; - const size_t count = std::max<size_t>(COMMAND_TIMEOUT_MS, 2000) / sleepMs; - for (size_t i = 0; i < count; ++i) - { - std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); - cleanupDocBrokers(); - if (DocBrokers.empty()) - break; - docBrokersLock.unlock(); - - // Give them time to save and cleanup. - std::this_thread::sleep_for(std::chrono::milliseconds(sleepMs)); - } + // This shouldn't happen, but it's fail safe to always cleanup properly. + LOG_WRN("Exiting WSD without ShutdownRequestFlag. Setting it now."); + ShutdownRequestFlag = true; } - else + + // Don't stop the DocBroker, they will exit. + constexpr size_t sleepMs = 500; + constexpr size_t count = (COMMAND_TIMEOUT_MS * 4) / sleepMs; + for (size_t i = 0; i < count; ++i) { - // Stop and join. - for (auto& docBrokerIt : DocBrokers) - docBrokerIt.second->joinThread(); + std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); + LOG_DBG("Waiting for " << DocBrokers.size() << " documents to stop."); + cleanupDocBrokers(); + if (DocBrokers.empty()) + break; + docBrokersLock.unlock(); + + // Give them time to save and cleanup. + std::this_thread::sleep_for(std::chrono::milliseconds(sleepMs)); } // Disable thread checking - we'll now cleanup lots of things if we can Socket::InhibitThreadChecks = true; SocketPoll::InhibitThreadChecks = true; - DocBrokers.clear(); + // Wait for the DocumentBrokers. They must be saving/uploading now. + // Do not stop them! Otherwise they might not save/upload the document. + // We block until they finish, or the service stopping times out. + { + std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex); + for (auto& docBrokerIt : DocBrokers) + { + std::shared_ptr<DocumentBroker> docBroker = docBrokerIt.second; + if (docBroker && docBroker->isAlive()) + { + LOG_DBG("Joining docBroker [" << docBrokerIt.first << "]."); + docBroker->joinThread(); + } + } + + // Now should be safe to destroy what's left. + cleanupDocBrokers(); + DocBrokers.clear(); + } #if !defined(KIT_IN_PROCESS) && !defined(MOBILEAPP) // Terminate child processes _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits