[Libreoffice-commits] online.git: 7 commits - net/Socket.hpp net/WebSocketHandler.hpp test/httpcrashtest.cpp test/httpwstest.cpp wsd/DocumentBroker.cpp wsd/DocumentBroker.hpp wsd/LOOLWSD.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Tue Mar 21 03:26:40 UTC 2017
net/Socket.hpp | 3 ++-
net/WebSocketHandler.hpp | 5 +++--
test/httpcrashtest.cpp | 27 +++++++++++++++++++++++++++
test/httpwstest.cpp | 2 +-
wsd/DocumentBroker.cpp | 23 +++++++++++++++++++----
wsd/DocumentBroker.hpp | 3 +++
wsd/LOOLWSD.cpp | 28 +++++++++++++++++++---------
7 files changed, 74 insertions(+), 17 deletions(-)
New commits:
commit dc9049951404eab3b9bf90ed9a25376bc1b422d5
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 22:59:08 2017 -0400
wsd: remove the socket on move
Avoid explicit socket removal too.
Change-Id: I44d773761a5a463aad828c19c6b394bb6bac63d8
diff --git a/net/Socket.hpp b/net/Socket.hpp
index c40802f1..e0ecc807 100644
--- a/net/Socket.hpp
+++ b/net/Socket.hpp
@@ -329,7 +329,8 @@ public:
_pollFds[i].fd << " in " << _name << ": " << exc.what());
}
- if (res == Socket::HandleResult::SOCKET_CLOSED)
+ if (res == Socket::HandleResult::SOCKET_CLOSED ||
+ res == Socket::HandleResult::MOVED)
{
LOG_DBG("Removing socket #" << _pollFds[i].fd << " (of " <<
_pollSockets.size() << ") from " << _name);
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 1a5732d2..b93098c6 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1694,8 +1694,8 @@ private:
if (AdminSocketHandler::handleInitialRequest(_socket, request))
{
// Hand the socket over to the Admin poll.
- WebServerPoll.releaseSocket(socket);
Admin::instance().insertNewSocket(socket);
+ socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
}
}
// Client post and websocket connections
@@ -1720,7 +1720,7 @@ private:
reqPathTokens.count() > 0 && reqPathTokens[0] == "lool")
{
// All post requests have url prefix 'lool'.
- handlePostRequest_Blocks(request, message);
+ socketOwnership = handlePostRequest_Blocks(request, message);
}
else if (reqPathTokens.count() > 2 && reqPathTokens[0] == "lool" && reqPathTokens[2] == "ws")
{
@@ -1885,13 +1885,14 @@ private:
return "application/octet-stream";
}
- void handlePostRequest_Blocks(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
+ SocketHandlerInterface::SocketOwnership handlePostRequest_Blocks(const Poco::Net::HTTPRequest& request, Poco::MemoryInputStream& message)
{
LOG_INF("Post request: [" << request.getURI() << "]");
Poco::Net::HTTPResponse response;
auto socket = _socket.lock();
+ SocketHandlerInterface::SocketOwnership socketOwnership = SocketHandlerInterface::SocketOwnership::UNCHANGED;
StringTokenizer tokens(request.getURI(), "/?");
if (tokens.count() >= 3 && tokens[2] == "convert-to")
{
@@ -1932,8 +1933,8 @@ private:
{
// Transfer the client socket to the DocumentBroker.
// Move the socket into DocBroker.
- WebServerPoll.releaseSocket(socket);
docBroker->addSocketToPoll(socket);
+ socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
clientSession->setSaveAsSocket(socket);
@@ -1971,7 +1972,7 @@ private:
throw BadRequestException("Failed to convert and send file.");
}
- return;
+ return socketOwnership;
}
else if (tokens.count() >= 4 && tokens[3] == "insertfile")
{
@@ -2014,7 +2015,7 @@ private:
File(tmpPath).moveTo(fileName);
response.setContentLength(0);
socket->send(response);
- return;
+ return socketOwnership;
}
}
}
@@ -2085,7 +2086,7 @@ private:
}
(void)responded;
- return; // responded;
+ return socketOwnership;
}
throw BadRequestException("Invalid or unknown request.");
@@ -2149,7 +2150,6 @@ private:
socket->setHandler(clientSession);
// Move the socket into DocBroker.
- WebServerPoll.releaseSocket(socket);
docBroker->addSocketToPoll(socket);
socketOwnership = SocketHandlerInterface::SocketOwnership::MOVED;
}
commit 5eb6607f086864df357b0300d755eca5fabde204
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 22:58:28 2017 -0400
wsd: terminate the DocBroker when the kit disconnects
This can happen most notably when the kit crashes.
In this case the DocBroker is terminated so all
client connections are closed, allowing clients
to re-connect and re-open the doc.
Change-Id: I9854e11b002ca6e3c2a1a0bbb91ca087679d25bb
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index ba13caba..1a5732d2 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -1386,7 +1386,8 @@ public:
if (docBroker)
{
// FIXME: No need to notify if asked to stop.
- docBroker->childSocketTerminated();
+ auto lock = docBroker->getLock();
+ docBroker->terminateChild(lock, "Service unavailable", true);
}
}
@@ -1401,6 +1402,15 @@ private:
void onDisconnect() override
{
LOG_TRC("Prisoner connection disconnected");
+
+ // Notify the broker that we're done.
+ auto child = _childProcess.lock();
+ auto docBroker = child ? child->getDocumentBroker() : nullptr;
+ if (docBroker)
+ {
+ auto lock = docBroker->getLock();
+ docBroker->terminateChild(lock, "Service unavailable", false);
+ }
}
/// Called after successful socket reads.
commit b01e555b4709d798c3809c888a4b20f8d22eaabd
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 22:55:52 2017 -0400
wsd: correct copying of socket shutdown message
Avoid segfaulting.
Change-Id: I320606937a0d3a3e53270a70ef0a00fcb01b855a
diff --git a/net/WebSocketHandler.hpp b/net/WebSocketHandler.hpp
index 9fe7aab6..64b057b9 100644
--- a/net/WebSocketHandler.hpp
+++ b/net/WebSocketHandler.hpp
@@ -113,8 +113,9 @@ public:
std::vector<char> buf(2 + len);
buf[0] = ((((int)statusCode) >> 8) & 0xff);
buf[1] = ((((int)statusCode) >> 0) & 0xff);
- std::copy(statusMessage.begin(), statusMessage.end(), buf.end());
- const unsigned char flags = static_cast<unsigned char>(WSFrameMask::Fin) | static_cast<char>(WSOpCode::Close);
+ std::copy(statusMessage.begin(), statusMessage.end(), buf.begin() + 2);
+ const unsigned char flags = static_cast<unsigned char>(WSFrameMask::Fin)
+ | static_cast<char>(WSOpCode::Close);
auto lock = socket->getWriteLock();
sendFrame(socket, buf.data(), buf.size(), flags);
commit 4df1e3c8f20b0fad919c8ac25361fc7301de8900
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 21:57:21 2017 -0400
wsd: timeout after last save before terminating DocBroker
Change-Id: I87430285be7350ffbd3dfd6516257ed2b4ce1869
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index f0e79f59..cfdec34a 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -143,6 +143,7 @@ DocumentBroker::DocumentBroker(const std::string& uri,
_childRoot(childRoot),
_cacheRoot(getCachePath(uriPublic.toString())),
_lastSaveTime(std::chrono::steady_clock::now()),
+ _lastSaveRequestTime(std::chrono::steady_clock::now()),
_markToDestroy(false),
_lastEditableSession(false),
_isLoaded(false),
@@ -259,7 +260,8 @@ void DocumentBroker::pollThread()
}
// If all sessions have been removed, no reason to linger.
- if (_sessions.empty())
+ if (_sessions.empty() && std::chrono::duration_cast<std::chrono::milliseconds>
+ (std::chrono::steady_clock::now() - _lastSaveRequestTime).count() > COMMAND_TIMEOUT_MS)
{
LOG_INF("No more sessions in doc [" << _docKey << "]. Terminating.");
_stop = true;
@@ -716,6 +718,7 @@ bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
LOG_TRC(".uno:Save arguments: " << saveArgs);
const auto command = "uno .uno:Save " + saveArgs;
forwardToChild(savingSession->getId(), command);
+ _lastSaveRequestTime = std::chrono::steady_clock::now();
return true;
}
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index 744cf102..d3b87b8b 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -371,6 +371,9 @@ private:
/// document was modified and saved or not.
std::chrono::steady_clock::time_point _lastSaveTime;
+ /// The last time we sent a save request.
+ std::chrono::steady_clock::time_point _lastSaveRequestTime;
+
/// The document's last-modified time on storage.
Poco::Timestamp _documentLastModifiedTime;
commit 58d3b7f9ffdd388839f6b669c47ab00a64cfb632
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 21:33:59 2017 -0400
wsd: new test to validate recovery after kit crash
Change-Id: I406e5b72f6edf46dfde5da8c07daed21aef8b130
diff --git a/test/httpcrashtest.cpp b/test/httpcrashtest.cpp
index 9cb7d437..b6230d5a 100644
--- a/test/httpcrashtest.cpp
+++ b/test/httpcrashtest.cpp
@@ -57,12 +57,14 @@ class HTTPCrashTest : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testBarren);
CPPUNIT_TEST(testCrashKit);
+ CPPUNIT_TEST(testRecoverAfterKitCrash);
CPPUNIT_TEST(testCrashForkit);
CPPUNIT_TEST_SUITE_END();
void testBarren();
void testCrashKit();
+ void testRecoverAfterKitCrash();
void testCrashForkit();
static
@@ -172,6 +174,31 @@ void HTTPCrashTest::testCrashKit()
}
}
+void HTTPCrashTest::testRecoverAfterKitCrash()
+{
+ const auto testname = "recoverAfterKitCrash ";
+ try
+ {
+ auto socket = loadDocAndGetSocket("empty.odt", _uri, testname);
+
+ std::cerr << "Killing loolkit instances." << std::endl;
+
+ killLoKitProcesses("(loolkit)");
+ countLoolKitProcesses(0);
+
+ // We expect the client connection to close.
+ std::cerr << "Reconnect after kill." << std::endl;
+
+ auto socket2 = loadDocAndGetSocket("empty.odt", _uri, testname);
+ sendTextFrame(socket2, "status", testname);
+ assertResponseString(socket2, "status:", testname);
+ }
+ catch (const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText());
+ }
+}
+
void HTTPCrashTest::testCrashForkit()
{
const auto testname = "crashForkit ";
commit eab17f44966af3c47245ce4200e66795592c7d72
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 20:57:50 2017 -0400
wsd: assert response in test instead of failing with exception
Change-Id: I1090ee25ba87acbcbb1c66007fbf3363966952e4
diff --git a/test/httpwstest.cpp b/test/httpwstest.cpp
index 1e7c4f8b..ed5649ca 100644
--- a/test/httpwstest.cpp
+++ b/test/httpwstest.cpp
@@ -2335,7 +2335,7 @@ void HTTPWSTest::testAlertAllUsers()
for (int i = 0; i < 2; i++)
{
- std::string response = getResponseString(socket[i], "error:", testname);
+ const std::string response = assertResponseString(socket[i], "error:", testname);
Poco::StringTokenizer tokens(response.substr(6), " ", Poco::StringTokenizer::TOK_IGNORE_EMPTY | Poco::StringTokenizer::TOK_TRIM);
std::string cmd;
LOOLProtocol::getTokenString(tokens, "cmd", cmd);
commit c7b4d9ecab2af70d49f6505b9d215504606af5b0
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Mon Mar 20 20:42:09 2017 -0400
wsd: fix rendering options regression
The rendering options must be last in the load
command. This is because it could contain
characters that interfere with the parsing.
Change-Id: I14930d580d59014532d07410251acbd1316b4f61
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index de09bccb..f0e79f59 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -228,7 +228,12 @@ void DocumentBroker::pollThread()
assert(!_uriJailed.empty());
std::vector<std::string> tokens = LOOLProtocol::tokenize(message);
if (tokens.size() > 1 && tokens[1] == "load")
- message += std::string(" jail=") + _uriJailed.toString();
+ {
+ // The json options must come last.
+ message = tokens[0] + ' ' + tokens[1] + ' ' + tokens[2];
+ message += " jail=" + _uriJailed.toString() + ' ';
+ message += Poco::cat(std::string(" "), tokens.begin() + 3, tokens.end());
+ }
LOG_DBG("Sending a queued message: " + message);
_childProcess->sendTextFrame(message);
@@ -1171,8 +1176,15 @@ bool DocumentBroker::forwardToChild(const std::string& viewId, const std::string
if (it != _sessions.end())
{
assert(!_uriJailed.empty());
- if (LOOLProtocol::getFirstToken(message) == "load")
- msg += " jail=" + _uriJailed.toString();
+
+ std::vector<std::string> tokens = LOOLProtocol::tokenize(msg);
+ if (tokens.size() > 1 && tokens[1] == "load")
+ {
+ // The json options must come last.
+ msg = tokens[0] + ' ' + tokens[1] + ' ' + tokens[2];
+ msg += " jail=" + _uriJailed.toString() + ' ';
+ msg += Poco::cat(std::string(" "), tokens.begin() + 3, tokens.end());
+ }
_childProcess->sendTextFrame(msg);
return true;
More information about the Libreoffice-commits
mailing list