common/SigUtil.cpp | 9 ++++++++- common/SigUtil.hpp | 7 +++++-- common/Unit.cpp | 2 +- kit/ForKit.cpp | 8 ++++---- kit/Kit.cpp | 18 +++++++++--------- wsd/Admin.cpp | 2 +- wsd/DocumentBroker.cpp | 8 ++++---- wsd/DocumentBroker.hpp | 2 +- wsd/LOOLWSD.cpp | 22 +++++++++++----------- wsd/SenderQueue.hpp | 6 +++--- 10 files changed, 47 insertions(+), 37 deletions(-)
New commits: commit bd4d72d41fcb3ed0b0b48344cee1999f5134b536 Author: Miklos Vajna <vmik...@collabora.com> AuthorDate: Thu Aug 8 09:10:59 2019 +0200 Commit: Miklos Vajna <vmik...@collabora.com> CommitDate: Thu Aug 8 09:10:59 2019 +0200 common: wrap TerminationFlag in a getter function to avoid ODR violation Otherwise both loolwsd and unit-copy-paste.so would have a TerminationFlag: ==11732==ERROR: AddressSanitizer: odr-violation (0x00000208f4a0): [1] size=1 'TerminationFlag' ../common/SigUtil.cpp:41:19 [2] size=1 'TerminationFlag' common/SigUtil.cpp:41:19 These globals were registered at these points: [1]: #0 0x5f9988 in __asan_register_globals.part.13 /home/vmiklos/git/libreoffice/lode/packages/llvm-472c6ef8b0f53061b049039f9775ab127beafbe4.src/compiler-rt/lib/asan/asan_globals.cc:365 #1 0x7f5df9cf18cb in asan.module_ctor (/home/vmiklos/git/libreoffice/online-san/test/../test/.libs/unit-copy-paste.so+0x60a8cb) [2]: #0 0x5f9988 in __asan_register_globals.part.13 /home/vmiklos/git/libreoffice/lode/packages/llvm-472c6ef8b0f53061b049039f9775ab127beafbe4.src/compiler-rt/lib/asan/asan_globals.cc:365 #1 0xe2b4fe in asan.module_ctor (/home/vmiklos/git/libreoffice/online-san/loolwsd+0xe2b4fe) Change-Id: Ic620b143ecb77699f40676ff39d0fa7abceb34d5 diff --git a/common/SigUtil.cpp b/common/SigUtil.cpp index df7809702..c5029a229 100644 --- a/common/SigUtil.cpp +++ b/common/SigUtil.cpp @@ -38,7 +38,14 @@ #include "Common.hpp" #include "Log.hpp" -std::atomic<bool> TerminationFlag(false); +static std::atomic<bool> TerminationFlag(false); +namespace SigUtil +{ + std::atomic<bool>& getTerminationFlag() + { + return TerminationFlag; + } +} std::atomic<bool> DumpGlobalState(false); #if MOBILEAPP diff --git a/common/SigUtil.hpp b/common/SigUtil.hpp index 39706372d..f3d620b92 100644 --- a/common/SigUtil.hpp +++ b/common/SigUtil.hpp @@ -20,8 +20,11 @@ extern std::atomic<bool> ShutdownRequestFlag; static constexpr bool ShutdownRequestFlag(false); #endif -/// Flag to stop pump loops. -extern std::atomic<bool> TerminationFlag; +namespace SigUtil +{ + /// Flag to stop pump loops. + std::atomic<bool>& getTerminationFlag(); +} /// Flag to dump internal state extern std::atomic<bool> DumpGlobalState; diff --git a/common/Unit.cpp b/common/Unit.cpp index c1848a62f..1dbe09645 100644 --- a/common/Unit.cpp +++ b/common/Unit.cpp @@ -189,7 +189,7 @@ void UnitBase::exitTest(TestResult result) _retValue = result == TestResult::Ok ? Poco::Util::Application::EXIT_OK : Poco::Util::Application::EXIT_SOFTWARE; - TerminationFlag = true; + SigUtil::getTerminationFlag() = true; SocketPoll::wakeupWorld(); } diff --git a/kit/ForKit.cpp b/kit/ForKit.cpp index bce34780f..81bff060e 100644 --- a/kit/ForKit.cpp +++ b/kit/ForKit.cpp @@ -77,13 +77,13 @@ public: bool pollAndDispatch() { std::string message; - const int ready = readLine(message, [](){ return TerminationFlag.load(); }); + const int ready = readLine(message, [](){ return SigUtil::getTerminationFlag().load(); }); if (ready <= 0) { // Termination is done via SIGTERM, which breaks the wait. if (ready < 0) { - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { LOG_INF("Poll interrupted in " << getName() << " and TerminationFlag is set."); } @@ -221,7 +221,7 @@ static void cleanupChildren() LOG_INF("Child " << exitedChildPid << " has exited, will remove its jail [" << it->second << "]."); jails.emplace_back(it->second); childJails.erase(it); - if (childJails.empty() && !TerminationFlag) + if (childJails.empty() && !SigUtil::getTerminationFlag()) { // We ran out of kits and we aren't terminating. LOG_WRN("No live Kits exist, and we are not terminating yet."); @@ -569,7 +569,7 @@ int main(int argc, char** argv) CommandDispatcher commandDispatcher(0); LOG_INF("ForKit process is ready."); - while (!TerminationFlag) + while (!SigUtil::getTerminationFlag()) { UnitKit::get().invokeForKitTest(); diff --git a/kit/Kit.cpp b/kit/Kit.cpp index a76b6d1f0..6f5bda553 100644 --- a/kit/Kit.cpp +++ b/kit/Kit.cpp @@ -1313,7 +1313,7 @@ public: static void GlobalCallback(const int type, const char* p, void* data) { - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { return; } @@ -1350,7 +1350,7 @@ public: static void ViewCallback(const int type, const char* p, void* data) { - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { return; } @@ -1993,7 +1993,7 @@ public: LOG_TRC("Kit Recv " << LOOLProtocol::getAbbreviatedMessage(input)); - if (_stop || TerminationFlag) + if (_stop || SigUtil::getTerminationFlag()) { LOG_INF("_stop or TerminationFlag is set, breaking out of loop"); break; @@ -2245,7 +2245,7 @@ protected: } // Note: Syntax or parsing errors here are unexpected and fatal. - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { LOG_DBG("Too late, TerminationFlag is set, we're going down"); } @@ -2273,7 +2273,7 @@ protected: else if (tokens[0] == "exit") { LOG_TRC("Setting TerminationFlag due to 'exit' command from parent."); - TerminationFlag = true; + SigUtil::getTerminationFlag() = true; document.reset(); } else if (tokens[0] == "tile" || tokens[0] == "tilecombine" || tokens[0] == "canceltiles" || @@ -2309,7 +2309,7 @@ protected: { #if !MOBILEAPP LOG_WRN("Kit connection lost without exit arriving from wsd. Setting TerminationFlag"); - TerminationFlag = true; + SigUtil::getTerminationFlag() = true; #endif } }; @@ -2345,7 +2345,7 @@ public: // returns the number of events signalled int kitPoll(int timeoutUs) { - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { LOG_TRC("Termination of unipoll mainloop flagged"); return -1; @@ -2378,7 +2378,7 @@ public: timeoutMs = std::chrono::duration_cast<std::chrono::milliseconds>(_pollEnd - now).count(); ++eventsSignalled; } - while (timeoutMs > 0 && !TerminationFlag && maxExtraEvents-- > 0); + while (timeoutMs > 0 && !SigUtil::getTerminationFlag() && maxExtraEvents-- > 0); } drainQueue(std::chrono::steady_clock::now()); @@ -2387,7 +2387,7 @@ public: if (document && document->purgeSessions() == 0) { LOG_INF("Last session discarded. Setting TerminationFlag"); - TerminationFlag = true; + SigUtil::getTerminationFlag() = true; return -1; } #endif diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp index 9d9fc2548..05babf1d2 100644 --- a/wsd/Admin.cpp +++ b/wsd/Admin.cpp @@ -389,7 +389,7 @@ void Admin::pollingThread() lastMem = lastCPU; lastNet = lastCPU; - while (!isStop() && !TerminationFlag && !ShutdownRequestFlag) + while (!isStop() && !SigUtil::getTerminationFlag() && !ShutdownRequestFlag) { const std::chrono::steady_clock::time_point now = std::chrono::steady_clock::now(); diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index be154d537..39e2be94c 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -231,7 +231,7 @@ void DocumentBroker::pollThread() // Nominal time between retries, lest we busy-loop. getNewChild could also wait, so don't double that here. std::this_thread::sleep_for(std::chrono::milliseconds(CHILD_REBALANCE_INTERVAL_MS / 10)); } - while (!_stop && _poll->continuePolling() && !TerminationFlag && !ShutdownRequestFlag); + while (!_stop && _poll->continuePolling() && !SigUtil::getTerminationFlag() && !ShutdownRequestFlag); #else _childProcess = getNewChild_Blocks(getPublicUri().getPath()); #endif @@ -283,7 +283,7 @@ void DocumentBroker::pollThread() auto last30SecCheckTime = std::chrono::steady_clock::now(); // Main polling loop goodness. - while (!_stop && _poll->continuePolling() && !TerminationFlag) + while (!_stop && _poll->continuePolling() && !SigUtil::getTerminationFlag()) { _poll->poll(SocketPoll::DefaultPollTimeoutMs); @@ -388,7 +388,7 @@ void DocumentBroker::pollThread() LOG_INF("Finished polling doc [" << _docKey << "]. stop: " << _stop << ", continuePolling: " << _poll->continuePolling() << ", ShutdownRequestFlag: " << ShutdownRequestFlag << - ", TerminationFlag: " << TerminationFlag << ", closeReason: " << _closeReason << ". Flushing socket."); + ", TerminationFlag: " << SigUtil::getTerminationFlag() << ", closeReason: " << _closeReason << ". Flushing socket."); if (_isModified) { @@ -412,7 +412,7 @@ void DocumentBroker::pollThread() LOG_INF("Finished flushing socket for doc [" << _docKey << "]. stop: " << _stop << ", continuePolling: " << _poll->continuePolling() << ", ShutdownRequestFlag: " << ShutdownRequestFlag << - ", TerminationFlag: " << TerminationFlag << ". Terminating child with reason: [" << _closeReason << "]."); + ", TerminationFlag: " << SigUtil::getTerminationFlag() << ". Terminating child with reason: [" << _closeReason << "]."); // Terminate properly while we can. terminateChild(_closeReason); diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp index b50f6d706..fe370cdd6 100644 --- a/wsd/DocumentBroker.hpp +++ b/wsd/DocumentBroker.hpp @@ -48,7 +48,7 @@ public: bool continuePolling() override { - return SocketPoll::continuePolling() && !TerminationFlag; + return SocketPoll::continuePolling() && !SigUtil::getTerminationFlag(); } }; diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp index 1e7febb0b..fcf25e9cd 100644 --- a/wsd/LOOLWSD.cpp +++ b/wsd/LOOLWSD.cpp @@ -1406,7 +1406,7 @@ bool LOOLWSD::checkAndRestoreForKit() if (ForKitProcId == -1) { // Fire the ForKit process for the first time. - if (!ShutdownRequestFlag && !TerminationFlag && !createForKit()) + if (!ShutdownRequestFlag && !SigUtil::getTerminationFlag() && !createForKit()) { // Should never fail. LOG_FTL("Failed to spawn loolforkit."); @@ -1435,7 +1435,7 @@ bool LOOLWSD::checkAndRestoreForKit() } // Spawn a new forkit and try to dust it off and resume. - if (!ShutdownRequestFlag && !TerminationFlag && !createForKit()) + if (!ShutdownRequestFlag && !SigUtil::getTerminationFlag() && !createForKit()) { LOG_FTL("Failed to spawn forkit instance. Shutting down."); SigUtil::requestShutdown(); @@ -1469,7 +1469,7 @@ bool LOOLWSD::checkAndRestoreForKit() { // No child processes. // Spawn a new forkit and try to dust it off and resume. - if (!ShutdownRequestFlag && !TerminationFlag && !createForKit()) + if (!ShutdownRequestFlag && !SigUtil::getTerminationFlag() && !createForKit()) { LOG_FTL("Failed to spawn forkit instance. Shutting down."); SigUtil::requestShutdown(); @@ -1547,7 +1547,7 @@ void PrisonerPoll::wakeupHook() // block until the replay finishes replayThread->join(); - TerminationFlag = true; + SigUtil::getTerminationFlag() = true; } #endif } @@ -1676,7 +1676,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w cleanupDocBrokers(); - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { LOG_ERR("TerminationFlag set. Not loading new session [" << id << "]"); return nullptr; @@ -1706,7 +1706,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w LOG_DBG("No DocumentBroker with docKey [" << docKey << "] found. New Child and Document."); } - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) { LOG_ERR("TerminationFlag is set. Not loading new session [" << id << "]"); return nullptr; @@ -3100,7 +3100,7 @@ public: << " Security " << (LOOLWSD::NoCapsForKit ? "no" : "") << " chroot, " << (LOOLWSD::NoSeccomp ? "no" : "") << " api lockdown\n" #endif - << " TerminationFlag: " << TerminationFlag << "\n" + << " TerminationFlag: " << SigUtil::getTerminationFlag() << "\n" << " isShuttingDown: " << ShutdownRequestFlag << "\n" << " NewChildren: " << NewChildren.size() << "\n" << " OutstandingForks: " << OutstandingForks << "\n" @@ -3399,7 +3399,7 @@ int LOOLWSD::innerMain() const auto startStamp = std::chrono::steady_clock::now(); - while (!TerminationFlag && !ShutdownRequestFlag) + while (!SigUtil::getTerminationFlag() && !ShutdownRequestFlag) { UnitWSD::get().invokeTest(); @@ -3430,14 +3430,14 @@ int LOOLWSD::innerMain() // Stop the listening to new connections // and wait until sockets close. LOG_INF("Stopping server socket listening. ShutdownRequestFlag: " << - ShutdownRequestFlag << ", TerminationFlag: " << TerminationFlag); + ShutdownRequestFlag << ", TerminationFlag: " << SigUtil::getTerminationFlag()); // Wait until documents are saved and sessions closed. srv.stop(); // atexit handlers tend to free Admin before Documents LOG_INF("Cleaning up lingering documents."); - if (ShutdownRequestFlag || TerminationFlag) + if (ShutdownRequestFlag || SigUtil::getTerminationFlag()) { // Don't stop the DocBroker, they will exit. const size_t sleepMs = 300; @@ -3528,7 +3528,7 @@ void LOOLWSD::cleanup() int LOOLWSD::main(const std::vector<std::string>& /*args*/) { #if MOBILEAPP - TerminationFlag = false; + SigUtil::getTerminationFlag() = false; #endif int returnValue; diff --git a/wsd/SenderQueue.hpp b/wsd/SenderQueue.hpp index 251be7df3..3f08c6286 100644 --- a/wsd/SenderQueue.hpp +++ b/wsd/SenderQueue.hpp @@ -40,7 +40,7 @@ public: { std::unique_lock<std::mutex> lock(_mutex); - if (!TerminationFlag && deduplicate(item)) + if (!SigUtil::getTerminationFlag() && deduplicate(item)) _queue.push_back(item); return _queue.size(); @@ -51,7 +51,7 @@ public: { std::unique_lock<std::mutex> lock(_mutex); - if (!_queue.empty() && !TerminationFlag) + if (!_queue.empty() && !SigUtil::getTerminationFlag()) { item = _queue.front(); _queue.pop_front(); @@ -59,7 +59,7 @@ public: } else { - if (TerminationFlag) + if (SigUtil::getTerminationFlag()) LOG_DBG("SenderQueue: TerminationFlag is set"); return false; } _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits