[Libreoffice-commits] online.git: Branch 'distro/collabora/collabora-online-2-1' - 10 commits - common/Session.cpp common/Session.hpp net/Socket.cpp net/Socket.hpp net/Ssl.cpp net/Ssl.hpp net/WebSocketHandler.hpp wsd/Admin.cpp wsd/AdminModel.cpp wsd/ClientSession.cpp wsd/ClientSession.hpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp wsd/LOOLWSD.hpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Thu Mar 30 06:31:42 UTC 2017
common/Session.cpp | 6 --
common/Session.hpp | 11 ++++
net/Socket.cpp | 2
net/Socket.hpp | 5 ++
net/Ssl.cpp | 8 ++-
net/Ssl.hpp | 16 +-----
net/WebSocketHandler.hpp | 6 +-
wsd/Admin.cpp | 8 +--
wsd/AdminModel.cpp | 2
wsd/ClientSession.cpp | 14 ++---
wsd/ClientSession.hpp | 7 +-
wsd/DocumentBroker.cpp | 26 ++++++++--
wsd/DocumentBroker.hpp | 4 -
wsd/LOOLWSD.cpp | 112 +++++++++++++++++++++--------------------------
wsd/LOOLWSD.hpp | 6 ++
15 files changed, 129 insertions(+), 104 deletions(-)
New commits:
commit 6d9b3791d791737275392bd569cfeec1b32d8ca6
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 21:02:49 2017 -0400
wsd: handlePostRequest no longer blocks
Change-Id: Id5054ce3b8d5937154493959b1e71e1a0c7387bc
(cherry picked from commit 2fa34440c53778747a17b5c4ad7c3ce638e8da25)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 015f4563..455e03e5 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1664,7 +1664,7 @@ private:
reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
{
// All post requests have url prefix 'lool'.
- socketOwnership = handlePostRequest_Blocks(request, message);
+ socketOwnership = handlePostRequest(request, message);
}
else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws")
{
@@ -1829,7 +1829,7 @@ private:
return "application/octet-stream";
}
- SocketHandlerInterface::SocketOwnership handlePostRequest_Blocks(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
+ SocketHandlerInterface::SocketOwnership handlePostRequest(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
{
LOG_INF("Post request: [" << request.getURI() << "]");
commit d578eed6dd4ba56cfef7d25a55b1305153c4e690
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 21:01:01 2017 -0400
wsd: warn if isCorrectThread will fail
Change-Id: I362b23e651c00a6514bd1e44fa0961269252bcdd
(cherry picked from commit 492b818022c766bab123706d0cec12905f4fa8cf)
diff --git a/net/Socket.hpp b/net/Socket.hpp
index 95384397..8bddac16 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -290,6 +290,11 @@ public:
/// Are we running in either shutdown, or the polling thread.
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);
+
return _stop || std::this_thread::get_id() == _owner;
}
commit a6b92de48e8beae970ae786acd275247af751375
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 20:39:44 2017 -0400
wsd: ClientSession::isLoaded -> isAttached
Since this doesn't necessary mean the document
was loaded completely (as the similarly named
flag in DocumentBroker signifies) rather that
the session was added to DocumentBroker.
Change-Id: Ibfc702bbd111ade2715dcb28ac3aa4e9e8e025dd
(cherry picked from commit 48ad9a247297f9b52b05c63f6eb0a02e81ad6dcc)
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 8c725596..10a11343 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -38,7 +38,7 @@ ClientSession::ClientSession(const std::string& id,
_uriPublic(uriPublic),
_isReadOnly(readOnly),
_isDocumentOwner(false),
- _isLoaded(false),
+ _isAttached(false),
_stop(false)
{
const size_t curConnections = ++LOOLWSD::NumConnections;
@@ -789,7 +789,7 @@ void ClientSession::dumpState(std::ostream& os)
os << "\t\tisReadOnly: " << _isReadOnly
<< "\n\t\tisDocumentOwner: " << _isDocumentOwner
- << "\n\t\tisLoaded: " << _isLoaded
+ << "\n\t\tisAttached: " << _isAttached
<< "\n\t\tstop: " <<_stop
<< "\n";
}
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 58b54b5a..59f8b738 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -36,8 +36,8 @@ public:
bool isReadOnly() const { return _isReadOnly; }
/// Returns true if a document is loaded (i.e. we got status message).
- bool isLoaded() const { return _isLoaded; }
- void setLoaded() { _isLoaded = true; }
+ bool isAttached() const { return _isAttached; }
+ void setAttached() { _isAttached = true; }
const std::string getUserId() const { return _userId; }
void setUserId(const std::string& userId) { _userId = userId; }
@@ -149,7 +149,7 @@ private:
/// The socket to which the converted (saveas) doc is sent.
std::shared_ptr<StreamSocket> _saveAsSocket;
- bool _isLoaded;
+ bool _isAttached;
/// Wopi FileInfo object
std::unique_ptr<WopiStorage::WOPIFileInfo> _wopiFileInfo;
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 29f3a2d4..a0cf1067 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -217,7 +217,7 @@ void DocumentBroker::pollThread()
try
{
auto& session = pair.second;
- if (!session->isLoaded())
+ if (!session->isAttached())
addSession(session);
}
catch (const std::exception& exc)
@@ -772,7 +772,7 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
_markToDestroy = false;
_stop = false;
- session->setLoaded();
+ session->setAttached();
const auto id = session->getId();
const auto count = _sessions.size();
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 7b6e765c..ceb3db48 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -262,7 +262,7 @@ public:
std::string getJailRoot() const;
- /// Queue a new session to be added asynchronously.
+ /// 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);
commit c596946c481fab5a20b9bc029c34dc33a9a94b4e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 20:38:41 2017 -0400
wsd: avoid unnecessary temp string construction
...when sending text messages over socket and
make return value indicative of success/failure.
Change-Id: Ie4d99103b0d49d238152f7da3155ebcb6ccd4e22
(cherry picked from commit 30d58f96a433b4b10d0f6a357a4fd19f2b5df852)
diff --git a/common/Session.cpp b/common/Session.cpp
index 4c5c40cb..4c30f30f 100644
--- a/common/Session.cpp
+++ b/common/Session.cpp
@@ -64,15 +64,13 @@ Session::~Session()
bool Session::sendTextFrame(const char* buffer, const int length)
{
LOG_TRC(getName() << ": Send: " << getAbbreviatedMessage(buffer, length));
- sendMessage(buffer, length, WSOpCode::Text);
- return true;
+ return (sendMessage(buffer, length, WSOpCode::Text) >= length);
}
bool Session::sendBinaryFrame(const char *buffer, int length)
{
LOG_TRC(getName() << ": Send: " << std::to_string(length) << " bytes.");
- sendMessage(buffer, length, WSOpCode::Binary);
- return true;
+ return (sendMessage(buffer, length, WSOpCode::Binary) >= length);
}
void Session::parseDocOptions(const std::vector<std::string>& tokens, int& part, std::string& timestamp)
diff --git a/common/Session.hpp b/common/Session.hpp
index 7c337fe2..76d0d829 100644
--- a/common/Session.hpp
+++ b/common/Session.hpp
@@ -44,6 +44,17 @@ public:
return sendTextFrame(text.data(), text.size());
}
+ template <std::size_t N>
+ bool sendTextFrame(const char (&buffer)[N])
+ {
+ return (buffer != nullptr && N > 0 ? sendTextFrame(buffer, N) : false);
+ }
+
+ bool sendTextFrame(const char* buffer)
+ {
+ return (buffer != nullptr ? sendTextFrame(buffer, std::strlen(buffer)) : false);
+ }
+
virtual void handleMessage(bool fin, WSOpCode code, std::vector<char> &data) override;
/// Invoked when we want to disconnect a session.
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index d0c4585c..ac04da53 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -299,9 +299,9 @@ public:
void performWrites() override {}
/// Sends a WebSocket Text message.
- void sendMessage(const std::string& msg) const
+ int sendMessage(const std::string& msg) const
{
- sendMessage(msg.data(), msg.size(), WSOpCode::Text);
+ return sendMessage(msg.data(), msg.size(), WSOpCode::Text);
}
/// Sends a WebSocket message of WPOpCode type.
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 6314983b..8c725596 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -464,9 +464,9 @@ void ClientSession::performWrites()
std::shared_ptr<Message> item;
if (_senderQueue.dequeue(item))
{
- const std::vector<char>& data = item->data();
try
{
+ const std::vector<char>& data = item->data();
if (item->isBinary())
{
Session::sendBinaryFrame(data.data(), data.size());
@@ -478,8 +478,8 @@ void ClientSession::performWrites()
}
catch (const std::exception& ex)
{
- LOG_ERR("Failed to send message [" << LOOLProtocol::getAbbreviatedMessage(data) <<
- "] to " << getName() << ": " << ex.what());
+ LOG_ERR("Failed to send message " << item->abbr() <<
+ " to " << getName() << ": " << ex.what());
}
}
diff --git a/wsd/ClientSession.hpp b/wsd/ClientSession.hpp
index 5c505a43..58b54b5a 100644
--- a/wsd/ClientSession.hpp
+++ b/wsd/ClientSession.hpp
@@ -48,6 +48,7 @@ public:
/// Handle kit-to-client message.
bool handleKitToClientMessage(const char* data, const int size);
+ // sendTextFrame that takes std::string and string literal.
using Session::sendTextFrame;
bool sendBinaryFrame(const char* buffer, int length) override
commit b42f7a59a5b462cd1f7e874784ddac233ff3c2b2
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 20:03:01 2017 -0400
wsd: consistent naming sendFrame -> sendMessage
Change-Id: I06c6bb42392a8982a8bb232eee33ece4c8dfc451
(cherry picked from commit d77ede8954b88a0ba63e0c71e20a6429e59a2290)
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index ce0d13e9..d0c4585c 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -299,7 +299,7 @@ public:
void performWrites() override {}
/// Sends a WebSocket Text message.
- void sendFrame(const std::string& msg) const
+ void sendMessage(const std::string& msg) const
{
sendMessage(msg.data(), msg.size(), WSOpCode::Text);
}
@@ -321,7 +321,7 @@ public:
std::vector<char>& out = socket->_outBuffer;
//TODO: Support fragmented messages.
- const unsigned char fin = static_cast<unsigned char>(WSFrameMask::Fin);
+ 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)
diff --git a/wsd/Admin.cpp b/wsd/Admin.cpp
index bb235156..4ab3d821 100644
--- a/wsd/Admin.cpp
+++ b/wsd/Admin.cpp
@@ -65,7 +65,7 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
if (tokens.count() < 2)
{
LOG_DBG("Auth command without any token");
- sendFrame("InvalidAuthToken");
+ sendMessage("InvalidAuthToken");
shutdown();
return;
}
@@ -85,7 +85,7 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
else
{
LOG_DBG("Invalid auth token");
- sendFrame("InvalidAuthToken");
+ sendMessage("InvalidAuthToken");
shutdown();
return;
}
@@ -95,7 +95,7 @@ void AdminSocketHandler::handleMessage(bool /* fin */, WSOpCode /* code */,
{
LOG_DBG("Not authenticated - message is '" << firstLine << "' " <<
tokens.count() << " first: '" << tokens[0] << "'");
- sendFrame("NotAuthenticated");
+ sendMessage("NotAuthenticated");
shutdown();
return;
}
@@ -240,7 +240,7 @@ void AdminSocketHandler::sendTextFrame(const std::string& message)
{
UnitWSD::get().onAdminQueryMessage(message);
if (_isAuthenticated)
- sendFrame(message);
+ sendMessage(message);
else
LOG_TRC("Skip sending message to non-authenticated client: '" << message << "'");
}
diff --git a/wsd/AdminModel.cpp b/wsd/AdminModel.cpp
index c23641c6..5e1aefc7 100644
--- a/wsd/AdminModel.cpp
+++ b/wsd/AdminModel.cpp
@@ -70,7 +70,7 @@ bool Subscriber::notify(const std::string& message)
try
{
UnitWSD::get().onAdminNotifyMessage(message);
- webSocket->sendFrame(message);
+ webSocket->sendMessage(message);
return true;
}
catch (const std::exception& ex)
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 40d78f1d..6314983b 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -747,7 +747,7 @@ void ClientSession::onDisconnect()
LOG_ERR("Error in client request handler: " << exc.toString());
const std::string status = "error: cmd=internal kind=unauthorized";
LOG_TRC("Sending to Client [" << status << "].");
- sendFrame(status);
+ sendMessage(status);
}
catch (const std::exception& exc)
{
@@ -773,7 +773,7 @@ void ClientSession::onDisconnect()
else
{
static const std::string msg("close: recycling");
- sendFrame(msg);
+ sendMessage(msg);
shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY);
}
}
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 65e57fed..29f3a2d4 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -194,7 +194,7 @@ void DocumentBroker::pollThread()
// FIXME: need to notify all clients and shut this down ...
#if 0
const std::string msg = SERVICE_UNAVAILABLE_INTERNAL_ERROR;
- ws.sendFrame(msg);
+ ws.sendMessage(msg);
// abnormal close frame handshake
ws.shutdown(WebSocketHandler::StatusCodes::ENDPOINT_GOING_AWAY);
#endif
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 8f670c53..7b6e765c 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -153,7 +153,7 @@ public:
if (_ws)
{
LOG_TRC("DocBroker to Child: " << data);
- _ws->sendFrame(data);
+ _ws->sendMessage(data);
return true;
}
}
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 544901c0..015f4563 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -196,7 +196,7 @@ inline void shutdownLimitReached(WebSocketHandler& ws)
try
{
// Let the client know we are shutting down.
- ws.sendFrame(error);
+ ws.sendMessage(error);
// Shutdown.
ws.shutdown(WebSocketHandler::StatusCodes::POLICY_VIOLATION);
@@ -1283,7 +1283,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w
// Indicate to the client that we're connecting to the docbroker.
const std::string statusConnect = "statusindicator: connect";
LOG_TRC("Sending to Client [" << statusConnect << "].");
- ws.sendFrame(statusConnect);
+ ws.sendMessage(statusConnect);
if (!docBroker)
{
@@ -1330,7 +1330,7 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
{
const std::string statusReady = "statusindicator: ready";
LOG_TRC("Sending to Client [" << statusReady << "].");
- ws->sendFrame(statusReady);
+ ws->sendMessage(statusReady);
}
// In case of WOPI, if this session is not set as readonly, it might be set so
@@ -2059,7 +2059,7 @@ private:
// Indicate to the client that document broker is searching.
const std::string status("statusindicator: find");
LOG_TRC("Sending to Client [" << status << "].");
- ws.sendFrame(status);
+ ws.sendMessage(status);
const auto uriPublic = DocumentBroker::sanitizeURI(url);
const auto docKey = DocumentBroker::getDocKey(uriPublic);
commit 7b0e6a44d8ba158a3a5333457148cd7b3c423b0f
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 27 20:50:30 2017 -0400
wsd: flush sockets using timed loops
Change-Id: If6cc6194aa36a770913b777a8f19617fea6ba4a7
Reviewed-on: https://gerrit.libreoffice.org/35786
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 06249045006c438e41ec7037778fcc57017735ec)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 339edaea..65e57fed 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -248,8 +248,17 @@ void DocumentBroker::pollThread()
}
// Flush socket data.
- for (int i = 0; i < 7 && _poll->getSocketCount() > 0; ++i)
- _poll->poll(POLL_TIMEOUT_MS / 5);
+ const int flushTimeoutMs = POLL_TIMEOUT_MS * 2; // ~1000ms
+ const auto flushStartTime = std::chrono::steady_clock::now();
+ while (_poll->getSocketCount())
+ {
+ const auto now = std::chrono::steady_clock::now();
+ const int elapsedMs = std::chrono::duration_cast<std::chrono::milliseconds>(now - flushStartTime).count();
+ if (elapsedMs > flushTimeoutMs)
+ break;
+
+ _poll->poll(std::min(flushTimeoutMs - elapsedMs, POLL_TIMEOUT_MS / 5));
+ }
// Terminate properly while we can.
auto lock = getLock();
commit 4a7c0c683de07cacdb20b860658250d4f5072833
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Mon Mar 27 20:14:16 2017 +0100
tdf#106797 - avoid locking up / crashing on exceptions from main.
(cherry picked from commit e26079eecb78947f2394fcad8092483890946773)
diff --git a/net/Ssl.cpp b/net/Ssl.cpp
index 6e266c70..74f606cf 100644
--- a/net/Ssl.cpp
+++ b/net/Ssl.cpp
@@ -9,6 +9,7 @@
#include "config.h"
+#include <assert.h>
#include "Ssl.hpp"
#include <sys/syscall.h>
@@ -25,7 +26,6 @@ extern "C"
};
}
-std::atomic<int> SslContext::RefCount(0);
std::unique_ptr<SslContext> SslContext::Instance;
std::vector<std::unique_ptr<std::mutex>> SslContext::Mutexes;
@@ -132,6 +132,12 @@ SslContext::~SslContext()
CONF_modules_free();
}
+void SslContext::uninitialize()
+{
+ assert (Instance);
+ Instance.reset();
+}
+
void SslContext::lock(int mode, int n, const char* /*file*/, int /*line*/)
{
if (mode & CRYPTO_LOCK)
diff --git a/net/Ssl.hpp b/net/Ssl.hpp
index 2c1cabd7..7c13474a 100644
--- a/net/Ssl.hpp
+++ b/net/Ssl.hpp
@@ -31,19 +31,11 @@ public:
const std::string& keyFilePath,
const std::string& caFilePath)
{
- if (++RefCount == 1)
- {
- Instance.reset(new SslContext(certFilePath, keyFilePath, caFilePath));
- }
+ assert (!Instance);
+ Instance.reset(new SslContext(certFilePath, keyFilePath, caFilePath));
}
- static void uninitialize()
- {
- if (--RefCount == 0)
- {
- Instance.reset();
- }
- }
+ static void uninitialize();
static SSL* newSsl()
{
@@ -59,6 +51,7 @@ private:
void initDH();
void initECDH();
+ void shutdown();
std::string getLastErrorMsg();
@@ -71,7 +64,6 @@ private:
static void dynlockDestroy(struct CRYPTO_dynlock_value* lock, const char* file, int line);
private:
- static std::atomic<int> RefCount;
static std::unique_ptr<SslContext> Instance;
static std::vector<std::unique_ptr<std::mutex>> Mutexes;
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index fa35e900..544901c0 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -2322,7 +2322,7 @@ bool LOOLWSD::handleShutdownRequest()
return false;
}
-int LOOLWSD::main(const std::vector<std::string>& /*args*/)
+int LOOLWSD::innerMain()
{
#ifndef FUZZER
SigUtil::setUserSignals();
@@ -2404,6 +2404,7 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
{
const auto msg = "Failed to fork child processes.";
LOG_FTL(msg);
+ std::cerr << msg << std::endl;
throw std::runtime_error(msg);
}
@@ -2492,7 +2493,11 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
LOG_INF("Removing jail [" << path << "].");
FileUtil::removeFile(path, true);
}
+ return Application::EXIT_OK;
+}
+void LOOLWSD::cleanup()
+{
// Finally, we no longer need SSL.
if (LOOLWSD::isSSLEnabled())
{
@@ -2502,14 +2507,28 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
SslContext::uninitialize();
#endif
}
+}
+
+int LOOLWSD::main(const std::vector<std::string>& /*args*/)
+{
+ int returnValue;
+
+ try {
+ returnValue = innerMain();
+ } catch (...) {
+ cleanup();
+ throw;
+ }
+
+ cleanup();
- int returnValue = Application::EXIT_OK;
UnitWSD::get().returnValue(returnValue);
LOG_INF("Process [loolwsd] finished.");
return returnValue;
}
+
void UnitWSD::testHandleRequest(TestRequest type, UnitHTTPServerRequest& /* request */, UnitHTTPServerResponse& /* response */)
{
switch (type)
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index 10c42c32..d20314c1 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -105,8 +105,12 @@ protected:
void initialize(Poco::Util::Application& self) override;
void defineOptions(Poco::Util::OptionSet& options) override;
void handleOption(const std::string& name, const std::string& value) override;
+ int innerMain();
int main(const std::vector<std::string>& args) override;
+ /// Handle various global static destructors.
+ void cleanup();
+
private:
static Util::RuntimeConstant<bool> SSLEnabled;
static Util::RuntimeConstant<bool> SSLTermination;
commit 54a7f69681ea56add3423760ac6342027841920e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Mar 28 01:07:07 2017 -0400
wsd: simplify and cleanup session creation
Change-Id: I8cc05bc7a8dc89c6a521b81c6d59ff1e9968763a
Reviewed-on: https://gerrit.libreoffice.org/35789
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit fbf3b87626ab67d112813d5a207db5cb780ce701)
diff --git a/net/Socket.cpp b/net/Socket.cpp
index 008070e3..cb863b1f 100644
--- a/net/Socket.cpp
+++ b/net/Socket.cpp
@@ -47,7 +47,7 @@ SocketPoll::SocketPoll(const std::string& threadName)
// Create the wakeup fd.
if (::pipe2(_wakeup, O_CLOEXEC | O_NONBLOCK) == -1)
{
- throw std::runtime_error("Failed to allocate pipe for SocketPoll waking.");
+ throw std::runtime_error("Failed to allocate pipe for SocketPoll [" + threadName + "] waking.");
}
{
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 0398dda2..339edaea 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -723,7 +723,7 @@ std::string DocumentBroker::getJailRoot() const
size_t DocumentBroker::queueSession(std::shared_ptr<ClientSession>& session)
{
- Util::assertIsLocked(_mutex);
+ std::unique_lock<std::mutex> lock(_mutex);
_sessions.emplace(session->getId(), session);
_poll->wakeup();
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 9a70ac7f..fa35e900 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1261,9 +1261,7 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w
LOG_DBG("Found DocumentBroker with docKey [" << docKey << "].");
docBroker = it->second;
- // Avoid notifying the client - either we catch and stop the
- // destruction when we add the session, -or- the client
- // re-connects.
+ // Destroying the document? Let the client reconnect.
if (docBroker->isMarkedToDestroy())
{
LOG_WRN("DocBroker with docKey [" << docKey << "] that is marked to be destroyed. Rejecting client request.");
@@ -1309,28 +1307,6 @@ static std::shared_ptr<DocumentBroker> findOrCreateDocBroker(WebSocketHandler& w
return docBroker;
}
-/// Remove DocumentBroker session and instance from DocBrokers.
-static void removeDocBrokerSession(const std::shared_ptr<DocumentBroker>& docBroker, const std::string& id = "")
-{
- LOG_CHECK_RET(docBroker && "Null docBroker instance", );
-
- const auto docKey = docBroker->getDocKey();
- LOG_DBG("Removing docBroker [" << docKey << "]" << (id.empty() ? "" : (" and session [" + id + "].")));
-
- std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex);
- auto lock = docBroker->getLock();
-
- if (!id.empty())
- docBroker->removeSession(id);
-
- if (docBroker->getSessionsCount() == 0 || !docBroker->isAlive())
- {
- LOG_INF("Removing unloaded DocumentBroker for docKey [" << docKey << "].");
- DocBrokers.erase(docKey);
- docBroker->terminateChild(lock, "", true);
- }
-}
-
static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHandler* ws,
const std::string& id,
const Poco::URI& uriPublic,
@@ -1340,10 +1316,14 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
LOG_CHECK_RET(docBroker && "Null docBroker instance", nullptr);
try
{
- auto lock = docBroker->getLock();
-
- if (docBroker->isMarkedToDestroy())
- LOG_WRN("DocBroker is marked to destroy, attempting to add session anyway.");
+ const std::string fs = FileUtil::checkDiskSpaceOnRegisteredFileSystems();
+ if (!fs.empty())
+ {
+ LOG_WRN("File system of [" << fs << "] is dangerously low on disk space.");
+ const std::string diskfullMsg = "error: cmd=internal kind=diskfull";
+ // Alert all other existing sessions also
+ Util::alertAllUsers(diskfullMsg);
+ }
// Now we have a DocumentBroker and we're ready to process client commands.
if (ws)
@@ -1360,23 +1340,11 @@ static std::shared_ptr<ClientSession> createNewClientSession(const WebSocketHand
docBroker->queueSession(session);
- lock.unlock();
-
- const std::string fs = FileUtil::checkDiskSpaceOnRegisteredFileSystems();
- if (!fs.empty())
- {
- LOG_WRN("File system of [" << fs << "] is dangerously low on disk space.");
- const std::string diskfullMsg = "error: cmd=internal kind=diskfull";
- // Alert all other existing sessions also
- Util::alertAllUsers(diskfullMsg);
- }
-
return session;
}
catch (const std::exception& exc)
{
LOG_WRN("Exception while preparing session [" << id << "]: " << exc.what());
- removeDocBrokerSession(docBroker, id);
}
return nullptr;
@@ -2117,7 +2085,6 @@ private:
auto docBroker = findOrCreateDocBroker(ws, url, docKey, _id, uriPublic);
if (docBroker)
{
- // TODO: Move to DocumentBroker.
auto clientSession = createNewClientSession(&ws, _id, uriPublic, docBroker, isReadOnly);
if (clientSession)
{
@@ -2136,7 +2103,10 @@ private:
docBroker->startThread();
}
else
+ {
LOG_WRN("Failed to create Client Session with id [" << _id << "] on docKey [" << docKey << "].");
+ cleanupDocBrokers();
+ }
}
else
LOG_WRN("Failed to create DocBroker with docKey [" << docKey << "].");
commit cd5cc83f0ace6a4ca8f8dddb5b1c4f45288138d1
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Tue Mar 28 01:05:18 2017 -0400
wsd: protect DocBrokers session member
It is accessed from the PrisonerPoll
when cleaning up.
Change-Id: Ieb57cdd63cc08632bcdaa4fc5ccd4a1a53c06fe7
Reviewed-on: https://gerrit.libreoffice.org/35788
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 99844c1c924e6c2de8011c9f680151f228e74f61)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 4b271021..0398dda2 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -516,6 +516,8 @@ bool DocumentBroker::saveToStorageInternal(const std::string& sessionId,
return true;
}
+ std::unique_lock<std::mutex> lock(_mutex);
+
const auto it = _sessions.find(sessionId);
if (it == _sessions.end())
{
commit 05a16501b16ce557344d6c1180e407a5eb75d86e
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 27 21:46:16 2017 -0400
wsd: trigger child and doc housekeeping upon terminating kit
Change-Id: I7ffbadb40221c19b24fd172d1b9aabcaa8c581e5
Reviewed-on: https://gerrit.libreoffice.org/35787
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
(cherry picked from commit 1a6f6e9a65559756ead5c61804744e520ea749d3)
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index 97b81fdd..4b271021 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -1295,6 +1295,9 @@ void DocumentBroker::terminateChild(std::unique_lock<std::mutex>& lock, const st
// Stop the polling thread.
_poll->stop();
_stop = true;
+
+ // Trigger cleanup.
+ LOOLWSD::triggerChildAndDocHousekeeping();
}
void DocumentBroker::closeDocument(const std::string& reason)
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index bdad1aaf..9a70ac7f 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1112,9 +1112,6 @@ bool LOOLWSD::checkAndRestoreForKit()
void PrisonerPoll::wakeupHook()
{
- /// FIXME: we should do this less frequently
- /// currently the prisoner poll wakes up quite
- /// a lot.
if (!LOOLWSD::checkAndRestoreForKit())
{
// No children have died.
@@ -1144,6 +1141,18 @@ void PrisonerPoll::wakeupHook()
#endif
}
}
+
+ std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex, std::defer_lock);
+ if (docBrokersLock.try_lock())
+ {
+ cleanupDocBrokers();
+ }
+}
+
+void LOOLWSD::triggerChildAndDocHousekeeping()
+{
+ PrisonerPoll.wakeup();
+
}
bool LOOLWSD::createForKit()
@@ -2450,7 +2459,7 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
UnitWSD::get().invokeTest();
// This timeout affects the recovery time of prespawned children.
- mainWait.poll(SocketPoll::DefaultPollTimeoutMs);
+ mainWait.poll(SocketPoll::DefaultPollTimeoutMs * 4);
// Wake the prisoner poll to spawn some children, if necessary.
PrisonerPoll.wakeup();
@@ -2461,12 +2470,6 @@ int LOOLWSD::main(const std::vector<std::string>& /*args*/)
UnitWSD::get().getTimeoutMilliSeconds())
UnitWSD::get().timeout();
- std::unique_lock<std::mutex> docBrokersLock(DocBrokersMutex, std::defer_lock);
- if (docBrokersLock.try_lock())
- {
- cleanupDocBrokers();
- }
-
#if ENABLE_DEBUG
if (careerSpanSeconds > 0 && time(nullptr) > startTimeSpan + careerSpanSeconds)
{
diff --git a/wsd/LOOLWSD.hpp b/wsd/LOOLWSD.hpp
index 94ff1475..10c42c32 100644
--- a/wsd/LOOLWSD.hpp
+++ b/wsd/LOOLWSD.hpp
@@ -99,6 +99,8 @@ public:
/// Return true when successfull.
static bool createForKit();
+ static void triggerChildAndDocHousekeeping();
+
protected:
void initialize(Poco::Util::Application& self) override;
void defineOptions(Poco::Util::OptionSet& options) override;
More information about the Libreoffice-commits
mailing list