[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - 46 commits - common/Common.hpp common/FileUtil.cpp common/IoUtil.cpp common/LOOLWebSocket.hpp common/Session.cpp common/Session.hpp common/Unit.hpp common/Util.cpp configure.ac kit/ChildSession.cpp kit/Kit.cpp kit/Kit.hpp loleaflet/dist loleaflet/reference.html loleaflet/src Makefile.am net/Socket.cpp net/Socket.hpp net/Ssl.cpp net/Ssl.hpp net/WebSocketHandler.hpp test/helpers.hpp test/httpwstest.cpp test/Makefile.am test/UnitMinSocketBufferSize.cpp test/UnitStorage.cpp wsd/Admin.cpp wsd/Admin.hpp wsd/AdminModel.cpp wsd/AdminModel.hpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/FileServer.cpp wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp wsd/reference.txt
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Tue Apr 4 04:55:42 UTC 2017
Makefile.am | 28 +---
common/Common.hpp | 6
common/FileUtil.cpp | 5
common/IoUtil.cpp | 21 ---
common/LOOLWebSocket.hpp | 18 --
common/Session.cpp | 8 +
common/Session.hpp | 8 +
common/Unit.hpp | 7 +
common/Util.cpp | 12 -
configure.ac | 2
kit/ChildSession.cpp | 2
kit/Kit.cpp | 32 +++-
kit/Kit.hpp | 1
loleaflet/dist/toolbar/toolbar.js | 6
loleaflet/reference.html | 3
loleaflet/src/admin/AdminSocketSettings.js | 4
loleaflet/src/control/Control.ContextMenu.js | 16 ++
loleaflet/src/control/Control.Menubar.js | 5
loleaflet/src/control/Control.Scroll.js | 13 +-
loleaflet/src/core/Socket.js | 4
loleaflet/src/layer/tile/TileLayer.js | 42 +++---
loleaflet/src/map/Map.js | 14 +-
net/Socket.cpp | 122 ++++++++++++------
net/Socket.hpp | 159 +++++++++++++-----------
net/Ssl.cpp | 39 +++---
net/Ssl.hpp | 3
net/WebSocketHandler.hpp | 59 +++------
test/Makefile.am | 8 -
test/UnitMinSocketBufferSize.cpp | 86 -------------
test/UnitStorage.cpp | 44 ++++--
test/helpers.hpp | 18 --
test/httpwstest.cpp | 23 ---
wsd/Admin.cpp | 36 ++---
wsd/Admin.hpp | 5
wsd/AdminModel.cpp | 60 ++++++++-
wsd/AdminModel.hpp | 12 +
wsd/ClientSession.cpp | 14 +-
wsd/ClientSession.hpp | 6
wsd/DocumentBroker.cpp | 111 ++++++++---------
wsd/DocumentBroker.hpp | 36 ++---
wsd/FileServer.cpp | 3
wsd/LOOLWSD.cpp | 175 ++++++++++++++-------------
wsd/LOOLWSD.hpp | 4
wsd/reference.txt | 6
44 files changed, 665 insertions(+), 621 deletions(-)
New commits:
commit 7f499cca85877c3dca682fb5cb83a65d567ccf06
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Apr 4 00:11:06 2017 -0400
wsd: move correct-thread assertions
Change-Id: Iba5d58d74720aaf02a372f25148047e79c89c1bd
Reviewed-on: https://gerrit.libreoffice.org/36060
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit d11b3a76d9284c23b0262abd2c48a2dc89bed571)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 483e7c5e..97626af9 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -277,6 +277,8 @@ bool DocumentBroker::isAlive() const
DocumentBroker::~DocumentBroker()
{
+ assert(isCorrectThread());
+
Admin::instance().rmDoc(_docKey);
LOG_INF("~DocumentBroker [" << _uriPublic.toString() <<
@@ -630,6 +632,8 @@ void DocumentBroker::setLoaded()
bool DocumentBroker::autoSave(const bool force)
{
+ assert(isCorrectThread());
+
if (_sessions.empty() || _storage == nullptr || !_isLoaded ||
!_childProcess->isAlive() || (!_isModified && !force))
{
@@ -669,6 +673,8 @@ bool DocumentBroker::autoSave(const bool force)
bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
{
+ assert(isCorrectThread());
+
LOG_INF("Autosave triggered for doc [" << _docKey << "].");
std::shared_ptr<ClientSession> savingSession;
@@ -1168,6 +1174,8 @@ void DocumentBroker::setModified(const bool value)
bool DocumentBroker::forwardToChild(const std::string& viewId, const std::string& message)
{
+ assert(isCorrectThread());
+
LOG_TRC("Forwarding payload to child [" << viewId << "]: " << message);
std::string msg = "child-" + viewId + ' ' + message;
@@ -1242,7 +1250,7 @@ bool DocumentBroker::forwardToClient(const std::shared_ptr<Message>& payload)
void DocumentBroker::childSocketTerminated()
{
- std::lock_guard<std::mutex> lock(_mutex);
+ assert(isCorrectThread());
if (!_childProcess->isAlive())
{
@@ -1338,6 +1346,7 @@ void DocumentBroker::dumpState(std::ostream& os)
os << "\n public uri: " << _uriPublic.toString();
os << "\n jailed uri: " << _uriJailed.toString();
os << "\n doc key: " << _docKey;
+ os << "\n doc id: " << _docId;
os << "\n num sessions: " << getSessionsCount();
os << "\n last editable?: " << _lastEditableSession;
std::time_t t = std::chrono::system_clock::to_time_t(
commit 98d3565f7c7ce832e948a66ad9ab6bdacb43b488
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Apr 4 00:09:12 2017 -0400
wsd: move socket to DocBroker in callback
Change-Id: I23af97788d64268a822700ab16d63b970795a105
Reviewed-on: https://gerrit.libreoffice.org/36059
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 4a5c71c13c6c41710ef8bba70f79975c93fd86c2)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 19259e56..9e7bc8f8 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2080,16 +2080,18 @@ private:
{
// Transfer the client socket to the DocumentBroker.
- // Set the ClientSession to handle Socket events.
- socket->setHandler(clientSession);
- LOG_DBG("Socket #" << socket->getFD() << " handler is " << clientSession->getName());
-
- // Move the socket into DocBroker.
- docBroker->addSocketToPoll(socket);
+ // Remove from current poll as we're moving ownership.
socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
- docBroker->addCallback([docBroker, clientSession]()
+ docBroker->addCallback([docBroker, socket, clientSession]()
{
+ // Set the ClientSession to handle Socket events.
+ socket->setHandler(clientSession);
+ LOG_DBG("Socket #" << socket->getFD() << " handler is " << clientSession->getName());
+
+ // Move the socket into DocBroker.
+ docBroker->addSocketToPoll(socket);
+
// Add and load the session.
docBroker->addSession(clientSession);
});
commit 821149df51a0c48d3b5d923764fac15d10fba7de
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 23:57:59 2017 -0400
wsd: cleanup deflating HTTP responses
Change-Id: Id21bdfcb5d3e04f27b681ee9581a0ed06283d163
Reviewed-on: https://gerrit.libreoffice.org/36058
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit b734284b599bb612cd8eee13e761ce42c8eb8a6a)
diff --git a/net/Socket.cpp b/net/Socket.cpp
index bac3a741..1f6d64d0 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -205,15 +205,17 @@ namespace HttpHelper
response.set("ETag", "\"" LOOLWSD_VERSION_HASH "\"");
}
- if(!deflate)
+ int bufferSize = std::min(st.st_size, (off_t)Socket::MaximumSendBufferSize);
+ if (st.st_size >= socket->getSendBufferSize())
{
- int bufferSize = std::min(st.st_size, (off_t)Socket::MaximumSendBufferSize);
- if (st.st_size >= socket->getSendBufferSize())
- {
- socket->setSocketBufferSize(bufferSize);
- bufferSize = socket->getSendBufferSize();
- }
+ socket->setSocketBufferSize(bufferSize);
+ bufferSize = socket->getSendBufferSize();
+ }
+ // Deflate is done over the full file, which can be too large.
+ // Skip deflating (ironically) if the file is too large.
+ if (!deflate || st.st_size > Socket::MaximumSendBufferSize * 10)
+ {
response.setContentLength(st.st_size);
std::ostringstream oss;
response.write(oss);
@@ -246,18 +248,16 @@ namespace HttpHelper
socket->send(header);
std::ifstream file(path, std::ios::binary);
- uLong bufferSize;
- bufferSize = st.st_size;
- char buf[bufferSize];
bool flush = true;
do
{
- unsigned int a = 9;
- file.read(buf, sizeof(buf));
- long unsigned int size = file.gcount();
+ static const unsigned int level = 9;
+ char buf[st.st_size]; // FIXME: Should compress in chunks.
+ file.read(buf, st.st_size);
+ const long unsigned int size = file.gcount();
long unsigned int compSize = compressBound(size);
char cbuf[compSize];
- compress2((Bytef *)&cbuf, &compSize, (Bytef *)&buf, size, a) ;
+ compress2((Bytef *)&cbuf, &compSize, (Bytef *)&buf, size, level);
if (size > 0)
socket->send(cbuf, compSize, flush);
else
commit 31b8b73bb82a2f486ec6356a03cd668369cb8730
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 23:13:03 2017 -0400
wsd: don't invoke onDisconnect from wrong thread
...and warn if we are in the wrong thread.
This can happen when the socket is not properly
closed from the poll thread and is being destroyed.
Change-Id: I749c09b15d04b49038f7cee6a7a13e8f0145acff
Reviewed-on: https://gerrit.libreoffice.org/36057
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 7ca1f03770c117a316b23644306160a6a80ea583)
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 9cf83c49..9e06a040 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -610,7 +610,12 @@ public:
LOG_DBG("StreamSocket dtor #" << getFD());
if (!_closed)
- _socketHandler->onDisconnect();
+ {
+ if (isCorrectThread())
+ _socketHandler->onDisconnect();
+ else
+ LOG_WRN("#" << getFD() << " not properly shutdown. onDisconnect not called.");
+ }
if (!_shutdownSignalled)
{
commit 527170a92b30e261615b1e64492169c368326ad4
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 21:07:53 2017 -0400
wsd: don't close invalid forkit pipe
Change-Id: Ib66df894560ad592a7e90774897cb82b573dc77d
Reviewed-on: https://gerrit.libreoffice.org/36056
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 26ac97826614dc8e6f1f09199023e63fa240c31d)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 1ac9efa2..19259e56 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -347,7 +347,7 @@ static int rebalanceChildren(int balance)
{
Util::assertIsLocked(NewChildrenMutex);
- LOG_WRN("rebalance children to " << balance);
+ LOG_TRC("rebalance children to " << balance);
// Do the cleanup first.
const bool rebalance = cleanupChildren();
@@ -1182,9 +1182,9 @@ bool LOOLWSD::createForKit()
ForKitProcId = -1;
Admin::instance().setForKitPid(ForKitProcId);
- const int oldForKitWritePipe = ForKitWritePipe;
+ if (ForKitWritePipe != -1)
+ close(ForKitWritePipe);
ForKitWritePipe = -1;
- close(oldForKitWritePipe);
// ForKit always spawns one.
++OutstandingForks;
commit e16e58b4e0e2df76b466e60bfea46c44b1667515
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 21:06:51 2017 -0400
wsd: fix SSL initialization/uninitialization error
Valgrind found a number of erroneous data access
during the construction and destruction of SslContext.
Change-Id: Ie5072798a3660ed8acc707ba32ac196fa2d0f8af
Reviewed-on: https://gerrit.libreoffice.org/36055
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 762ba09370800a38c5bf48b6950188a31a9d0cdf)
diff --git a/net/Ssl.cpp b/net/Ssl.cpp
index d6d13575..e350257c 100644
--- a/net/Ssl.cpp
+++ b/net/Ssl.cpp
@@ -26,31 +26,30 @@ extern "C"
};
}
-std::unique_ptr<SslContext> SslContext::Instance;
-std::vector<std::unique_ptr<std::mutex>> SslContext::Mutexes;
+std::unique_ptr<SslContext> SslContext::Instance(nullptr);
SslContext::SslContext(const std::string& certFilePath,
const std::string& keyFilePath,
const std::string& caFilePath) :
_ctx(nullptr)
{
-#if OPENSSL_VERSION_NUMBER >= 0x0907000L
- OPENSSL_config(nullptr);
-#endif
-
- SSL_library_init();
- SSL_load_error_strings();
- OpenSSL_add_all_algorithms();
-
const std::vector<char> rand = Util::rng::getBytes(512);
RAND_seed(&rand[0], rand.size());
// Initialize multi-threading support.
for (int x = 0; x < CRYPTO_num_locks(); ++x)
{
- Mutexes.emplace_back(new std::mutex);
+ _mutexes.emplace_back(new std::mutex);
}
+#if OPENSSL_VERSION_NUMBER >= 0x0907000L
+ OPENSSL_config(nullptr);
+#endif
+
+ SSL_library_init();
+ SSL_load_error_strings();
+ OpenSSL_add_all_algorithms();
+
CRYPTO_set_locking_callback(&SslContext::lock);
CRYPTO_set_id_callback(&SslContext::id);
CRYPTO_set_dynlock_create_callback(&SslContext::dynlockCreate);
@@ -130,6 +129,8 @@ SslContext::~SslContext()
CRYPTO_set_id_callback(0);
CONF_modules_free();
+
+ _mutexes.clear();
}
void SslContext::uninitialize()
@@ -140,13 +141,17 @@ void SslContext::uninitialize()
void SslContext::lock(int mode, int n, const char* /*file*/, int /*line*/)
{
- if (mode & CRYPTO_LOCK)
- {
- Mutexes[n]->lock();
- }
- else
+ assert(n < CRYPTO_num_locks());
+ if (Instance)
{
- Mutexes[n]->unlock();
+ if (mode & CRYPTO_LOCK)
+ {
+ Instance->_mutexes[n]->lock();
+ }
+ else
+ {
+ Instance->_mutexes[n]->unlock();
+ }
}
}
diff --git a/net/Ssl.hpp b/net/Ssl.hpp
index 7c13474a..b6fc0427 100644
--- a/net/Ssl.hpp
+++ b/net/Ssl.hpp
@@ -65,7 +65,8 @@ private:
private:
static std::unique_ptr<SslContext> Instance;
- static std::vector<std::unique_ptr<std::mutex>> Mutexes;
+
+ std::vector<std::unique_ptr<std::mutex>> _mutexes;
SSL_CTX* _ctx;
};
commit 6a8814e985f93cb1fc97ebb7ed0608fbb14aa502
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 21:05:36 2017 -0400
wsd: enable logging with valgrind
Change-Id: I411f7de3d5764cd25af211f2dc77bf0e290adbc7
Reviewed-on: https://gerrit.libreoffice.org/36054
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 66a76f644bde3aa21f9de4e095be8c8300cbf021)
diff --git a/Makefile.am b/Makefile.am
index 76f1dc8d..a438ff7d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -226,27 +226,32 @@ run: all @JAILS_PATH@ @SYSTEMPLATE_PATH@/system_stamp
--o:ssl.cert_file_path="$(abs_top_srcdir)/etc/cert.pem" \
--o:ssl.key_file_path="$(abs_top_srcdir)/etc/key.pem" \
--o:ssl.ca_file_path="$(abs_top_srcdir)/etc/ca-chain.cert.pem" \
- --o:admin_console.username=admin --o:admin_console.password=admin
+ --o:admin_console.username=admin --o:admin_console.password=admin \
+ --o:logging.file[@enable]=false --o:logging.level=trace
run-valgrind: all @JAILS_PATH@ @SYSTEMPLATE_PATH@/system_stamp
@echo "Launching loolwsd under valgrind (but not forkit/loolkit, yet)"
+ @cp $(abs_top_srcdir)/test/data/hello.odt $(abs_top_srcdir)/test/data/hello-world.odt
valgrind --tool=memcheck --trace-children=no -v --read-var-info=yes \
./loolwsd --o:sys_template_path="@SYSTEMPLATE_PATH@" --o:lo_template_path="@LO_PATH@" \
--o:child_root_path="@JAILS_PATH@" --o:storage.filesystem[@allow]=true \
--o:ssl.cert_file_path="$(abs_top_srcdir)/etc/cert.pem" \
--o:ssl.key_file_path="$(abs_top_srcdir)/etc/key.pem" \
--o:ssl.ca_file_path="$(abs_top_srcdir)/etc/ca-chain.cert.pem" \
- --o:admin_console.username=admin --o:admin_console.password=admin
+ --o:admin_console.username=admin --o:admin_console.password=admin \
+ --o:logging.file[@enable]=false --o:logging.level=trace
run-callgrind: all @JAILS_PATH@ @SYSTEMPLATE_PATH@/system_stamp
@echo "Launching loolwsd under valgrind (but not forkit/loolkit, yet)"
+ @cp $(abs_top_srcdir)/test/data/hello.odt $(abs_top_srcdir)/test/data/hello-world.odt
valgrind --tool=callgrind --simulate-cache=yes --dump-instr=yes --num-callers=50 --error-limit=no \
./loolwsd --o:sys_template_path="@SYSTEMPLATE_PATH@" --o:lo_template_path="@LO_PATH@" \
--o:child_root_path="@JAILS_PATH@" --o:storage.filesystem[@allow]=true \
--o:ssl.cert_file_path="$(abs_top_srcdir)/etc/cert.pem" \
--o:ssl.key_file_path="$(abs_top_srcdir)/etc/key.pem" \
--o:ssl.ca_file_path="$(abs_top_srcdir)/etc/ca-chain.cert.pem" \
- --o:admin_console.username=admin --o:admin_console.password=admin
+ --o:admin_console.username=admin --o:admin_console.password=admin \
+ --o:logging.file[@enable]=false --o:logging.level=trace
else
SYSTEM_STAMP =
commit 124d904b88284934619392ae7e4c81516981fffb
Author: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
Date: Fri Mar 24 14:24:19 2017 +0000
Use hub link for git log
Change-Id: Iaf4a6f5568f2e84d8261756607a62598a7144df1
Reviewed-on: https://gerrit.libreoffice.org/35656
Reviewed-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
Tested-by: Samuel Mehrbrodt <Samuel.Mehrbrodt at cib.de>
(cherry picked from commit 460da71ce23639b275e95f53fd2a5c8a1ddc2901)
diff --git a/loleaflet/src/admin/AdminSocketSettings.js b/loleaflet/src/admin/AdminSocketSettings.js
index 5a7ec207..537d1487 100644
--- a/loleaflet/src/admin/AdminSocketSettings.js
+++ b/loleaflet/src/admin/AdminSocketSettings.js
@@ -70,7 +70,7 @@ var AdminSocketSettings = AdminSocketBase.extend({
var loolwsdVersionObj = JSON.parse(textMsg.substring(textMsg.indexOf('{')));
var h = loolwsdVersionObj.Hash;
if (parseInt(h,16).toString(16) === h.toLowerCase().replace(/^0+/, '')) {
- h = '<a target="_blank" href="https://gerrit.libreoffice.org/gitweb?p=online.git;a=log;h=' + h + '">' + h + '</a>';
+ h = '<a target="_blank" href="https://hub.libreoffice.org/git-online/' + h + '">' + h + '</a>';
$('#loolwsd-version').html(loolwsdVersionObj.Version + ' (git hash: ' + h + ')');
}
else {
@@ -81,7 +81,7 @@ var AdminSocketSettings = AdminSocketBase.extend({
var lokitVersionObj = JSON.parse(textMsg.substring(textMsg.indexOf('{')));
var h = lokitVersionObj.BuildId.substring(0, 7);
if (parseInt(h,16).toString(16) === h.toLowerCase().replace(/^0+/, '')) {
- h = '<a target="_blank" href="https://gerrit.libreoffice.org/gitweb?p=core.git;a=log;h=' + h + '">' + h + '</a>';
+ h = '<a target="_blank" href="https://hub.libreoffice.org/git-core/' + h + '">' + h + '</a>';
}
$('#lokit-version').html(lokitVersionObj.ProductName + ' ' +
lokitVersionObj.ProductVersion + lokitVersionObj.ProductExtension.replace('.10.','-') +
diff --git a/loleaflet/src/core/Socket.js b/loleaflet/src/core/Socket.js
index ea8f15e9..787a487e 100644
--- a/loleaflet/src/core/Socket.js
+++ b/loleaflet/src/core/Socket.js
@@ -176,7 +176,7 @@ L.Socket = L.Class.extend({
var loolwsdVersionObj = JSON.parse(textMsg.substring(textMsg.indexOf('{')));
var h = loolwsdVersionObj.Hash;
if (parseInt(h,16).toString(16) === h.toLowerCase().replace(/^0+/, '')) {
- h = '<a target="_blank" href="https://gerrit.libreoffice.org/gitweb?p=online.git;a=log;h=' + h + '">' + h + '</a>';
+ h = '<a target="_blank" href="https://hub.libreoffice.org/git-online/' + h + '">' + h + '</a>';
$('#loolwsd-version').html(loolwsdVersionObj.Version + ' (git hash: ' + h + ')');
}
else {
@@ -192,7 +192,7 @@ L.Socket = L.Class.extend({
var lokitVersionObj = JSON.parse(textMsg.substring(textMsg.indexOf('{')));
var h = lokitVersionObj.BuildId.substring(0, 7);
if (parseInt(h,16).toString(16) === h.toLowerCase().replace(/^0+/, '')) {
- h = '<a target="_blank" href="https://gerrit.libreoffice.org/gitweb?p=core.git;a=log;h=' + h + '">' + h + '</a>';
+ h = '<a target="_blank" href="https://hub.libreoffice.org/git-core/' + h + '">' + h + '</a>';
}
$('#lokit-version').html(lokitVersionObj.ProductName + ' ' +
lokitVersionObj.ProductVersion + lokitVersionObj.ProductExtension.replace('.10.','-') +
commit be6668862f290198d1f4fef7978c9610cc04ff1e
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Mon Apr 3 18:21:20 2017 +0100
Admin model locking - major cleanup.
Do everything in the Admin Model in the AdminPoll thread.
Everything else can push work there safely through callbacks.
(cherry picked from commit 0806986c8cd8147b925aeb167ff0708fac6f9725)
diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp
index 4ab3d821..78da7ac1 100644
--- a/wsd/Admin.cpp
+++ b/wsd/Admin.cpp
@@ -57,7 +57,6 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
return;
}
- std::unique_lock<std::mutex> modelLock(_admin->getLock());
AdminModel& model = _admin->getModel();
if (tokens[0] == "auth")
@@ -262,14 +261,13 @@ bool AdminSocketHandler::handleInitialRequest(
Admin &admin = Admin::instance();
auto handler = std::make_shared<AdminSocketHandler>(&admin, socketWeak, request);
socket->setHandler(handler);
-
- { // FIXME: weird locking around subscribe ...
- std::unique_lock<std::mutex> modelLock(admin.getLock());
- // Subscribe the websocket of any AdminModel updates
- AdminModel& model = admin.getModel();
+ admin.addCallback([handler, sessionId]
+ {
+ Admin &adminIn = Admin::instance();
+ AdminModel& model = adminIn.getModel();
handler->_sessionId = sessionId;
model.subscribe(sessionId, handler);
- }
+ });
return true;
}
@@ -293,7 +291,6 @@ Admin::Admin() :
{
LOG_INF("Admin ctor.");
- std::unique_lock<std::mutex> modelLock(getLock());
const auto totalMem = getTotalMemoryUsage();
LOG_TRC("Total memory used: " << totalMem);
_model.addMemStats(totalMem);
@@ -308,6 +305,8 @@ void Admin::pollingThread()
{
std::chrono::steady_clock::time_point lastCPU, lastMem;
+ _model.setThreadOwner(std::this_thread::get_id());
+
lastCPU = std::chrono::steady_clock::now();
lastMem = lastCPU;
@@ -326,7 +325,6 @@ void Admin::pollingThread()
std::chrono::duration_cast<std::chrono::milliseconds>(now - lastCPU).count();
if (memWait <= 0)
{
- std::unique_lock<std::mutex> modelLock(getLock());
const auto totalMem = getTotalMemoryUsage();
if (totalMem != _lastTotalMemory)
{
@@ -349,21 +347,20 @@ void Admin::pollingThread()
void Admin::addDoc(const std::string& docKey, Poco::Process::PID pid, const std::string& filename, const std::string& sessionId)
{
- std::unique_lock<std::mutex> modelLock(_modelMutex);
- _model.addDocument(docKey, pid, filename, sessionId);
+ addCallback([this, docKey, pid, filename, sessionId]
+ { _model.addDocument(docKey, pid, filename, sessionId); });
}
void Admin::rmDoc(const std::string& docKey, const std::string& sessionId)
{
- std::unique_lock<std::mutex> modelLock(_modelMutex);
- _model.removeDocument(docKey, sessionId);
+ addCallback([this, docKey, sessionId]
+ { _model.removeDocument(docKey, sessionId); });
}
void Admin::rmDoc(const std::string& docKey)
{
- std::unique_lock<std::mutex> modelLock(_modelMutex);
LOG_INF("Removing complete doc [" << docKey << "] from Admin.");
- _model.removeDocument(docKey);
+ addCallback([this, docKey]{ _model.removeDocument(docKey); });
}
void Admin::rescheduleMemTimer(unsigned interval)
@@ -382,8 +379,6 @@ void Admin::rescheduleCpuTimer(unsigned interval)
unsigned Admin::getTotalMemoryUsage()
{
- Util::assertIsLocked(_modelMutex);
-
// To simplify and clarify this; since load, link and pre-init all
// inside the forkit - we should account all of our fixed cost of
// memory to the forkit; and then count only dirty pages in the clients
@@ -413,14 +408,13 @@ AdminModel& Admin::getModel()
void Admin::updateLastActivityTime(const std::string& docKey)
{
- std::unique_lock<std::mutex> modelLock(_modelMutex);
- _model.updateLastActivityTime(docKey);
+ addCallback([this, docKey]{ _model.updateLastActivityTime(docKey); });
}
void Admin::updateMemoryDirty(const std::string& docKey, int dirty)
{
- std::unique_lock<std::mutex> modelLock(_modelMutex);
- _model.updateMemoryDirty(docKey, dirty);
+ addCallback([this, docKey, dirty]
+ { _model.updateMemoryDirty(docKey, dirty); });
}
void Admin::dumpState(std::ostream& os)
diff --git a/wsd/Admin.hpp b/wsd/Admin.hpp
index a816b636..033619a7 100644
--- a/wsd/Admin.hpp
+++ b/wsd/Admin.hpp
@@ -96,16 +96,15 @@ public:
void rescheduleCpuTimer(unsigned interval);
- std::unique_lock<std::mutex> getLock() { return std::unique_lock<std::mutex>(_modelMutex); }
-
void updateLastActivityTime(const std::string& docKey);
void updateMemoryDirty(const std::string& docKey, int dirty);
void dumpState(std::ostream& os) override;
private:
+ /// The model is accessed only during startup & in
+ /// the Admin Poll thread.
AdminModel _model;
- std::mutex _modelMutex;
int _forKitPid;
long _lastTotalMemory;
diff --git a/wsd/AdminModel.cpp b/wsd/AdminModel.cpp
index 5e1aefc7..1f3e6dcc 100644
--- a/wsd/AdminModel.cpp
+++ b/wsd/AdminModel.cpp
@@ -94,8 +94,26 @@ void Subscriber::unsubscribe(const std::string& command)
_subscriptions.erase(command);
}
+bool AdminModel::isCorrectThread() const
+{
+#if ENABLE_DEBUG
+ // FIXME: share this code [!]
+ const bool sameThread = std::this_thread::get_id() == _owner;
+ if (!sameThread)
+ LOG_WRN("Admin command invoked from foreign thread. Expected: 0x" << std::hex <<
+ _owner << " but called from 0x" << std::this_thread::get_id() << " (" <<
+ std::dec << Util::getThreadId() << ").");
+
+ return sameThread;
+#else
+ return true;
+#endif
+}
+
std::string AdminModel::query(const std::string& command)
{
+ assert (isCorrectThread());
+
const auto token = LOOLProtocol::getFirstToken(command);
if (token == "documents")
{
@@ -132,6 +150,8 @@ std::string AdminModel::query(const std::string& command)
/// Returns memory consumed by all active loolkit processes
unsigned AdminModel::getKitsMemoryUsage()
{
+ assert (isCorrectThread());
+
unsigned totalMem = 0;
unsigned docs = 0;
for (const auto& it : _documents)
@@ -158,6 +178,8 @@ unsigned AdminModel::getKitsMemoryUsage()
void AdminModel::subscribe(int sessionId, const std::weak_ptr<WebSocketHandler>& ws)
{
+ assert (isCorrectThread());
+
const auto ret = _subscribers.emplace(sessionId, Subscriber(sessionId, ws));
if (!ret.second)
{
@@ -167,6 +189,8 @@ void AdminModel::subscribe(int sessionId, const std::weak_ptr<WebSocketHandler>&
void AdminModel::subscribe(int sessionId, const std::string& command)
{
+ assert (isCorrectThread());
+
auto subscriber = _subscribers.find(sessionId);
if (subscriber != _subscribers.end())
{
@@ -176,37 +200,39 @@ void AdminModel::subscribe(int sessionId, const std::string& command)
void AdminModel::unsubscribe(int sessionId, const std::string& command)
{
+ assert (isCorrectThread());
+
auto subscriber = _subscribers.find(sessionId);
if (subscriber != _subscribers.end())
- {
subscriber->second.unsubscribe(command);
- }
}
void AdminModel::addMemStats(unsigned memUsage)
{
+ assert (isCorrectThread());
+
_memStats.push_back(memUsage);
if (_memStats.size() > _memStatsSize)
- {
_memStats.pop_front();
- }
notify("mem_stats " + std::to_string(memUsage));
}
void AdminModel::addCpuStats(unsigned cpuUsage)
{
+ assert (isCorrectThread());
+
_cpuStats.push_back(cpuUsage);
if (_cpuStats.size() > _cpuStatsSize)
- {
_cpuStats.pop_front();
- }
notify("cpu_stats " + std::to_string(cpuUsage));
}
void AdminModel::setCpuStatsSize(unsigned size)
{
+ assert (isCorrectThread());
+
int wasteValuesLen = _cpuStats.size() - size;
while (wasteValuesLen-- > 0)
{
@@ -219,6 +245,8 @@ void AdminModel::setCpuStatsSize(unsigned size)
void AdminModel::setMemStatsSize(unsigned size)
{
+ assert (isCorrectThread());
+
int wasteValuesLen = _memStats.size() - size;
while (wasteValuesLen-- > 0)
{
@@ -231,6 +259,8 @@ void AdminModel::setMemStatsSize(unsigned size)
void AdminModel::notify(const std::string& message)
{
+ assert (isCorrectThread());
+
if (!_subscribers.empty())
{
LOG_TRC("Message to admin console: " << message);
@@ -251,6 +281,8 @@ void AdminModel::notify(const std::string& message)
void AdminModel::addDocument(const std::string& docKey, Poco::Process::PID pid,
const std::string& filename, const std::string& sessionId)
{
+ assert (isCorrectThread());
+
const auto ret = _documents.emplace(docKey, Document(docKey, pid, filename));
ret.first->second.addView(sessionId);
LOG_DBG("Added admin document [" << docKey << "].");
@@ -289,6 +321,8 @@ void AdminModel::addDocument(const std::string& docKey, Poco::Process::PID pid,
void AdminModel::removeDocument(const std::string& docKey, const std::string& sessionId)
{
+ assert (isCorrectThread());
+
auto docIt = _documents.find(docKey);
if (docIt != _documents.end() && !docIt->second.isExpired())
{
@@ -311,6 +345,8 @@ void AdminModel::removeDocument(const std::string& docKey, const std::string& se
void AdminModel::removeDocument(const std::string& docKey)
{
+ assert (isCorrectThread());
+
auto docIt = _documents.find(docKey);
if (docIt != _documents.end())
{
@@ -332,6 +368,8 @@ void AdminModel::removeDocument(const std::string& docKey)
std::string AdminModel::getMemStats()
{
+ assert (isCorrectThread());
+
std::ostringstream oss;
for (const auto& i: _memStats)
{
@@ -343,6 +381,8 @@ std::string AdminModel::getMemStats()
std::string AdminModel::getCpuStats()
{
+ assert (isCorrectThread());
+
std::ostringstream oss;
for (const auto& i: _cpuStats)
{
@@ -354,6 +394,8 @@ std::string AdminModel::getCpuStats()
unsigned AdminModel::getTotalActiveViews()
{
+ assert (isCorrectThread());
+
unsigned numTotalViews = 0;
for (const auto& it: _documents)
{
@@ -368,6 +410,8 @@ unsigned AdminModel::getTotalActiveViews()
std::string AdminModel::getDocuments() const
{
+ assert (isCorrectThread());
+
std::ostringstream oss;
for (const auto& it: _documents)
{
@@ -389,6 +433,8 @@ std::string AdminModel::getDocuments() const
void AdminModel::updateLastActivityTime(const std::string& docKey)
{
+ assert (isCorrectThread());
+
auto docIt = _documents.find(docKey);
if (docIt != _documents.end())
{
@@ -410,6 +456,8 @@ bool Document::updateMemoryDirty(int dirty)
void AdminModel::updateMemoryDirty(const std::string& docKey, int dirty)
{
+ assert (isCorrectThread());
+
auto docIt = _documents.find(docKey);
if (docIt != _documents.end() &&
docIt->second.updateMemoryDirty(dirty))
diff --git a/wsd/AdminModel.hpp b/wsd/AdminModel.hpp
index 8250687c..57bd702b 100644
--- a/wsd/AdminModel.hpp
+++ b/wsd/AdminModel.hpp
@@ -138,7 +138,8 @@ private:
class AdminModel
{
public:
- AdminModel()
+ AdminModel() :
+ _owner(std::this_thread::get_id())
{
LOG_INF("AdminModel ctor.");
}
@@ -148,6 +149,12 @@ public:
LOG_INF("AdminModel dtor.");
}
+ /// All methods here must be called from the Admin socket-poll
+ void setThreadOwner(const std::thread::id &id) { _owner = id; }
+
+ /// In debug mode check that code is running in the correct thread.
+ bool isCorrectThread() const;
+
std::string query(const std::string& command);
/// Returns memory consumed by all active loolkit processes
@@ -199,6 +206,9 @@ private:
std::list<unsigned> _cpuStats;
unsigned _cpuStatsSize = 100;
+
+ // always enabled to avoid ABI change in debug mode ...
+ std::thread::id _owner;
};
#endif
commit b19f0b9cf4e6a7c6b9eaf9c2f4195c269c70f194
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Mar 31 20:58:33 2017 +0100
Join threads to force a reasonably sensible shutdown sequence.
ie. actually wait until documents are saved and sessions closed.
(cherry picked from commit 94022e90d9f76b71c2dfde711188736d4c9f8b08)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index aab632bb..483e7c5e 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -295,6 +295,11 @@ DocumentBroker::~DocumentBroker()
_childProcess.reset();
}
+void DocumentBroker::joinThread()
+{
+ _poll->joinThread();
+}
+
bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const std::string& jailId)
{
assert(isCorrectThread());
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 91dfb63a..168734cc 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -225,6 +225,9 @@ public:
//TODO: Take reason to broadcast to clients.
void stop() { _stop = true; }
+ /// Thread safe termination of this broker if it has a lingering thread
+ void joinThread();
+
/// Loads a document from the public URI into the jail.
bool load(const std::shared_ptr<ClientSession>& session, const std::string& jailId);
bool isLoaded() const { return _isLoaded; }
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index a47a3511..1ac9efa2 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2179,6 +2179,8 @@ public:
{
_stop = true;
SocketPoll::wakeupWorld();
+ _acceptPoll.joinThread();
+ WebServerPoll.joinThread();
}
void dumpState(std::ostream& os)
@@ -2458,10 +2460,11 @@ int LOOLWSD::innerMain()
// Wait until documents are saved and sessions closed.
srv.stop();
- WebServerPoll.stop();
// atexit handlers tend to free Admin before Documents
LOG_INF("Cleaning up lingering documents.");
+ for (auto& docBrokerIt : DocBrokers)
+ docBrokerIt.second->joinThread();
DocBrokers.clear();
#ifndef KIT_IN_PROCESS
@@ -2470,6 +2473,8 @@ int LOOLWSD::innerMain()
SigUtil::killChild(ForKitProcId);
#endif
+ PrisonerPoll.joinThread();
+
// Terminate child processes
LOG_INF("Requesting child processes to terminate.");
for (auto& child : NewChildren)
commit 35fd298f0a78b74d797cc8fd6e71040412e52b42
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Mar 31 17:55:26 2017 +0100
Correct obsolete method name.
(cherry picked from commit 3e1351ec79d5a5804f15ce32f3324ceb93750f47)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 910f328f..aab632bb 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -759,7 +759,7 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
throw;
}
- // Below values are recalculated when startDestroy() is called (before destroying the
+ // Below values are recalculated when destroyIfLastEditor() is called (before destroying the
// document). It is safe to reset their values to their defaults whenever a new session is added.
_lastEditableSession = false;
_markToDestroy = false;
commit baaf3127b2463da5b44054caa598a298242df39a
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 01:02:35 2017 -0400
wsd: remove LOOL_CHECK_THREADS
isCorrectThread now always checks
with ENABLE_DEBUG.
Change-Id: I2b5747f3ab18c8ebdbc92e7ffc86a2469b8c7d13
Reviewed-on: https://gerrit.libreoffice.org/36038
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 8932a1e92e9dbe61841a92d970751ecc41f32a80)
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 7d227732..9cf83c49 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -195,7 +195,7 @@ public:
#endif
}
- virtual bool isCorrectThread(bool hard = false)
+ virtual bool isCorrectThread()
{
#if ENABLE_DEBUG
const bool sameThread = std::this_thread::get_id() == _owner;
@@ -204,12 +204,8 @@ public:
_owner << " but called from 0x" << std::this_thread::get_id() << " (" <<
std::dec << Util::getThreadId() << ").");
- if (hard)
- return sameThread;
- else
- return !getenv("LOOL_CHECK_THREADS") || sameThread;
+ return sameThread;
#else
- (void)hard;
return true;
#endif
}
@@ -455,7 +451,7 @@ public:
{
assert(socket);
assert(isCorrectThread());
- assert(socket->isCorrectThread(true));
+ assert(socket->isCorrectThread());
auto it = std::find(_pollSockets.begin(), _pollSockets.end(), socket);
assert(it != _pollSockets.end());
@@ -650,7 +646,7 @@ public:
/// Send data to the socket peer.
void send(const char* data, const int len, const bool flush = true)
{
- assert(isCorrectThread(true));
+ assert(isCorrectThread());
if (data != nullptr && len > 0)
{
_outBuffer.insert(_outBuffer.end(), data, data + len);
@@ -732,7 +728,7 @@ protected:
HandleResult handlePoll(std::chrono::steady_clock::time_point now,
const int events) override
{
- assert(isCorrectThread(true));
+ assert(isCorrectThread());
_socketHandler->checkTimeout(now);
@@ -800,7 +796,7 @@ protected:
/// Override to write data out to socket.
virtual void writeOutgoingData()
{
- assert(isCorrectThread(true));
+ assert(isCorrectThread());
assert(!_outBuffer.empty());
do
{
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 0cf63a36..8d689de3 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -330,7 +330,7 @@ protected:
if (!socket || data == nullptr || len == 0)
return -1;
- assert(socket->isCorrectThread(true));
+ assert(socket->isCorrectThread());
std::vector<char>& out = socket->_outBuffer;
out.push_back(flags);
commit 7833d4f4096ee42586cf4ae2409165d45640a7d5
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Apr 3 00:45:40 2017 -0400
wsd: remove nextmessage
This was a workaround to Poco's limitation
of requiring socket receiveFrame be given
preallocated buffer, which couldn't be
exceeded by a larger payload. This meant
the receiver had to know the maximum
payload in advance.
Since only the Kit uses Poco sockets,
and the Kit never receives large payloads,
this preamble is now obsolete.
100% (94/94) of old-style tests PASS.
Change-Id: I76776f89497409e5755e335a3e25553e91cf0876
Reviewed-on: https://gerrit.libreoffice.org/36037
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 95d51493aa6f480948f07354f079e1b8d556108c)
diff --git a/common/Common.hpp b/common/Common.hpp
index ab4e1bdd..09bafad3 100644
--- a/common/Common.hpp
+++ b/common/Common.hpp
@@ -25,12 +25,6 @@ constexpr int WS_SEND_TIMEOUT_MS = 1000;
/// which can be 1500 bytes long.
constexpr long READ_BUFFER_SIZE = 64 * 1024;
-/// Size beyond which messages will be sent preceded with
-/// 'nextmessage' frame to let the receiver know in advance
-/// the size of the larger coming message. All messages up to,
-/// but not including, this size are considered small messages.
-constexpr int LARGE_MESSAGE_SIZE = READ_BUFFER_SIZE - 512;
-
/// Message larger than this will be dropped as invalid
/// or as intentionally flooding the server.
constexpr int MAX_MESSAGE_SIZE = 2 * 1024 * READ_BUFFER_SIZE;
diff --git a/common/IoUtil.cpp b/common/IoUtil.cpp
index f94d0b3c..da9a4242 100644
--- a/common/IoUtil.cpp
+++ b/common/IoUtil.cpp
@@ -143,27 +143,6 @@ void SocketProcessor(const std::shared_ptr<LOOLWebSocket>& ws,
}
}
}
- else
- {
- int size = 0;
- Poco::StringTokenizer tokens(firstLine, " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
- // Check if it is a "nextmessage:" and in that case read the large
- // follow-up message separately, and handle that only.
- if (tokens.count() == 2 && tokens[0] == "nextmessage:" &&
- LOOLProtocol::getTokenInteger(tokens[1], "size", size) && size > 0)
- {
- LOG_TRC("SocketProcessor [" << name << "]: Getting large message of " << size << " bytes.");
- if (size > MAX_MESSAGE_SIZE)
- {
- LOG_ERR("SocketProcessor [" << name << "]: Large-message size (" << size << ") over limit or invalid.");
- }
- else
- {
- payload.resize(size);
- continue;
- }
- }
- }
LOG_CHECK(n > 0);
diff --git a/common/LOOLWebSocket.hpp b/common/LOOLWebSocket.hpp
index 24bc7a0e..784a0915 100644
--- a/common/LOOLWebSocket.hpp
+++ b/common/LOOLWebSocket.hpp
@@ -161,23 +161,7 @@ public:
static const Poco::Timespan waitZero(0);
std::unique_lock<std::mutex> lock(_mutexWrite);
- if (length >= LARGE_MESSAGE_SIZE)
- {
- const std::string nextmessage = "nextmessage: size=" + std::to_string(length);
- const int size = nextmessage.size();
-
- if (Poco::Net::WebSocket::sendFrame(nextmessage.data(), size) == size)
- {
- LOG_TRC("Sent long message preample: " + nextmessage);
- }
- else
- {
- LOG_WRN("Failed to send long message preample.");
- return -1;
- }
- }
-
- int result = Poco::Net::WebSocket::sendFrame(buffer, length, flags);
+ const int result = Poco::Net::WebSocket::sendFrame(buffer, length, flags);
lock.unlock();
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 35a48673..40a9a2df 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -659,7 +659,7 @@ public:
return;
}
- LOG_TRC("Sending render-tile response (" + std::to_string(output.size()) + " bytes) for: " + response);
+ LOG_TRC("Sending render-tile response (" << output.size() << " bytes) for: " << response);
ws->sendFrame(output.data(), output.size(), WebSocket::FRAME_BINARY);
}
@@ -741,7 +741,8 @@ public:
if (hash != 0 && tiles[tileIndex].getOldHash() == hash)
{
// The tile content is identical to what the client already has, so skip it
- LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," << positionY << ") oldhash==hash (" << hash << "), skipping");
+ LOG_TRC("Match for tile #" << tileIndex << " at (" << positionX << "," <<
+ positionY << ") oldhash==hash (" << hash << "), skipping");
tiles.erase(tiles.begin() + tileIndex);
continue;
}
@@ -756,7 +757,8 @@ public:
}
const auto imgSize = output.size() - oldSize;
- LOG_TRC("Encoded tile #" << tileIndex << " at (" << positionX << "," << positionY << ") with oldhash=" << tiles[tileIndex].getOldHash() << ", hash=" << hash << " in " << imgSize << " bytes.");
+ LOG_TRC("Encoded tile #" << tileIndex << " at (" << positionX << "," << positionY << ") with oldhash=" <<
+ tiles[tileIndex].getOldHash() << ", hash=" << hash << " in " << imgSize << " bytes.");
tiles[tileIndex].setHash(hash);
tiles[tileIndex].setImgSize(imgSize);
tileIndex++;
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 1c6caf46..0cf63a36 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -311,31 +311,11 @@ public:
/// 0 for closed/invalid socket, and -1 for other errors.
int sendMessage(const char* data, const size_t len, const WSOpCode code, const bool flush = true) const
{
- if (data == nullptr || len == 0)
- return -1;
-
- auto socket = _socket.lock();
- if (socket == nullptr)
- return -1; // no socket == error.
-
- assert(socket->isCorrectThread(true));
- std::vector<char>& out = socket->_outBuffer;
-
//TODO: Support fragmented messages.
- static const unsigned char fin = static_cast<unsigned char>(WSFrameMask::Fin);
-
- // FIXME: need to support fragmented mesages, but for now send prefix message with size.
- if (len >= LARGE_MESSAGE_SIZE)
- {
- const std::string nextmessage = "nextmessage: size=" + std::to_string(len);
- const unsigned char size = (nextmessage.size() & 0xff);
- out.push_back(static_cast<char>(fin | WSOpCode::Text));
- out.push_back(size);
- out.insert(out.end(), nextmessage.data(), nextmessage.data() + size);
- socket->writeOutgoingData();
- }
+ static const unsigned char Fin = static_cast<unsigned char>(WSFrameMask::Fin);
- return sendFrame(socket, data, len, static_cast<unsigned char>(fin | code), flush);
+ auto socket = _socket.lock();
+ return sendFrame(socket, data, len, static_cast<unsigned char>(Fin | code), flush);
}
protected:
diff --git a/test/helpers.hpp b/test/helpers.hpp
index a950de9d..ce6aef98 100644
--- a/test/helpers.hpp
+++ b/test/helpers.hpp
@@ -222,7 +222,7 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
timedout = false;
}
- response.resize(READ_BUFFER_SIZE);
+ response.resize(READ_BUFFER_SIZE * 8);
int bytes = ws.receiveFrame(response.data(), response.size(), flags);
response.resize(std::max(bytes, 0));
std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
@@ -233,22 +233,6 @@ std::vector<char> getResponseMessage(LOOLWebSocket& ws, const std::string& prefi
{
return response;
}
- else if (LOOLProtocol::matchPrefix("nextmessage", message))
- {
- int size = 0;
- if (LOOLProtocol::getTokenIntegerFromMessage(message, "size", size) && size > 0)
- {
- response.resize(size);
- bytes = ws.receiveFrame(response.data(), response.size(), flags);
- response.resize(std::max(bytes, 0));
- std::cerr << name << "Got " << LOOLProtocol::getAbbreviatedFrameDump(response.data(), bytes, flags) << std::endl;
- if (bytes > 0 &&
- LOOLProtocol::matchPrefix(prefix, LOOLProtocol::getFirstLine(response)))
- {
- return response;
- }
- }
- }
}
else
{
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a89e3862..910f328f 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1077,7 +1077,7 @@ void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
}
else
{
- LOG_DBG("Render request declined for " << firstLine);
+ LOG_WRN("Dropping empty tile response: " << firstLine);
// They will get re-issued if we don't forget them.
}
}
@@ -1111,7 +1111,7 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
}
else
{
- LOG_ERR("Render request declined for " << firstLine);
+ LOG_WRN("Dropping empty tilecombine response: " << firstLine);
// They will get re-issued if we don't forget them.
}
}
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 60bf536d..91dfb63a 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -185,9 +185,6 @@ public:
}
private:
- void socketProcessor();
-
-private:
Poco::Process::PID _pid;
std::shared_ptr<WebSocketHandler> _ws;
std::shared_ptr<Socket> _socket;
commit a367552ef4a9ed69e8abef2edc8b928b656dde14
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 23:27:06 2017 -0400
wsd: const correctness and avoid unnecessary shared_ptr promotion
Change-Id: I4352d82e7b5c6873837e73ec04d894dce9a716b7
Reviewed-on: https://gerrit.libreoffice.org/36036
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit b52a8ac6e2806e3ebdc73726040eb70a908a40cc)
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index a7cae8f4..1c6caf46 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -126,15 +126,9 @@ public:
sendFrame(socket, buf.data(), buf.size(), flags);
}
- /// Implementation of the SocketHandlerInterface.
- virtual bool handleOneIncomingMessage()
+ bool handleOneIncomingMessage(const std::shared_ptr<StreamSocket>& socket)
{
- auto socket = _socket.lock();
- if (socket == nullptr)
- {
- LOG_ERR("No socket associated with WebSocketHandler 0x" << std::hex << this << std::dec);
- return false;
- }
+ assert(socket && "Expected a valid socket instance.");
// websocket fun !
const size_t len = socket->_inBuffer.size();
@@ -148,9 +142,9 @@ public:
return false;
unsigned char *p = reinterpret_cast<unsigned char*>(&socket->_inBuffer[0]);
- bool fin = p[0] & 0x80;
- WSOpCode code = static_cast<WSOpCode>(p[0] & 0x0f);
- bool hasMask = p[1] & 0x80;
+ const bool fin = p[0] & 0x80;
+ const WSOpCode code = static_cast<WSOpCode>(p[0] & 0x0f);
+ const bool hasMask = p[1] & 0x80;
size_t payloadLen = p[1] & 0x7f;
size_t headerLen = 2;
@@ -204,7 +198,8 @@ public:
socket->_inBuffer.erase(socket->_inBuffer.begin(), socket->_inBuffer.begin() + headerLen + payloadLen);
// FIXME: fin, aggregating payloads into _wsPayload etc.
- LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " << code << " fin? " << fin << ", payload length: " << _wsPayload.size());
+ LOG_TRC("#" << socket->getFD() << ": Incoming WebSocket message code " << code <<
+ " fin? " << fin << ", mask? " << hasMask << " payload length: " << _wsPayload.size());
switch (code)
{
@@ -254,8 +249,16 @@ public:
/// Implementation of the SocketHandlerInterface.
virtual SocketHandlerInterface::SocketOwnership handleIncomingMessage() override
{
- while (handleOneIncomingMessage())
- ; // can have multiple msgs in one recv'd packet.
+ auto socket = _socket.lock();
+ if (socket == nullptr)
+ {
+ LOG_ERR("No socket associated with WebSocketHandler 0x" << std::hex << this << std::dec);
+ }
+ else
+ {
+ while (handleOneIncomingMessage(socket))
+ ; // can have multiple msgs in one recv'd packet.
+ }
return SocketHandlerInterface::SocketOwnership::UNCHANGED;
}
commit 9f6ad58320f8b40f768935b9329e9997ef99c2f6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 21:33:36 2017 -0400
wsd: process callbacks before poll handlers
Callbacks are used to initialize handlers,
as is the case with addSession on DocumentBroker.
If the socket gets data before the callback is
invoked, the handler will fail since the expected
initialization hasn't happened yet.
This race indeed happens (rarely) with addSession.
100% (94/94) of old-style tests PASS.
Change-Id: Id9b4f63b45c5564add252e1671b7b0b08aff8150
Reviewed-on: https://gerrit.libreoffice.org/36035
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 78d1cc4ac5ec421777085abf2835425954378105)
diff --git a/net/Socket.hpp b/net/Socket.hpp
index e3d9569d..7d227732 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -327,32 +327,7 @@ public:
LOG_TRC("Poll completed with " << rc << " live polls max (" <<
timeoutMaxMs << "ms)" << ((rc==0) ? "(timedout)" : ""));
- // Fire the callback and remove dead fds.
- std::chrono::steady_clock::time_point newNow =
- std::chrono::steady_clock::now();
- for (int i = static_cast<int>(size) - 1; i >= 0; --i)
- {
- Socket::HandleResult res = Socket::HandleResult::SOCKET_CLOSED;
- try
- {
- res = _pollSockets[i]->handlePoll(newNow, _pollFds[i].revents);
- }
- catch (const std::exception& exc)
- {
- LOG_ERR("Error while handling poll for socket #" <<
- _pollFds[i].fd << " in " << _name << ": " << exc.what());
- }
-
- if (res == Socket::HandleResult::SOCKET_CLOSED ||
- res == Socket::HandleResult::MOVED)
- {
- LOG_DBG("Removing socket #" << _pollFds[i].fd << " (of " <<
- _pollSockets.size() << ") from " << _name);
- _pollSockets.erase(_pollSockets.begin() + i);
- }
- }
-
- // Process the wakeup pipe (always the last entry).
+ // First process the wakeup pipe (always the last entry).
if (_pollFds[size].revents)
{
std::vector<CallbackFn> invoke;
@@ -399,6 +374,31 @@ public:
"] wakeup hook: " << exc.what());
}
}
+
+ // Fire the poll callbacks and remove dead fds.
+ std::chrono::steady_clock::time_point newNow =
+ std::chrono::steady_clock::now();
+ for (int i = static_cast<int>(size) - 1; i >= 0; --i)
+ {
+ Socket::HandleResult res = Socket::HandleResult::SOCKET_CLOSED;
+ try
+ {
+ res = _pollSockets[i]->handlePoll(newNow, _pollFds[i].revents);
+ }
+ catch (const std::exception& exc)
+ {
+ LOG_ERR("Error while handling poll for socket #" <<
+ _pollFds[i].fd << " in " << _name << ": " << exc.what());
+ }
+
+ if (res == Socket::HandleResult::SOCKET_CLOSED ||
+ res == Socket::HandleResult::MOVED)
+ {
+ LOG_DBG("Removing socket #" << _pollFds[i].fd << " (of " <<
+ _pollSockets.size() << ") from " << _name);
+ _pollSockets.erase(_pollSockets.begin() + i);
+ }
+ }
}
/// Write to a wakeup descriptor
@@ -430,8 +430,6 @@ public:
if (newSocket)
{
std::lock_guard<std::mutex> lock(_mutex);
- // Beware - _thread may not be created & started yet.
- newSocket->setThreadOwner(_thread.get_id());
LOG_DBG("Inserting socket #" << newSocket->getFD() << " into " << _name);
_newSockets.emplace_back(newSocket);
wakeup();
commit 2220ff2786682096042519a01aec093a700ea906
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 19:56:42 2017 -0400
wsd: do child housekeeping on finishing DocBroker thread
Change-Id: I109737b79759986cb2a1cbfc6d711ee2f19ff59d
Reviewed-on: https://gerrit.libreoffice.org/36034
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit c81db872352443cb0c3ac0f7c0d68f27fcd34963)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index ae7f6e71..a89e3862 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -260,6 +260,9 @@ void DocumentBroker::pollThread()
_poll->poll(std::min(flushTimeoutMs - elapsedMs, POLL_TIMEOUT_MS / 5));
}
+ // Cleanup.
+ LOOLWSD::doHousekeeping();
+
LOG_INF("Finished docBroker polling thread for docKey [" << _docKey << "].");
}
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 69625ca9..a47a3511 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1101,6 +1101,11 @@ bool LOOLWSD::checkAndRestoreForKit()
void PrisonerPoll::wakeupHook()
{
+ LOOLWSD::doHousekeeping();
+}
+
+void LOOLWSD::doHousekeeping()
+{
if (!LOOLWSD::checkAndRestoreForKit())
{
// No children have died.
@@ -1136,11 +1141,6 @@ void PrisonerPoll::wakeupHook()
cleanupDocBrokers();
}
-void LOOLWSD::triggerChildAndDocHousekeeping()
-{
- PrisonerPoll.wakeup();
-}
-
bool LOOLWSD::createForKit()
{
#ifdef KIT_IN_PROCESS
@@ -2428,9 +2428,9 @@ int LOOLWSD::innerMain()
UnitWSD::get().invokeTest();
// This timeout affects the recovery time of prespawned children.
- int msWait = UnitWSD::isUnitTesting() ?
- UnitWSD::get().getTimeoutMilliSeconds() / 4 :
- SocketPoll::DefaultPollTimeoutMs * 4;
+ const int msWait = UnitWSD::isUnitTesting() ?
+ UnitWSD::get().getTimeoutMilliSeconds() / 4 :
+ SocketPoll::DefaultPollTimeoutMs * 4;
mainWait.poll(msWait);
// Wake the prisoner poll to spawn some children, if necessary.
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index b86b23ac..072a4a88 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -100,7 +100,9 @@ public:
/// Return true when successfull.
static bool createForKit();
- static void triggerChildAndDocHousekeeping();
+ /// Checks forkit (and respawns), rebalances
+ /// child kit processes and cleans up DocBrokers.
+ static void doHousekeeping();
protected:
void initialize(Poco::Util::Application& self) override;
commit cf7706acdf1eb340e3b7e2318481f996a05cfe4c
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 19:56:13 2017 -0400
wsd: catch exceptions from callbacks and wakup hook
Change-Id: Ib4579a34c91cfe43e5bd7038b175175a9ab0036a
Reviewed-on: https://gerrit.libreoffice.org/36033
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 6e596d11f3ebe09ad1b50c395f920b591805f4b6)
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 200c389b..e3d9569d 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -377,9 +377,27 @@ public:
}
for (size_t i = 0; i < invoke.size(); ++i)
- invoke[i]();
+ {
+ try
+ {
+ invoke[i]();
+ }
+ catch (const std::exception& exc)
+ {
+ LOG_ERR("Exception while invoking poll [" << _name <<
+ "] callback: " << exc.what());
+ }
+ }
- wakeupHook();
+ try
+ {
+ wakeupHook();
+ }
+ catch (const std::exception& exc)
+ {
+ LOG_ERR("Exception while invoking poll [" << _name <<
+ "] wakeup hook: " << exc.what());
+ }
}
}
commit bc31792cfbc74a79efc942385fe019d6ad30171b
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 18:43:21 2017 -0400
wsd: fix race in setting SocketPoll owner thread id
Change-Id: Idace925ab02425ed66ac07efc22ab933d229d42e
Reviewed-on: https://gerrit.libreoffice.org/36032
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit d7858b08b90f989b0fe08c1f0309e4cd7f82c409)
diff --git a/net/Socket.cpp b/net/Socket.cpp
index 3a07470c..bac3a741 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -52,16 +52,8 @@ SocketPoll::SocketPoll(const std::string& threadName)
throw std::runtime_error("Failed to allocate pipe for SocketPoll [" + threadName + "] waking.");
}
- {
- std::lock_guard<std::mutex> lock(getPollWakeupsMutex());
- getWakeupsArray().push_back(_wakeup[1]);
- }
-
-#if ENABLE_DEBUG
- _owner = std::this_thread::get_id();
- LOG_DBG("Thread affinity of " << _name << " set to 0x" <<
- std::hex << _owner << "." << std::dec);
-#endif
+ std::lock_guard<std::mutex> lock(getPollWakeupsMutex());
+ getWakeupsArray().push_back(_wakeup[1]);
}
SocketPoll::~SocketPoll()
@@ -92,7 +84,6 @@ void SocketPoll::startThread()
try
{
_thread = std::thread(&SocketPoll::pollingThreadEntry, this);
- _owner = _thread.get_id();
}
catch (const std::exception& exc)
{
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 9977f481..200c389b 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -493,6 +493,12 @@ private:
Util::setThreadName(_name);
LOG_INF("Starting polling thread [" << _name << "].");
+#if ENABLE_DEBUG
+ _owner = std::this_thread::get_id();
+ LOG_DBG("Thread affinity of " << _name << " set to 0x" <<
+ std::hex << _owner << "." << std::dec);
+#endif
+
// Invoke the virtual implementation.
pollingThread();
}
commit 463e71ba28b9b8a047b30af68c64ac269b324881
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 17:50:17 2017 -0400
wsd: set DocBroker poll thread once
Change-Id: Ic6397893b2b9b04c6715c393c3f176785b1e2b77
Reviewed-on: https://gerrit.libreoffice.org/36031
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit bd82bf4c4b7ed57d55cd1cf05ea681517b8c1bf3)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 259ca5aa..ae7f6e71 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -155,7 +155,7 @@ DocumentBroker::DocumentBroker(const std::string& uri,
_cursorPosY(0),
_cursorWidth(0),
_cursorHeight(0),
- _poll(new DocumentBrokerPoll("docbrk_poll", *this)),
+ _poll(new DocumentBrokerPoll("docbroker_" + _docId, *this)),
_stop(false),
_tileVersion(0),
_debugRenderedTileCount(0)
@@ -180,8 +180,6 @@ bool DocumentBroker::isCorrectThread()
// The inner heart of the DocumentBroker - our poll loop.
void DocumentBroker::pollThread()
{
- Util::setThreadName("docbroker_" + _docId);
-
LOG_INF("Starting docBroker polling thread for docKey [" << _docKey << "].");
_threadStart = std::chrono::steady_clock::now();
commit 2be22c8cfb171d4bb63e3633f7b0dab9a5880c37
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 17:49:14 2017 -0400
wsd: initialization and logging
Change-Id: Icd82a966b94875a65ddb3817c88a3c4c7bedd4ff
Reviewed-on: https://gerrit.libreoffice.org/36030
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit d6577654bdc45892d9546ecfccdccd549b01921a)
diff --git a/net/Socket.cpp b/net/Socket.cpp
index 2b9899f8..3a07470c 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -43,7 +43,8 @@ SocketPoll::SocketPoll(const std::string& threadName)
: _name(threadName),
_stop(false),
_threadStarted(false),
- _threadFinished(false)
+ _threadFinished(false),
+ _owner(std::this_thread::get_id())
{
// Create the wakeup fd.
if (::pipe2(_wakeup, O_CLOEXEC | O_NONBLOCK) == -1)
@@ -56,9 +57,11 @@ SocketPoll::SocketPoll(const std::string& threadName)
getWakeupsArray().push_back(_wakeup[1]);
}
+#if ENABLE_DEBUG
_owner = std::this_thread::get_id();
LOG_DBG("Thread affinity of " << _name << " set to 0x" <<
std::hex << _owner << "." << std::dec);
+#endif
}
SocketPoll::~SocketPoll()
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 94d4cc34..9977f481 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -47,15 +47,14 @@ public:
Socket() :
_fd(socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0)),
- _sendBufferSize(DefaultSendBufferSize)
+ _sendBufferSize(DefaultSendBufferSize),
+ _owner(std::this_thread::get_id())
{
init();
}
virtual ~Socket()
{
- // TODO: Should we shutdown here or up to the client?
-
LOG_TRC("#" << getFD() << " Socket dtor.");
// Doesn't block on sockets; no error handling needed.
@@ -231,11 +230,13 @@ protected:
_sendBufferSize = DefaultSendBufferSize;
#if ENABLE_DEBUG
_owner = std::this_thread::get_id();
- LOG_DBG("#" << _fd << " Thread affinity set to 0x" << std::hex << _owner << "." << std::dec);
+ LOG_DBG("#" << _fd << " Thread affinity set to 0x" << std::hex <<
+ _owner << "." << std::dec);
const int oldSize = getSocketBufferSize();
setSocketBufferSize(0);
- LOG_TRC("#" << _fd << ": Buffer size: " << getSendBufferSize() << " (was " << oldSize << ")");
+ LOG_TRC("#" << _fd << ": Buffer size: " << getSendBufferSize() <<
+ " (was " << oldSize << ")");
#endif
}
@@ -323,8 +324,8 @@ public:
rc = ::poll(&_pollFds[0], size + 1, std::max(timeoutMaxMs,0));
}
while (rc < 0 && errno == EINTR);
- LOG_TRC("Poll completed with " << rc << " live polls max (" << timeoutMaxMs << "ms)"
- << ((rc==0) ? "(timedout)" : ""));
+ LOG_TRC("Poll completed with " << rc << " live polls max (" <<
+ timeoutMaxMs << "ms)" << ((rc==0) ? "(timedout)" : ""));
// Fire the callback and remove dead fds.
std::chrono::steady_clock::time_point newNow =
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 37bd506f..69625ca9 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1322,9 +1322,7 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
// In case of WOPI, if this session is not set as readonly, it might be set so
// later after making a call to WOPI host which tells us the permission on files
// (UserCanWrite param).
- auto session = std::make_shared<ClientSession>(id, docBroker, uriPublic, isReadOnly);
-
- return session;
+ return std::make_shared<ClientSession>(id, docBroker, uriPublic, isReadOnly);
}
catch (const std::exception& exc)
{
commit 1440b3a98f3cef28794de2c38210554ec15e7256
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 17:38:05 2017 -0400
wsd: better thread affinity logging
Change-Id: I9e4bc3fe864aa409dc4874a9d6fc4ab22bfea592
Reviewed-on: https://gerrit.libreoffice.org/36029
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit e0822f851647b17fca3043c8f17e52ebe8090aa2)
diff --git a/common/Util.cpp b/common/Util.cpp
index 8e9552b6..3260d130 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -265,13 +265,13 @@ namespace Util
strncpy(ThreadName, s.c_str(), 31);
ThreadName[31] = '\0';
if (prctl(PR_SET_NAME, reinterpret_cast<unsigned long>(s.c_str()), 0, 0, 0) != 0)
- {
- LOG_SYS("Cannot set thread name to " << s << ".");
- }
+ LOG_SYS("Cannot set thread name of " << getThreadId() << " (0x" <<
+ std::hex << std::this_thread::get_id() <<
+ std::dec << ") to [" << s << "].");
else
- {
- LOG_INF("Thread " << std::hex << std::this_thread::get_id() << std::dec << " is now called " << s);
- }
+ LOG_INF("Thread " << getThreadId() << " (0x" <<
+ std::hex << std::this_thread::get_id() <<
+ std::dec << ") is now called [" << s << "].");
}
const char *getThreadName()
diff --git a/net/Socket.cpp b/net/Socket.cpp
index 39906bf7..2b9899f8 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -57,6 +57,8 @@ SocketPoll::SocketPoll(const std::string& threadName)
}
_owner = std::this_thread::get_id();
+ LOG_DBG("Thread affinity of " << _name << " set to 0x" <<
+ std::hex << _owner << "." << std::dec);
}
SocketPoll::~SocketPoll()
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 4d723d99..94d4cc34 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -185,7 +185,12 @@ public:
void setThreadOwner(const std::thread::id &id)
{
#if ENABLE_DEBUG
- _owner = id;
+ if (id != _owner)
+ {
+ LOG_DBG("#" << _fd << " Thread affinity set to 0x" << std::hex <<
+ id << " (was 0x" << _owner << ")." << std::dec);
+ _owner = id;
+ }
#else
(void)id;
#endif
@@ -196,8 +201,10 @@ public:
#if ENABLE_DEBUG
const bool sameThread = std::this_thread::get_id() == _owner;
if (!sameThread)
- LOG_WRN("#" << _fd << " invoked from foreign thread. Expected: " <<
- std::hex << _owner << std::dec);
+ LOG_WRN("#" << _fd << " Invoked from foreign thread. Expected: 0x" << std::hex <<
+ _owner << " but called from 0x" << std::this_thread::get_id() << " (" <<
+ std::dec << Util::getThreadId() << ").");
+
if (hard)
return sameThread;
else
@@ -224,12 +231,12 @@ protected:
_sendBufferSize = DefaultSendBufferSize;
#if ENABLE_DEBUG
_owner = std::this_thread::get_id();
+ LOG_DBG("#" << _fd << " Thread affinity set to 0x" << std::hex << _owner << "." << std::dec);
const int oldSize = getSocketBufferSize();
setSocketBufferSize(0);
LOG_TRC("#" << _fd << ": Buffer size: " << getSendBufferSize() << " (was " << oldSize << ")");
#endif
-
}
private:
@@ -291,9 +298,9 @@ public:
bool isCorrectThread() const
{
if (std::this_thread::get_id() != _owner)
- LOG_WRN("Incorrect thread affinity. Expected: 0x" << std::hex << _owner <<
- " but called from " << std::this_thread::get_id() << std::dec <<
- ", stop: " << _stop);
+ LOG_WRN("Incorrect thread affinity for " << _name << ". Expected: 0x" << std::hex <<
+ _owner << " (" << std::dec << Util::getThreadId() << ") but called from 0x" <<
+ std::hex << std::this_thread::get_id() << std::dec << ", stop: " << _stop);
return _stop || std::this_thread::get_id() == _owner;
}
commit 733a3287fac550d2298a55680aad3e0775292657
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 15:55:56 2017 -0400
wsd: log the DocBroker ID in the Kit as well
This matches the document between WSD and kit,
making logs much easier to read.
Change-Id: If55a9eb84b4a22d2dc4dd53f5f6ab322ebc3646e
Reviewed-on: https://gerrit.libreoffice.org/36028
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit c9365ad67961aee7b4be0b23883c973c38c088e1)
diff --git a/kit/Kit.cpp b/kit/Kit.cpp
index 6131ba40..35a48673 100644
--- a/kit/Kit.cpp
+++ b/kit/Kit.cpp
@@ -445,12 +445,14 @@ public:
Document(const std::shared_ptr<lok::Office>& loKit,
const std::string& jailId,
const std::string& docKey,
+ const std::string& docId,
const std::string& url,
std::shared_ptr<TileQueue> tileQueue,
const std::shared_ptr<LOOLWebSocket>& ws)
: _loKit(loKit),
_jailId(jailId),
_docKey(docKey),
+ _docId(docId),
_url(url),
_tileQueue(std::move(tileQueue)),
_ws(ws),
@@ -461,7 +463,9 @@ public:
_stop(false),
_isLoading(0)
{
- LOG_INF("Document ctor for url [" << _url << "] on child [" << _jailId << "].");
+ LOG_INF("Document ctor for [" << _docKey <<
+ "] url [" << _url << "] on child [" << _jailId <<
+ "] and id [" << _docId << "].");
assert(_loKit);
_callbackThread.start(*this);
@@ -469,8 +473,10 @@ public:
~Document()
{
- LOG_INF("~Document dtor for url [" << _url << "] on child [" << _jailId <<
- "]. There are " << _sessions.size() << " views.");
+ LOG_INF("~Document dtor for [" << _docKey <<
+ "] url [" << _url << "] on child [" << _jailId <<
+ "] and id [" << _docId << "]. There are " <<
+ _sessions.size() << " views.");
// Wait for the callback worker to finish.
_stop = true;
@@ -1357,7 +1363,7 @@ private:
void run() override
{
- Util::setThreadName("lok_handler");
+ Util::setThreadName("lokit_" + _docId);
LOG_DBG("Thread started.");
@@ -1515,7 +1521,10 @@ private:
private:
std::shared_ptr<lok::Office> _loKit;
const std::string _jailId;
+ /// URL-based key. May be repeated during the lifetime of WSD.
const std::string _docKey;
+ /// Short numerical ID. Unique during the lifetime of WSD.
+ const std::string _docId;
const std::string _url;
std::string _jailedUrl;
std::string _renderOpts;
@@ -1795,6 +1804,7 @@ void lokit_main(const std::string& childRoot,
{
const std::string& sessionId = tokens[1];
const std::string& docKey = tokens[2];
+ const std::string& docId = tokens[3];
std::string url;
URI::decode(docKey, url);
@@ -1802,7 +1812,7 @@ void lokit_main(const std::string& childRoot,
if (!document)
{
- document = std::make_shared<Document>(loKit, jailId, docKey, url, queue, ws);
+ document = std::make_shared<Document>(loKit, jailId, docKey, docId, url, queue, ws);
}
// Validate and create session.
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 494f21f3..259ca5aa 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -133,6 +133,8 @@ public:
}
};
+std::atomic<unsigned> DocumentBroker::DocBrokerId(1);
+
DocumentBroker::DocumentBroker(const std::string& uri,
const Poco::URI& uriPublic,
const std::string& docKey,
@@ -140,6 +142,7 @@ DocumentBroker::DocumentBroker(const std::string& uri,
_uriOrig(uri),
_uriPublic(uriPublic),
_docKey(docKey),
+ _docId(Util::encodeId(DocBrokerId++, 3)),
_childRoot(childRoot),
_cacheRoot(getCachePath(uriPublic.toString())),
_lastSaveTime(std::chrono::steady_clock::now()),
@@ -177,8 +180,7 @@ bool DocumentBroker::isCorrectThread()
// The inner heart of the DocumentBroker - our poll loop.
void DocumentBroker::pollThread()
{
- static std::atomic<unsigned> DocBrokerId(1);
- Util::setThreadName("docbroker_" + Util::encodeId(DocBrokerId++, 3));
+ Util::setThreadName("docbroker_" + _docId);
LOG_INF("Starting docBroker polling thread for docKey [" << _docKey << "].");
@@ -770,7 +772,7 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
const auto count = _sessions.size();
// Request a new session from the child kit.
- const std::string aMessage = "session " + id + ' ' + _docKey;
+ const std::string aMessage = "session " + id + ' ' + _docKey + ' ' + _docId;
_childProcess->sendTextFrame(aMessage);
// Tell the admin console about this new doc
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 915db5a6..60bf536d 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -359,7 +359,10 @@ private:
private:
const std::string _uriOrig;
const Poco::URI _uriPublic;
+ /// URL-based key. May be repeated during the lifetime of WSD.
const std::string _docKey;
+ /// Short numerical ID. Unique during the lifetime of WSD.
+ const std::string _docId;
const std::string _childRoot;
const std::string _cacheRoot;
std::shared_ptr<ChildProcess> _childProcess;
@@ -405,6 +408,9 @@ private:
std::chrono::steady_clock::time_point _threadStart;
std::chrono::milliseconds _loadDuration;
+ /// Unique DocBroker ID for tracing and debugging.
+ static std::atomic<unsigned> DocBrokerId;
+
static constexpr auto IdleSaveDurationMs = 30 * 1000;
static constexpr auto AutoSaveDurationMs = 300 * 1000;
};
commit e591ba75e2db8781954f406a2e8a8b06a5f5cddb
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sun Apr 2 14:48:59 2017 -0400
wsd: fix alertAllUsers test
Issue the fake disk space error
only after fully loading the doc.
This prevents handling the error
while loading, which terminates
the session.
Change-Id: I5acd9454b1aa9fb5d1f886fb23a76a2d808d4852
Reviewed-on: https://gerrit.libreoffice.org/36027
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 9936ac18697887ae927830258e4c16e0c4726905)
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 22fce917..55272f45 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -2334,28 +2334,15 @@ void HTTPWSTest::testAlertAllUsers()
const auto testname = "alertAllUsers ";
try
{
- std::string docPath[2];
- std::string docURL[2];
-
- getDocumentPathAndURL("Example.odt", docPath[0], docURL[0], testname);
- getDocumentPathAndURL("hello.odt", docPath[1], docURL[1], testname);
-
- Poco::Net::HTTPRequest* request[2];
-
- for (int i = 0; i < 2; i++)
- {
- request[i] = new Poco::Net::HTTPRequest(Poco::Net::HTTPRequest::HTTP_GET, docURL[i]);
- }
-
std::shared_ptr<LOOLWebSocket> socket[4];
- for (int i = 0; i < 2; i++)
- {
- socket[i] = connectLOKit(_uri, *(request[i%2]), _response);
- sendTextFrame(socket[i], "load url=" + docURL[i%2], testname);
- }
+ socket[0] = loadDocAndGetSocket("hello.odt", _uri, testname);
+ socket[1] = loadDocAndGetSocket("Example.odt", _uri, testname);
+
+ // Simulate disk full.
sendTextFrame(socket[0], "uno .uno:fakeDiskFull", testname);
+ // Assert that both clients get the error.
for (int i = 0; i < 2; i++)
{
const std::string response = assertResponseString(socket[i], "error:", testname);
commit b16353970414c275f80cd01b6a37dcc28ea5361a
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sat Apr 1 19:20:54 2017 -0400
wsd: remove queueSession and simplify session loading
Change-Id: Ia03a4ed64b743da8fa7e27de853623126698b9c0
Reviewed-on: https://gerrit.libreoffice.org/36016
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 63ab3bcfa49ec9f6efb0fa81657ea64eaf0ab007)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0a6d2f01..494f21f3 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -223,22 +223,6 @@ void DocumentBroker::pollThread()
// Main polling loop goodness.
while (!_stop && _poll->continuePolling() && !TerminationFlag && !ShutdownRequestFlag)
{
- // First, load new sessions.
- for (const auto& pair : _sessions)
- {
- try
- {
- auto& session = pair.second;
- if (!session->isAttached())
- addSession(session);
- }
- catch (const std::exception& exc)
- {
- LOG_ERR("Error while adding new session to doc [" << _docKey << "]: " << exc.what());
- //TODO: Send failure to client and remove session.
- }
- }
-
_poll->poll(SocketPoll::DefaultPollTimeoutMs);
if (!std::getenv("LOOL_NO_AUTOSAVE") && !_stop &&
@@ -746,16 +730,6 @@ std::string DocumentBroker::getJailRoot() const
return Poco::Path(_childRoot, _jailId).toString();
}
-size_t DocumentBroker::queueSession(std::shared_ptr<ClientSession>& session)
-{
- std::unique_lock<std::mutex> lock(_mutex);
-
- _sessions.emplace(session->getId(), session);
- _poll->wakeup();
-
- return _sessions.size();
-}
-
size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
{
assert(isCorrectThread());
@@ -788,6 +762,8 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
_markToDestroy = false;
_stop = false;
+ // Add and attach the session.
+ _sessions.emplace(session->getId(), session);
session->setAttached();
const auto id = session->getId();
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index a7473531..915db5a6 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -266,10 +266,8 @@ public:
std::string getJailRoot() const;
- /// Queue a new session to be attached asynchronously.
- /// @return amount of session we have after all the queued ones will be
- /// created.
- size_t queueSession(std::shared_ptr<ClientSession>& session);
+ /// Add a new session. Returns the new number of sessions.
+ size_t addSession(const std::shared_ptr<ClientSession>& session);
/// Removes a session by ID. Returns the new number of sessions.
size_t removeSession(const std::string& id, bool destroyIfLast = false);
@@ -354,9 +352,6 @@ private:
/// Forward a message from child session to its respective client session.
bool forwardToClient(const std::shared_ptr<Message>& payload);
- /// Add a new session. Returns the new number of sessions.
- size_t addSession(const std::shared_ptr<ClientSession>& session);
-
/// The thread function that all of the I/O for all sessions
/// associated with this document.
void pollThread();
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index f6e91d0f..37bd506f 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1324,8 +1324,6 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
// (UserCanWrite param).
auto session = std::make_shared<ClientSession>(id, docBroker, uriPublic, isReadOnly);
- docBroker->queueSession(session);
-
return session;
}
catch (const std::exception& exc)
@@ -1860,17 +1858,18 @@ private:
auto clientSession = createNewClientSession(nullptr, _id, uriPublic, docBroker, isReadOnly);
if (clientSession)
{
+ clientSession->setSaveAsSocket(socket);
+
// Transfer the client socket to the DocumentBroker.
// Move the socket into DocBroker.
docBroker->addSocketToPoll(socket);
socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
- clientSession->setSaveAsSocket(socket);
-
- docBroker->startThread();
-
- docBroker->addCallback([&]()
+ docBroker->addCallback([&, clientSession]()
{
+ // First add and load the session.
+ docBroker->addSession(clientSession);
+
// Load the document manually and request saving in the target format.
std::string encodedFrom;
URI::encode(docBroker->getPublicUri().getPath(), "", encodedFrom);
@@ -1891,6 +1890,8 @@ private:
clientSession->handleMessage(true, WebSocketHandler::WSOpCode::Text, saveasRequest);
});
+ docBroker->startThread();
+
sent = true;
}
else
@@ -2026,8 +2027,14 @@ private:
SocketHandlerInterface::SocketOwnership handleClientWsUpgrade(const Poco::Net::HTTPRequest& request, const std::string& url)
{
- // requestHandler = new ClientRequestHandler();
- LOG_INF("Client WS request: " << request.getURI() << ", url: " << url);
+ auto socket = _socket.lock();
+ if (!socket)
+ {
+ LOG_WRN("No socket to handle client WS upgrade for request: " << request.getURI() << ", url: " << url);
+ return SocketHandlerInterface::SocketOwnership::UNCHANGED;
+ }
+
+ LOG_INF("Client WS request: " << request.getURI() << ", url: " << url << ", socket #" << socket->getFD());
// First Upgrade.
WebSocketHandler ws(_socket, request);
@@ -2074,17 +2081,21 @@ private:
if (clientSession)
{
// Transfer the client socket to the DocumentBroker.
- auto socket = _socket.lock();
- if (socket)
+
+ // Set the ClientSession to handle Socket events.
+ socket->setHandler(clientSession);
+ LOG_DBG("Socket #" << socket->getFD() << " handler is " << clientSession->getName());
+
+ // Move the socket into DocBroker.
+ docBroker->addSocketToPoll(socket);
+ socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
+
+ docBroker->addCallback([docBroker, clientSession]()
{
- // Set the ClientSession to handle Socket events.
- socket->setHandler(clientSession);
- LOG_DBG("Socket #" << socket->getFD() << " handler is " << clientSession->getName());
+ // Add and load the session.
+ docBroker->addSession(clientSession);
+ });
- // Move the socket into DocBroker.
- docBroker->addSocketToPoll(socket);
- socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
- }
docBroker->startThread();
}
else
commit 27fbe2f4b9a25f4c35711e05531f8221ea40be1a
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
(cherry picked from commit aeb204fb1480ac269fe4f0e9dac34b34aaf01e8b)
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 09bc5ebd7f359d36096a22e1b574aa3bfbcc9458
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Mar 31 17:18:41 2017 +0100
Add hook for disk space check.
(cherry picked from commit 2bca560feb3fa31ee341347ec5c91c788c7c1ed7)
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 f6b1cbfb24c8093b020861048f72f04da6e73237
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Mar 31 17:06:47 2017 +0100
UnitStorage - first cut at restoring this.
(cherry picked from commit b9f18e63a3b68a13b5357266f349b23d6eeeb28d)
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;
commit 7ec42ccbd2de4b654229ea0b4494b510d849c143
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Mar 31 16:00:32 2017 +0100
UnitTimeout: repair unit test timeout fidelity.
(cherry picked from commit 68bbd40bdc6626061b4d7fc7569fc7622f2bbb4d)
diff --git a/test/Makefile.am b/test/Makefile.am
index 41aa6781..c70bfe84 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/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 1ceaae27..f6e91d0f 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2419,7 +2419,10 @@ int LOOLWSD::innerMain()
UnitWSD::get().invokeTest();
// This timeout affects the recovery time of prespawned children.
- mainWait.poll(SocketPoll::DefaultPollTimeoutMs * 4);
+ int msWait = UnitWSD::isUnitTesting() ?
+ UnitWSD::get().getTimeoutMilliSeconds() / 4 :
+ SocketPoll::DefaultPollTimeoutMs * 4;
+ mainWait.poll(msWait);
// Wake the prisoner poll to spawn some children, if necessary.
PrisonerPoll.wakeup();
commit b9397cc4a45dff331bf4cc6ed5ee9cc181b0bb52
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Fri Mar 31 15:54:26 2017 +0100
Tests: enable tilecache test and bin socket buffer size test.
(cherry picked from commit b70b17130435da4ea1b1ddacd46297407a3858df)
diff --git a/test/Makefile.am b/test/Makefile.am
index 6be2a535..41aa6781 100644
--- a/test/Makefile.am
+++ b/test/Makefile.am
@@ -16,8 +16,7 @@ noinst_LTLIBRARIES = \
unit-timeout.la unit-prefork.la \
unit-storage.la \
unit-admin.la unit-tilecache.la \
- unit-fuzz.la unit-oob.la \
- unit-minsocketbuffersize.la
+ unit-fuzz.la unit-oob.la
MAGIC_TO_FORCE_SHLIB_CREATION = -rpath /dummy
AM_LDFLAGS = -pthread -module $(MAGIC_TO_FORCE_SHLIB_CREATION) $(ZLIB_LIBS)
@@ -61,7 +60,6 @@ unit_timeout_la_SOURCES = UnitTimeout.cpp
unit_prefork_la_SOURCES = UnitPrefork.cpp
unit_storage_la_SOURCES = UnitStorage.cpp
unit_tilecache_la_SOURCES = UnitTileCache.cpp
-unit_minsocketbuffersize_la_SOURCES = UnitMinSocketBufferSize.cpp
if HAVE_LO_PATH
SYSTEM_STAMP = @SYSTEMPLATE_PATH@/system_stamp
@@ -74,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-storage.la unit-timeout.la unit-admin.la unit-minsocketbuffersize.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/UnitMinSocketBufferSize.cpp b/test/UnitMinSocketBufferSize.cpp
deleted file mode 100644
index 6bfb678b..00000000
--- a/test/UnitMinSocketBufferSize.cpp
+++ /dev/null
@@ -1,86 +0,0 @@
-/* -*- Mode: C++; tab-width: 4; indent-tabs-mode: nil; c-basic-offset: 4; fill-column: 100 -*- */
-/*
- * This file is part of the LibreOffice project.
- *
- * This Source Code Form is subject to the terms of the Mozilla Public
- * License, v. 2.0. If a copy of the MPL was not distributed with this
- * file, You can obtain one at http://mozilla.org/MPL/2.0/.
- */
-
-#include "config.h"
-
-#include "Log.hpp"
-#include "Protocol.hpp"
-#include "Unit.hpp"
-#include "UnitHTTP.hpp"
-#include "helpers.hpp"
-
-using namespace helpers;
-
-class UnitMinSocketBufferSize: public UnitWSD
-{
- enum class Phase {
- Load, // load the document
- Request, // Request tiles etc.
- CheckResponse // Check if we got correct response
- } _phase;
- std::string _docURL, _docPath;
- std::unique_ptr<UnitWebSocket> _ws;
-public:
- UnitMinSocketBufferSize() :
- _phase(Phase::Load)
- {
- }
-
- virtual void invokeTest()
- {
- switch (_phase)
- {
- case Phase::Load:
- {
- getDocumentPathAndURL("Example.odt", _docPath, _docURL, "unitMinSocketBufferSize ");
- _ws = std::unique_ptr<UnitWebSocket>(new UnitWebSocket(_docURL));
- assert(_ws.get());
-
- _phase = Phase::Request;
- LOG_DBG("Document loaded successfully.");
- break;
- }
- case Phase::Request:
- {
- const std::string loadMsg = "load url=" + _docURL;
- const std::string tilecombineMsg = "tilecombine part=0 width=256 height=256 tileposx=0,3840,7680,11520,0,3840,7680,11520,0,3840,7680,11520,0,3840,7680,11520,0,3840,7680,11520 tileposy=0,0,0,0,3840,3840,3840,3840,7680,7680,7680,7680,11520,11520,11520,11520,15360,15360,15360,15360 tilewidth=3840 tileheight=3840";
... etc. - the rest is truncated
More information about the Libreoffice-commits
mailing list