[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-1-0' - 3 commits - loolwsd/DocumentBroker.cpp loolwsd/test loolwsd/Unit.hpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Tue Oct 11 11:16:32 UTC 2016
loolwsd/DocumentBroker.cpp | 6 ++
loolwsd/Unit.hpp | 4 +
loolwsd/test/UnitPrefork.cpp | 101 +++++++++++++++++++++++++++----------------
3 files changed, 74 insertions(+), 37 deletions(-)
New commits:
commit a6412ca90b1ce56130b1b63ad084b61523890247
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Fri May 20 23:08:00 2016 -0400
loolwsd: UnitPrefork can't use the more recent child
Change-Id: I6f60761498e61d9ceb48bc5f9a41967152293590
Reviewed-on: https://gerrit.libreoffice.org/25246
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/test/UnitPrefork.cpp b/loolwsd/test/UnitPrefork.cpp
index 9c5766b..cd4bed0 100644
--- a/loolwsd/test/UnitPrefork.cpp
+++ b/loolwsd/test/UnitPrefork.cpp
@@ -32,7 +32,6 @@ const int NumToPrefork = 20;
// Inside the WSD process
class UnitPrefork : public UnitWSD
{
- int _numStarted;
std::string _failure;
Poco::Timestamp _startTime;
size_t _totalPSS;
@@ -43,8 +42,7 @@ class UnitPrefork : public UnitWSD
public:
UnitPrefork()
- : _numStarted(0),
- _totalPSS(0),
+ : _totalPSS(0),
_totalDirty(0)
{
setHasKitHooks();
@@ -99,6 +97,7 @@ public:
if (_cv.wait_for(lock, std::chrono::milliseconds(5 * 1000)) == std::cv_status::timeout)
{
_failure = "Timed out waiting for child to respond to unit-memdump.";
+ Log::error(_failure);
}
totalPSS = _totalPSS;
@@ -107,19 +106,21 @@ public:
virtual void newChild(const std::shared_ptr<Poco::Net::WebSocket> &socket) override
{
- ++_numStarted;
_childSockets.push_back(socket);
- if (_numStarted >= NumToPrefork)
+ if (_childSockets.size() > NumToPrefork)
{
Poco::Timestamp::TimeDiff elapsed = _startTime.elapsed();
auto totalTime = (1000. * elapsed)/Poco::Timestamp::resolution();
- Log::info() << "Launched " << _numStarted << " in "
+ Log::info() << "Launched " << _childSockets.size() << " in "
<< totalTime << Log::end;
size_t totalPSSKb = 0;
size_t totalDirtyKb = 0;
- for (auto child : _childSockets)
- getMemory(child, totalPSSKb, totalDirtyKb);
+ // Skip the last one as it's not completely initialized yet.
+ for (size_t i = 0; i < _childSockets.size() - 1; ++i)
+ {
+ getMemory(_childSockets[i], totalPSSKb, totalDirtyKb);
+ }
Log::info() << "Memory use total " << totalPSSKb << "k shared "
<< totalDirtyKb << "k dirty" << Log::end;
commit 88df7731da0d24a8925476300504c2fef235cebe
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Fri May 20 22:52:55 2016 -0400
loolwsd: cleanup and improvements to UnitPrefork
Change-Id: I15394fa9199f0d2489a184d4c07602da02cbab64
Reviewed-on: https://gerrit.libreoffice.org/25245
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/test/UnitPrefork.cpp b/loolwsd/test/UnitPrefork.cpp
index f9437bb..9c5766b 100644
--- a/loolwsd/test/UnitPrefork.cpp
+++ b/loolwsd/test/UnitPrefork.cpp
@@ -53,6 +53,11 @@ public:
virtual void returnValue(int &retValue) override
{
// 0 when empty (success), otherwise failure.
+ if (!_failure.empty())
+ {
+ Log::error("UnitPrefork failed due to: " + _failure);
+ }
+
retValue = !_failure.empty();
}
@@ -108,8 +113,9 @@ public:
{
Poco::Timestamp::TimeDiff elapsed = _startTime.elapsed();
+ auto totalTime = (1000. * elapsed)/Poco::Timestamp::resolution();
Log::info() << "Launched " << _numStarted << " in "
- << (1.0 * elapsed)/Poco::Timestamp::resolution() << Log::end;
+ << totalTime << Log::end;
size_t totalPSSKb = 0;
size_t totalDirtyKb = 0;
for (auto child : _childSockets)
@@ -123,10 +129,19 @@ public:
Log::info() << "Memory use average " << totalPSSKb << "k shared "
<< totalDirtyKb << "k dirty" << Log::end;
+ Log::info() << "Launch time total " << totalTime << " ms" << Log::end;
+ totalTime /= _childSockets.size();
+ Log::info() << "Launch time average " << totalTime << " ms" << Log::end;
+
if (!_failure.empty())
+ {
+ Log::error("UnitPrefork failed due to: " + _failure);
exitTest(TestResult::TEST_FAILED);
+ }
else
+ {
exitTest(TestResult::TEST_OK);
+ }
}
}
};
@@ -140,14 +155,13 @@ namespace {
if (!strncmp(line, tag, len))
{
while (!isdigit(line[len]) && line[len] != '\0')
- len++;
+ ++len;
const auto str = std::string(line + len, strlen(line + len) - 1);
- Log::info(std::string("does start with ") + tag + " '" + str + "'");
return line + len;
}
- else
- return 0;
+
+ return nullptr;
}
std::string readMemorySizes(FILE *inStream)
@@ -172,6 +186,7 @@ namespace {
Log::info("readMemorySize: [" + res + "].");
if (res.empty())
{
+ Log::error("Failed to read memory stats.");
throw std::runtime_error("Failed to read memory stats.");
}
@@ -273,7 +288,7 @@ public:
virtual bool filterKitMessage(const std::shared_ptr<Poco::Net::WebSocket> &ws,
std::string &message) override
{
- const auto token = LOOLProtocol::getFirstToken(message.c_str(), message.length());
+ const auto token = LOOLProtocol::getFirstToken(message);
if (token == "unit-memdump:")
{
std::string memory;
commit ffc6b2da48aff879ca5ff1f02bc0b4ab46b687a6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Fri May 20 22:31:53 2016 -0400
loolwsd: fix UnitPrefork deadlock/corruption
Reading from the socket in the test is not
thread-safe, and was causing all sorts of
problems.
The new code adds a test API and reads the
incoming data through it and not directly
from the socket. In addition, the read is
synchronized.
Change-Id: Id13821a40a59e32fd8a14f733a47306aee42ada8
Reviewed-on: https://gerrit.libreoffice.org/25244
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 85e2830..2737320 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -18,6 +18,7 @@
#include "Storage.hpp"
#include "TileCache.hpp"
#include "LOOLProtocol.hpp"
+#include "Unit.hpp"
using namespace LOOLProtocol;
@@ -26,6 +27,11 @@ void ChildProcess::socketProcessor()
IoUtil::SocketProcessor(_ws,
[this](const std::vector<char>& payload)
{
+ if (UnitWSD::get().filterChildMessage(payload))
+ {
+ return true;
+ }
+
auto docBroker = this->_docBroker.lock();
if (docBroker)
{
diff --git a/loolwsd/Unit.hpp b/loolwsd/Unit.hpp
index dcdd36c..9cebfaa 100644
--- a/loolwsd/Unit.hpp
+++ b/loolwsd/Unit.hpp
@@ -134,6 +134,10 @@ public:
Poco::Net::HTTPServerResponse& /* response */)
{ return false; }
+ /// Child sent a message
+ virtual bool filterChildMessage(const std::vector<char>& /* payload */)
+ { return false; }
+
// ---------------- TileCache hooks ----------------
/// Called before the lookupTile call returns. Should always be called to fire events.
virtual void lookupTile(int part, int width, int height, int tilePosX, int tilePosY,
diff --git a/loolwsd/test/UnitPrefork.cpp b/loolwsd/test/UnitPrefork.cpp
index c265831..f9437bb 100644
--- a/loolwsd/test/UnitPrefork.cpp
+++ b/loolwsd/test/UnitPrefork.cpp
@@ -14,6 +14,9 @@
#include <sys/types.h>
#include <dirent.h>
+#include <mutex>
+#include <condition_variable>
+
#include "Common.hpp"
#include "IoUtil.hpp"
#include "LOOLProtocol.hpp"
@@ -32,11 +35,17 @@ class UnitPrefork : public UnitWSD
int _numStarted;
std::string _failure;
Poco::Timestamp _startTime;
+ size_t _totalPSS;
+ size_t _totalDirty;
+ std::mutex _mutex;
+ std::condition_variable _cv;
std::vector< std::shared_ptr<Poco::Net::WebSocket> > _childSockets;
public:
UnitPrefork()
- : _numStarted(0)
+ : _numStarted(0),
+ _totalPSS(0),
+ _totalDirty(0)
{
setHasKitHooks();
}
@@ -52,29 +61,9 @@ public:
numPrefork = NumToPrefork;
}
- void getMemory(const std::shared_ptr<Poco::Net::WebSocket> &socket,
- size_t &totalPSS, size_t &totalDirty)
+ virtual bool filterChildMessage(const std::vector<char>& payload)
{
- /// Fetch memory usage data from the last process ...
- socket->sendFrame("unit-memdump: \n", sizeof("unit-memdump: \n")-1);
-
- static const Poco::Timespan waitTime(COMMAND_TIMEOUT_MS * 1000);
- if (!socket->poll(waitTime, Poco::Net::Socket::SELECT_READ))
- {
- _failure = "Timed out waiting for child to respond to unit-memdump command.";
- return;
- }
-
- int flags;
- char buffer[4096];
- const int length = IoUtil::receiveFrame(*socket, buffer, sizeof (buffer), flags);
- if (length <= 0 || ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) == Poco::Net::WebSocket::FRAME_OP_CLOSE))
- {
- _failure = "Failed to read child response to unit-memdump command.";
- return;
- }
-
- const std::string memory = LOOLProtocol::getFirstLine(buffer, length);
+ const std::string memory = LOOLProtocol::getFirstLine(payload);
if (!memory.compare(0,6,"Error:"))
{
_failure = memory;
@@ -84,9 +73,31 @@ public:
Log::info("Got memory stats [" + memory + "].");
Poco::StringTokenizer tokens(memory, " ");
assert(tokens.count() == 2);
- totalPSS += atoi(tokens[0].c_str());
- totalDirty += atoi(tokens[1].c_str());
+ _totalPSS += atoi(tokens[0].c_str());
+ _totalDirty += atoi(tokens[1].c_str());
}
+
+ // Don't signal before wait.
+ std::unique_lock<std::mutex> lock(_mutex);
+ _cv.notify_one();
+ return true;
+ }
+
+ void getMemory(const std::shared_ptr<Poco::Net::WebSocket> &socket,
+ size_t &totalPSS, size_t &totalDirty)
+ {
+ std::unique_lock<std::mutex> lock(_mutex);
+
+ /// Fetch memory usage data from the last process ...
+ socket->sendFrame("unit-memdump: \n", sizeof("unit-memdump: \n"));
+
+ if (_cv.wait_for(lock, std::chrono::milliseconds(5 * 1000)) == std::cv_status::timeout)
+ {
+ _failure = "Timed out waiting for child to respond to unit-memdump.";
+ }
+
+ totalPSS = _totalPSS;
+ totalDirty = _totalDirty;
}
virtual void newChild(const std::shared_ptr<Poco::Net::WebSocket> &socket) override
More information about the Libreoffice-commits
mailing list