[Libreoffice-commits] online.git: 3 commits - common/FileUtil.cpp common/Unit.hpp net/Socket.cpp net/Socket.hpp test/Makefile.am test/UnitStorage.cpp wsd/DocumentBroker.cpp
Michael Meeks
michael.meeks at collabora.com
Fri Mar 31 16:33:02 UTC 2017
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.meeks at 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.meeks at 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.meeks at 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;
More information about the Libreoffice-commits
mailing list