common/FileUtil.cpp | 5 +++++ common/Unit.hpp | 7 +++++++ net/Socket.cpp | 24 ++++++++++++++++-------- net/Socket.hpp | 3 +++ test/Makefile.am | 2 +- test/UnitStorage.cpp | 44 ++++++++++++++++++++++++++++++-------------- wsd/DocumentBroker.cpp | 3 +++ 7 files changed, 65 insertions(+), 23 deletions(-)
New commits: commit aeb204fb1480ac269fe4f0e9dac34b34aaf01e8b Author: Michael Meeks <michael.me...@collabora.com> Date: Fri Mar 31 17:28:20 2017 +0100 Kill race during DocumentBroker shutdown over child process. ==20033== Invalid read of size 4 ==20033== at 0x466504: ChildProcess::close(bool) (DocumentBroker.hpp:111) ==20033== by 0x44EA28: DocumentBroker::terminateChild(std::string const&, bool) (DocumentBroker.cpp:1313) ==20033== by 0x45F70E: DocumentBroker::pollThread() (DocumentBroker.cpp:264) ==20033== by 0x504B2F: SocketPoll::pollingThreadEntry() (Socket.hpp:486) ==20033== by 0x7310E6F: execute_native_thread_routine (thread.cc:84) ==20033== by 0x7AF60A3: start_thread (pthread_create.c:309) ==20033== by 0x7DF002C: clone (clone.S:111) ==20033== Address 0x0 is not stack'd, malloc'd or (recently) free'd diff --git a/net/Socket.cpp b/net/Socket.cpp index fdeb8677..39906bf7 100644 --- a/net/Socket.cpp +++ b/net/Socket.cpp @@ -61,14 +61,7 @@ SocketPoll::SocketPoll(const std::string& threadName) SocketPoll::~SocketPoll() { - stop(); - if (_threadStarted && _thread.joinable()) - { - if (_thread.get_id() == std::this_thread::get_id()) - LOG_ERR("DEADLOCK PREVENTED: joining own thread!"); - else - _thread.join(); - } + joinThread(); { std::lock_guard<std::mutex> lock(getPollWakeupsMutex()); @@ -104,6 +97,21 @@ void SocketPoll::startThread() } } +void SocketPoll::joinThread() +{ + stop(); + if (_threadStarted && _thread.joinable()) + { + if (_thread.get_id() == std::this_thread::get_id()) + LOG_ERR("DEADLOCK PREVENTED: joining own thread!"); + else + { + _thread.join(); + _threadStarted = false; + } + } +} + void SocketPoll::wakeupWorld() { for (const auto& fd : getWakeupsArray()) diff --git a/net/Socket.hpp b/net/Socket.hpp index c45caf77..4d723d99 100644 --- a/net/Socket.hpp +++ b/net/Socket.hpp @@ -451,6 +451,9 @@ public: /// Start the polling thread (if desired) void startThread(); + /// Stop and join the polling thread before returning (if active) + void joinThread(); + private: /// Initialize the poll fds array with the right events void setupPollFds(std::chrono::steady_clock::time_point now, diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp index 7894805c..0a6d2f01 100644 --- a/wsd/DocumentBroker.cpp +++ b/wsd/DocumentBroker.cpp @@ -295,6 +295,9 @@ DocumentBroker::~DocumentBroker() LOG_INF("~DocumentBroker [" << _uriPublic.toString() << "] destroyed with " << _sessions.size() << " sessions left."); + // Do this early - to avoid operating on _childProcess from two threads. + _poll->joinThread(); + if (!_sessions.empty()) { LOG_WRN("DocumentBroker still has unremoved sessions."); commit 2bca560feb3fa31ee341347ec5c91c788c7c1ed7 Author: Michael Meeks <michael.me...@collabora.com> Date: Fri Mar 31 17:18:41 2017 +0100 Add hook for disk space check. diff --git a/common/FileUtil.cpp b/common/FileUtil.cpp index 02d2e705..af6dfef5 100644 --- a/common/FileUtil.cpp +++ b/common/FileUtil.cpp @@ -27,6 +27,7 @@ #include "Log.hpp" #include "Util.hpp" +#include "Unit.hpp" namespace { @@ -233,6 +234,10 @@ namespace FileUtil { assert(!path.empty()); + bool hookResult; + if (UnitBase::get().filterCheckDiskSpace(path, hookResult)) + return hookResult; + struct statfs sfs; if (statfs(path.c_str(), &sfs) == -1) return true; diff --git a/common/Unit.hpp b/common/Unit.hpp index 69d0b3bc..e8197fd1 100644 --- a/common/Unit.hpp +++ b/common/Unit.hpp @@ -95,6 +95,13 @@ public: return false; } + /// Hook the disk space check + virtual bool filterCheckDiskSpace(const std::string & /* path */, + bool & /* newResult */) + { + return false; + } + /// If the test times out this gets invoked, the default just exits. virtual void timeout(); diff --git a/test/UnitStorage.cpp b/test/UnitStorage.cpp index e67118fc..525a2fd2 100644 --- a/test/UnitStorage.cpp +++ b/test/UnitStorage.cpp @@ -33,17 +33,11 @@ public: { } - virtual bool filterLoad(const std::string &sessionId, - const std::string &jailId, - bool &/* result */) + bool filterCheckDiskSpace(const std::string & /* path */, + bool &newResult) override { - LOG_TRC("FilterLoad: " << sessionId << " jail " << jailId); - if (_phase == Phase::Filter) - { - LOG_INF("Throwing low disk space exception."); - throw StorageSpaceLowException("test: low disk space"); - } - return false; + newResult = _phase != Phase::Filter; + return true; } void loadDocument(bool bExpectFailure) @@ -76,7 +70,7 @@ public: } } - virtual void invokeTest() + void invokeTest() override { LOG_TRC("invokeTest: " << (int)_phase); switch (_phase) commit b9f18e63a3b68a13b5357266f349b23d6eeeb28d Author: Michael Meeks <michael.me...@collabora.com> Date: Fri Mar 31 17:06:47 2017 +0100 UnitStorage - first cut at restoring this. diff --git a/test/Makefile.am b/test/Makefile.am index c70bfe84..2935e6a0 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -72,7 +72,7 @@ check-local: ./run_unit.sh --log-file test.log --trs-file test.trs # FIXME 2: unit-oob.la fails with symbol undefined: # UnitWSD::testHandleRequest(UnitWSD::TestRequest, UnitHTTPServerRequest&, UnitHTTPServerResponse&) , -TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la # unit-storage.la unit-admin.la +TESTS = unit-prefork.la unit-tilecache.la unit-timeout.la # unit-storage.la # unit-admin.la else TESTS = ${top_builddir}/test/test endif diff --git a/test/UnitStorage.cpp b/test/UnitStorage.cpp index c2cd31c0..e67118fc 100644 --- a/test/UnitStorage.cpp +++ b/test/UnitStorage.cpp @@ -33,40 +33,62 @@ public: { } - virtual bool filterLoad(const std::string &/* sessionId */, - const std::string &/* jailId */, + virtual bool filterLoad(const std::string &sessionId, + const std::string &jailId, bool &/* result */) { + LOG_TRC("FilterLoad: " << sessionId << " jail " << jailId); if (_phase == Phase::Filter) { - _phase = Phase::Reload; LOG_INF("Throwing low disk space exception."); throw StorageSpaceLowException("test: low disk space"); } return false; } - void loadDocument() + void loadDocument(bool bExpectFailure) { std::string docPath; std::string docURL; getDocumentPathAndURL("empty.odt", docPath, docURL, "unitStorage "); _ws = std::unique_ptr<UnitWebSocket>(new UnitWebSocket(docURL)); assert(_ws.get()); + int flags = 0, len;; + char reply[4096]; + while ((len = _ws->getLOOLWebSocket()->receiveFrame(reply, sizeof(reply) - 1, flags)) > 0) + { + reply[len] = '\0'; + if (bExpectFailure && + !strcmp(reply, "error: cmd=internal kind=diskfull")) + { + LOG_TRC("Got expected load failure error"); + _phase = Phase::Reload; + break; + } + else if (!bExpectFailure && + !strncmp(reply, "status: ", sizeof("status: ") - 1)) + { + LOG_TRC("Load completed as expected"); + break; + } + else + std::cerr << "reply '" << reply << "'\n"; + } } virtual void invokeTest() { + LOG_TRC("invokeTest: " << (int)_phase); switch (_phase) { case Phase::Load: _phase = Phase::Filter; - loadDocument(); + loadDocument(true); break; case Phase::Filter: break; case Phase::Reload: - loadDocument(); + loadDocument(false); _ws.reset(); exitTest(TestResult::Ok); break; _______________________________________________ Libreoffice-commits mailing list libreoffice-comm...@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits