[Libreoffice-commits] online.git: 4 commits - loolwsd/Admin.cpp loolwsd/ChildProcessSession.cpp loolwsd/ChildProcessSession.hpp loolwsd/IoUtil.cpp loolwsd/LOOLKit.cpp loolwsd/LOOLSession.cpp loolwsd/LOOLSession.hpp loolwsd/MasterProcessSession.cpp loolwsd/MasterProcessSession.hpp loolwsd/PROBLEMS loolwsd/protocol.txt loolwsd/test
Tor Lillqvist
tml at collabora.com
Wed Apr 13 12:58:01 UTC 2016
loolwsd/Admin.cpp | 6 ----
loolwsd/ChildProcessSession.cpp | 9 +------
loolwsd/ChildProcessSession.hpp | 3 --
loolwsd/IoUtil.cpp | 6 ----
loolwsd/LOOLKit.cpp | 12 ---------
loolwsd/LOOLSession.cpp | 8 +-----
loolwsd/LOOLSession.hpp | 4 +--
loolwsd/MasterProcessSession.cpp | 19 +++++---------
loolwsd/MasterProcessSession.hpp | 4 +--
loolwsd/PROBLEMS | 16 ------------
loolwsd/protocol.txt | 11 --------
loolwsd/test/httpwstest.cpp | 50 +++++++++++++++++++++------------------
12 files changed, 43 insertions(+), 105 deletions(-)
New commits:
commit 4087ac808922dc1e19cb77a0e4b5b575ce8fcda7
Author: Tor Lillqvist <tml at collabora.com>
Date: Wed Apr 13 15:56:23 2016 +0300
With "disconnect" messages gone, no need for a 'reason' to disconnect()
YAGNI.
diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp
index aeb4219..a7d00ca 100644
--- a/loolwsd/ChildProcessSession.cpp
+++ b/loolwsd/ChildProcessSession.cpp
@@ -283,7 +283,7 @@ ChildProcessSession::~ChildProcessSession()
_callbackThread.join();
}
-void ChildProcessSession::disconnect(const std::string& reason)
+void ChildProcessSession::disconnect()
{
if (!isDisconnected())
{
@@ -295,15 +295,10 @@ void ChildProcessSession::disconnect(const std::string& reason)
//TODO: Handle saving to temp etc.
_onUnload(getId());
- LOOLSession::disconnect(reason);
+ LOOLSession::disconnect();
}
}
-bool ChildProcessSession::handleDisconnect(StringTokenizer& tokens)
-{
- return LOOLSession::handleDisconnect(tokens);
-}
-
bool ChildProcessSession::_handleInput(const char *buffer, int length)
{
if (isInactive() && _loKitDocument != nullptr)
diff --git a/loolwsd/ChildProcessSession.hpp b/loolwsd/ChildProcessSession.hpp
index 0f14842..bb55b68 100644
--- a/loolwsd/ChildProcessSession.hpp
+++ b/loolwsd/ChildProcessSession.hpp
@@ -51,8 +51,7 @@ public:
virtual bool getPartPageRectangles(const char *buffer, int length) override;
- virtual void disconnect(const std::string& reason = "") override;
- virtual bool handleDisconnect(Poco::StringTokenizer& tokens) override;
+ virtual void disconnect() override;
int getViewId() const { return _viewId; }
diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 2e32ee1..9dda2e6 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -179,7 +179,7 @@ void LOOLSession::parseDocOptions(const StringTokenizer& tokens, int& part, std:
}
}
-void LOOLSession::disconnect(const std::string&)
+void LOOLSession::disconnect()
{
try
{
@@ -195,7 +195,7 @@ void LOOLSession::disconnect(const std::string&)
}
}
-bool LOOLSession::handleDisconnect(StringTokenizer& /*tokens*/)
+bool LOOLSession::handleDisconnect()
{
_disconnected = true;
IoUtil::shutdownWebSocket(_ws);
diff --git a/loolwsd/LOOLSession.hpp b/loolwsd/LOOLSession.hpp
index 2fc5429..8a5b503 100644
--- a/loolwsd/LOOLSession.hpp
+++ b/loolwsd/LOOLSession.hpp
@@ -49,10 +49,10 @@ public:
bool handleInput(const char *buffer, int length);
/// Invoked when we want to disconnect a session.
- virtual void disconnect(const std::string& reason = "");
+ virtual void disconnect();
/// Called to handle disconnection command from socket.
- virtual bool handleDisconnect(Poco::StringTokenizer& tokens);
+ virtual bool handleDisconnect();
bool isInactive() const { return getInactivityMS() >= InactivityThresholdMS; }
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index bd96572..e67b332 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -59,11 +59,11 @@ MasterProcessSession::~MasterProcessSession()
}
}
-void MasterProcessSession::disconnect(const std::string& reason)
+void MasterProcessSession::disconnect()
{
if (!isDisconnected())
{
- LOOLSession::disconnect(reason);
+ LOOLSession::disconnect();
// Release the save-as queue.
_saveAsQueue.put("");
@@ -71,25 +71,20 @@ void MasterProcessSession::disconnect(const std::string& reason)
auto peer = _peer.lock();
if (peer)
{
- peer->disconnect(reason);
+ peer->disconnect();
}
}
}
-bool MasterProcessSession::handleDisconnect(Poco::StringTokenizer& tokens)
+bool MasterProcessSession::handleDisconnect()
{
- Log::info("Graceful disconnect on " + getName() + " [" +
- (tokens.count() > 1 ? tokens[1] : std::string("no reason")) +
- "].");
+ Log::info("Graceful disconnect on " + getName() + ".");
- LOOLSession::handleDisconnect(tokens);
+ LOOLSession::handleDisconnect();
auto peer = _peer.lock();
if (peer)
- {
- const auto reason = (tokens.count() > 1 ? Poco::cat(std::string(" "), tokens.begin() + 1, tokens.end()) : "");
- peer->disconnect(reason);
- }
+ peer->disconnect();
return false;
}
diff --git a/loolwsd/MasterProcessSession.hpp b/loolwsd/MasterProcessSession.hpp
index 04d671d..47b2d97 100644
--- a/loolwsd/MasterProcessSession.hpp
+++ b/loolwsd/MasterProcessSession.hpp
@@ -36,8 +36,8 @@ class MasterProcessSession final : public LOOLSession, public std::enable_shared
virtual bool getPartPageRectangles(const char *buffer, int length) override;
- virtual void disconnect(const std::string& reason = "") override;
- virtual bool handleDisconnect(Poco::StringTokenizer& tokens) override;
+ virtual void disconnect() override;
+ virtual bool handleDisconnect() override;
/**
* Return the URL of the saved-as document when it's ready. If called
commit c9796255ae0a79598f27f4b82d1694480a39579a
Author: Tor Lillqvist <tml at collabora.com>
Date: Wed Apr 13 15:50:09 2016 +0300
Don't check for undocumented "eof" frames read from a websocket either
Now with "disconnect" frames gone, surely checks for "eof" ones can go
away too, whatever they were.
diff --git a/loolwsd/Admin.cpp b/loolwsd/Admin.cpp
index 4a3ded3..899e902 100644
--- a/loolwsd/Admin.cpp
+++ b/loolwsd/Admin.cpp
@@ -110,12 +110,6 @@ void AdminRequestHandler::handleWSRequests(HTTPServerRequest& request, HTTPServe
StringTokenizer tokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
Log::trace("Recv: " + firstLine);
- if (firstLine == "eof")
- {
- Log::info("Received EOF. Finishing.");
- break;
- }
-
if (tokens.count() < 1)
continue;
diff --git a/loolwsd/IoUtil.cpp b/loolwsd/IoUtil.cpp
index c002230..081e13e 100644
--- a/loolwsd/IoUtil.cpp
+++ b/loolwsd/IoUtil.cpp
@@ -144,12 +144,6 @@ void SocketProcessor(std::shared_ptr<WebSocket> ws,
break;
}
- if (firstLine == "eof")
- {
- Log::info("Received EOF. Finishing.");
- break;
- }
-
// Call the handler.
const auto success = handler(payload);
payload.resize(0);
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 2f34568..ce32008 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -301,12 +301,6 @@ public:
if (n > 0 && (flags & WebSocket::FRAME_OP_BITMASK) != WebSocket::FRAME_OP_CLOSE)
{
std::string firstLine = getFirstLine(buffer, n);
- if (firstLine == "eof")
- {
- Log::info("Received EOF. Finishing.");
- break;
- }
-
StringTokenizer tokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
// Check if it is a "nextmessage:" and in that case read the large
commit e5de11113b624895cec77f2598fb436b3006c660
Author: Tor Lillqvist <tml at collabora.com>
Date: Wed Apr 13 15:39:14 2016 +0300
Don't check for or send "disconnect" frames anywhere and don't document them
Follow-up to 68b3a2c81e92de7be3d1b6bd503ff78b5c924422.
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 2b16af4..2f34568 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -309,12 +309,6 @@ public:
StringTokenizer tokens(firstLine, " ", StringTokenizer::TOK_IGNORE_EMPTY | StringTokenizer::TOK_TRIM);
- if (firstLine == "disconnect")
- {
- Log::info("Client disconnected [" + (tokens.count() == 2 ? tokens[1] : std::string("no reason")) + "].");
- break;
- }
-
// Check if it is a "nextmessage:" and in that case read the large
// follow-up message separately, and handle that only.
int size;
diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 1b744f4..2e32ee1 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -179,16 +179,12 @@ void LOOLSession::parseDocOptions(const StringTokenizer& tokens, int& part, std:
}
}
-void LOOLSession::disconnect(const std::string& reason)
+void LOOLSession::disconnect(const std::string&)
{
try
{
if (!_disconnected)
{
- if (reason != "")
- sendTextFrame("disconnect " + reason);
- else
- sendTextFrame("disconnect");
_disconnected = true;
IoUtil::shutdownWebSocket(_ws);
}
diff --git a/loolwsd/PROBLEMS b/loolwsd/PROBLEMS
index 89ed2c7..9b8507a 100644
--- a/loolwsd/PROBLEMS
+++ b/loolwsd/PROBLEMS
@@ -7,22 +7,6 @@
one has no idea how it works. That was hopefully not the case when
recursive mutexes were introduced here? But probably it is by now...
-- The use of separate "disconnect" messages over the WebSocket
- connections is not good. It should be perfectly enough to just close
- the WebSocket connection gracefully using the WebSocket mechanism,
- i.e. a frame with the CLOSE opcode. Or just tearing down the socket
- without a CLOSE frame. The code needs to be prepared to handle these
- situations anyway, especially for the socket talking to the
- client. For the internal communication in the process tree,
- "disconnect" messages are barely acceptable, if the code is already
- written to generate and expect them.
-
- For the moment, we send them from the client too - to distinguish the
- browser crash or connection loss from a deliberate shutdown.
- We are saving the document in the case we don't get the explicit
- 'disconnect'. When we move to actually saving the document all
- the time automatically, the 'disconnect' message should be removed.
-
- Occasionally Control-C (SIGINT) doesn't shut fown loolwsd. One has
to kill it with SIGKILL. Which of course leaves all the chroot jails
around.
diff --git a/loolwsd/protocol.txt b/loolwsd/protocol.txt
index c2c0acb..a7e36c6 100644
--- a/loolwsd/protocol.txt
+++ b/loolwsd/protocol.txt
@@ -17,12 +17,6 @@ canceltiles
dropped and will not be handled. There is no guarantee of exactly
which tile: messages might still be sent back to the client.
-disconnect
-
- The socket is going to be shut down normally. This is to distinguish
- deliberate shutdown from the case when the connection was lost, or
- the browser was killed etc.
-
downloadas name=<fileName> id=<id> format=<document format> options=<SkipImages, etc>
Exports the current document to the desired format and returns a download URL
@@ -279,11 +273,6 @@ curpart: part=<partNumber>
needs to act on in addition to passing them on to the client, like
invalidatetiles:
-disconnect [reason]
-
- Sent before disconnecting gracefully.
- reason: optional human-readable reason to disconnect.
-
nextmessage: size=<upperlimit>
each tile: message sent from the child to the parent is preceded
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 1db58ef..cf015b5 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -216,7 +216,6 @@ void HTTPWSTest::testLoad()
}
while (n > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
- sendTextFrame(socket, "disconnect");
socket.shutdown();
Util::removeFile(documentPath);
}
@@ -268,7 +267,6 @@ void HTTPWSTest::testBadLoad()
}
while (n > 0 && (flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) != Poco::Net::WebSocket::FRAME_OP_CLOSE);
- sendTextFrame(socket, "disconnect");
socket.shutdown();
Util::removeFile(documentPath);
}
commit db14143d72cd4c2c9284edbf0b2ae853dd57b15d
Author: Tor Lillqvist <tml at collabora.com>
Date: Wed Apr 13 15:33:56 2016 +0300
Make the loolkit process counting more reliable
Sleep in both places before counting them, and catch Poco exceptions
while counting (can happen for instance when a /proc entry goes away
while we are looking for its 'comm' file).
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index e064ab2..1db58ef 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -819,9 +819,6 @@ void HTTPWSTest::testImpressPartCountChanged()
void HTTPWSTest::testNoExtraLoolKitsLeft()
{
- // Give polls in the lool processes time to time out
- Poco::Thread::sleep(POLL_TIMEOUT_MS*2);
-
int countNow = countLoolKitProcesses();
CPPUNIT_ASSERT_EQUAL(countNow, _initialLoolKitCount);
@@ -933,32 +930,41 @@ void HTTPWSTest::getResponseMessage(Poco::Net::WebSocket& ws, const std::string&
int HTTPWSTest::countLoolKitProcesses()
{
+ // Give polls in the lool processes time to time out
+ Poco::Thread::sleep(POLL_TIMEOUT_MS*3);
+
int result = 0;
for (auto i = Poco::DirectoryIterator(std::string("/proc")); i != Poco::DirectoryIterator(); ++i)
{
- Poco::Path procEntry = i.path();
- const std::string& fileName = procEntry.getFileName();
- int pid;
- std::size_t endPos = 0;
try
{
- pid = std::stoi(fileName, &endPos);
- }
- catch (const std::invalid_argument&)
- {
- pid = 0;
+ Poco::Path procEntry = i.path();
+ const std::string& fileName = procEntry.getFileName();
+ int pid;
+ std::size_t endPos = 0;
+ try
+ {
+ pid = std::stoi(fileName, &endPos);
+ }
+ catch (const std::invalid_argument&)
+ {
+ pid = 0;
+ }
+ if (pid > 1 && endPos == fileName.length())
+ {
+ Poco::FileInputStream comm(procEntry.toString() + "/comm");
+ std::string command;
+ Poco::StreamCopier::copyToString(comm, command);
+ if (command.length() > 0 && command.back() == '\n')
+ command.pop_back();
+ // std::cout << "For process " << pid << " comm is '" << command << "'" << std::endl;
+ if (command == "loolkit")
+ result++;
+ }
}
- if (pid > 1 && endPos == fileName.length())
+ catch (const Poco::Exception&)
{
- Poco::FileInputStream comm(procEntry.toString() + "/comm");
- std::string command;
- Poco::StreamCopier::copyToString(comm, command);
- if (command.length() > 0 && command.back() == '\n')
- command.pop_back();
- // std::cout << "For process " << pid << " comm is '" << command << "'" << std::endl;
- if (command == "loolkit")
- result++;
}
}
More information about the Libreoffice-commits
mailing list