loolwsd/ChildProcessSession.cpp | 7 +++-
loolwsd/IoUtil.cpp | 4 ++
loolwsd/IoUtil.hpp | 1
loolwsd/LOOLKit.cpp | 21 +++++++++++--
loolwsd/LOOLSession.cpp | 4 --
loolwsd/LOOLSession.hpp | 7 ++++
loolwsd/LOOLWSD.cpp | 53 ++++++++++++++++++++++++++++++++-
loolwsd/MasterProcessSession.cpp | 61 +++++++++++----------------------------
loolwsd/MasterProcessSession.hpp | 5 +--
loolwsd/test/httpwstest.cpp | 47 ++++++++++++++++++++++++++++++
10 files changed, 155 insertions(+), 55 deletions(-)
New commits:
commit c29944a386badbd7093d81ed2842e73b59f40cce
Author: Henry Castro <hcas...@collabora.com>
Date: Mon Apr 18 19:12:26 2016 -0400
loolwsd: fix close after close
The closing handshake.
Either peer can send a control frame with data containing
a specified control sequence to begin the closing handshake.
Upon receiving such a frame, the other peer sends a
Close frame in response, if it hasn't already sent one.
diff --git a/loolwsd/ChildProcessSession.cpp b/loolwsd/ChildProcessSession.cpp
index ac8b8a0..18bdfb6 100644
--- a/loolwsd/ChildProcessSession.cpp
+++ b/loolwsd/ChildProcessSession.cpp
@@ -78,7 +78,12 @@ public:
Log::trace() << "CallbackWorker::callback [" << _session.getViewId()
<< "] "
<< LOKitHelper::kitCallbackTypeToString(nType)
<< " [" << rPayload << "]." << Log::end;
- if (_session.isDisconnected())
+ if (_session.isCloseFrame())
+ {
+ Log::trace("LOKit document begin the closing handshake");
+ return;
+ }
+ else if (_session.isDisconnected())
{
Log::trace("Skipping callback on disconnected session " +
_session.getName());
return;
diff --git a/loolwsd/IoUtil.cpp b/loolwsd/IoUtil.cpp
index 93a61ea..92b0254 100644
--- a/loolwsd/IoUtil.cpp
+++ b/loolwsd/IoUtil.cpp
@@ -41,6 +41,7 @@ namespace IoUtil
// Handler returns false to end.
void SocketProcessor(std::shared_ptr<WebSocket> ws,
std::function<bool(const std::vector<char>&)> handler,
+ std::function<void()> closeFrame,
std::function<bool()> stopPredicate)
{
Log::info("SocketProcessor starting.");
@@ -93,6 +94,7 @@ void SocketProcessor(std::shared_ptr<WebSocket> ws,
}
else if (n <= 0 || ((flags & WebSocket::FRAME_OP_BITMASK) ==
WebSocket::FRAME_OP_CLOSE))
{
+ closeFrame();
Log::warn("Connection closed.");
break;
}
@@ -109,6 +111,7 @@ void SocketProcessor(std::shared_ptr<WebSocket> ws,
n = ws->receiveFrame(buffer, sizeof(buffer), flags);
if (n <= 0 || (flags & WebSocket::FRAME_OP_BITMASK) ==
WebSocket::FRAME_OP_CLOSE)
{
+ closeFrame();
Log::warn("Connection closed while reading multiframe
message.");
break;
}
@@ -138,6 +141,7 @@ void SocketProcessor(std::shared_ptr<WebSocket> ws,
if (n <= 0 || (flags & WebSocket::FRAME_OP_BITMASK) ==
WebSocket::FRAME_OP_CLOSE)
{
+ closeFrame();
Log::warn("Connection closed.");
break;
}
diff --git a/loolwsd/IoUtil.hpp b/loolwsd/IoUtil.hpp
index 476f515..2b49658 100644
--- a/loolwsd/IoUtil.hpp
+++ b/loolwsd/IoUtil.hpp
@@ -25,6 +25,7 @@ namespace IoUtil
//. Handler returns false to end.
void SocketProcessor(std::shared_ptr<Poco::Net::WebSocket> ws,
std::function<bool(const std::vector<char>&)> handler,
+ std::function<void()> closeFrame,
std::function<bool()> stopPredicate);
/// Call WebSocket::shutdown() ignoring Poco::IOException.
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 28e1d68..7a95bcb 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -51,6 +51,7 @@
#include "LOOLProtocol.hpp"
#include "QueueHandler.hpp"
#include "Unit.hpp"
+#include "UserMessages.hpp"
#include "Util.hpp"
#define LIB_SOFFICEAPP "lib" "sofficeapp" ".so"
@@ -280,18 +281,31 @@ public:
Thread queueHandlerThread;
queueHandlerThread.start(handler);
+ std::shared_ptr<ChildProcessSession> session = _session;
IoUtil::SocketProcessor(_ws,
- [&queue](const std::vector<char>& payload)
+ [&queue](const std::vector<char>& payload)
{
queue->put(payload);
return true;
},
- []() { return TerminationFlag; });
+ [&session]() { session->closeFrame(); },
+ [&queueHandlerThread]() { return TerminationFlag &&
queueHandlerThread.isRunning(); });
queue->clear();
queue->put("eof");
queueHandlerThread.join();
+
+ if (session->isCloseFrame())
+ {
+ Log::trace("Normal close handshake.");
+ _ws->shutdown();
+ }
+ else
+ {
+ Log::trace("Abnormal close handshake.");
+ _ws->shutdown(WebSocket::WS_ENDPOINT_GOING_AWAY,
SERVICE_UNAVALABLE_INTERNAL_ERROR);
+ }
}
catch (const Exception& exc)
{
@@ -1047,7 +1061,8 @@ void lokit_main(const std::string& childRoot,
return true;
},
- [&document]()
+ []() {},
+ [&document]()
{
if (document && document->canDiscard())
TerminationFlag = true;
diff --git a/loolwsd/LOOLSession.cpp b/loolwsd/LOOLSession.cpp
index 9dda2e6..090f2ed 100644
--- a/loolwsd/LOOLSession.cpp
+++ b/loolwsd/LOOLSession.cpp
@@ -56,6 +56,7 @@ LOOLSession::LOOLSession(const std::string& id, const Kind
kind,
_isDocPasswordProvided(false),
_isDocLoaded(false),
_isDocPasswordProtected(false),
+ _isCloseFrame(false),
_disconnected(false),
_lastActivityTime(std::chrono::steady_clock::now())
{
@@ -68,7 +69,6 @@ LOOLSession::LOOLSession(const std::string& id, const Kind
kind,
LOOLSession::~LOOLSession()
{
- IoUtil::shutdownWebSocket(_ws);
}
void LOOLSession::sendTextFrame(const std::string& text)
@@ -99,7 +99,6 @@ void LOOLSession::sendTextFrame(const std::string& text)
Log::warn() << "LOOLSession::sendTextFrame: "
<< "Exception: " << exc.displayText()
<< (exc.nested() ? "( " + exc.nested()->displayText() +
")" : "");
- IoUtil::shutdownWebSocket(_ws);
}
}
@@ -130,7 +129,6 @@ void LOOLSession::sendBinaryFrame(const char *buffer, int
length)
Log::warn() << "LOOLSession::sendBinaryFrame: "
<< "Exception: " << exc.displayText()
<< (exc.nested() ? "( " + exc.nested()->displayText() +
")" : "");
- IoUtil::shutdownWebSocket(_ws);
}
}
diff --git a/loolwsd/LOOLSession.hpp b/loolwsd/LOOLSession.hpp
index 8a5b503..617b159 100644
--- a/loolwsd/LOOLSession.hpp
+++ b/loolwsd/LOOLSession.hpp
@@ -10,6 +10,7 @@
#ifndef INCLUDED_LOOLSESSION_HPP
#define INCLUDED_LOOLSESSION_HPP
+#include <atomic>
#include <cassert>
#include <memory>
#include <mutex>
@@ -63,6 +64,9 @@ public:
return
std::chrono::duration_cast<std::chrono::milliseconds>(duration).count();
}
+ void closeFrame() { _isCloseFrame = true; };
+ bool isCloseFrame() const { return _isCloseFrame; }
+
protected:
LOOLSession(const std::string& id, const Kind kind,
std::shared_ptr<Poco::Net::WebSocket> ws);
@@ -125,6 +129,9 @@ protected:
/// Document options: a JSON string, containing options (rendering, also
possibly load in the future).
std::string _docOptions;
+ // Whether websocket received close frame. Closing Handshake
+ std::atomic<bool> _isCloseFrame;
+
private:
virtual bool _handleInput(const char *buffer, int length) = 0;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 0d9c516..5248b38 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -577,7 +577,8 @@ private:
queue->put(payload);
return true;
},
- []() { return TerminationFlag; });
+ [&session]() { session->closeFrame(); },
+ [&queueHandlerThread]() { return TerminationFlag &&
queueHandlerThread.isRunning(); });
docBrokersLock.lock();
const bool canDestroy = docBroker->canDestroy();
@@ -618,6 +619,26 @@ private:
Log::info("Removing complete doc [" + docKey + "] from Admin.");
Admin::instance().rmDoc(docKey);
}
+ docBrokersLock.unlock();
+
+ if (session->isCloseFrame())
+ {
+ Log::trace("Normal close handshake.");
+ if (session->shutdownPeer(WebSocket::WS_NORMAL_CLOSE, ""))
+ {
+ // Client initiated close handshake
+ // respond close frame
+ ws->shutdown();
+ }
+ }
+ else
+ {
+ // something wrong, with internal exceptions
+ Log::trace("Abnormal close handshake.");
+ session->closeFrame();
+ ws->shutdown(WebSocket::WS_ENDPOINT_GOING_AWAY,
SERVICE_UNAVALABLE_INTERNAL_ERROR);
+ session->shutdownPeer(WebSocket::WS_ENDPOINT_GOING_AWAY,
SERVICE_UNAVALABLE_INTERNAL_ERROR);
+ }
}
/// Sends back the WOPI Discovery XML.
@@ -742,6 +763,10 @@ public:
Log::error(std::string("ClientRequestHandler::handleRequest:
Exception: ") + exc.what());
response.setStatusAndReason(HTTPResponse::HTTP_SERVICE_UNAVAILABLE);
}
+ catch (...)
+ {
+ Log::error("ClientRequestHandler::handleRequest:: Unexpected
exception");
+ }
if (!responded)
{
@@ -925,11 +950,31 @@ public:
UnitWSD::get().onChildConnected(pid, sessionId);
IoUtil::SocketProcessor(ws,
- [&session](const std::vector<char>& payload)
+ [&session](const std::vector<char>& payload)
{
return session->handleInput(payload.data(),
payload.size());
},
+ [&session]() { session->closeFrame(); },
[]() { return TerminationFlag; });
+
+ if (session->isCloseFrame())
+ {
+ Log::trace("Normal close handshake.");
+ if (session->shutdownPeer(WebSocket::WS_NORMAL_CLOSE, ""))
+ {
+ // LOKit initiated close handshake
+ // respond close frame
+ ws->shutdown();
+ }
+ }
+ else
+ {
+ // something wrong, with internal exceptions
+ Log::trace("Abnormal close handshake.");
+ session->closeFrame();
+ ws->shutdown(WebSocket::WS_ENDPOINT_GOING_AWAY,
SERVICE_UNAVALABLE_INTERNAL_ERROR);
+ session->shutdownPeer(WebSocket::WS_ENDPOINT_GOING_AWAY,
SERVICE_UNAVALABLE_INTERNAL_ERROR);
+ }
}
catch (const Exception& exc)
{
@@ -941,6 +986,10 @@ public:
{
Log::error(std::string("PrisonerRequestHandler::handleRequest:
Exception: ") + exc.what());
}
+ catch (...)
+ {
+ Log::error("PrisonerRequestHandler::handleRequest:: Unexpected
exception");
+ }
if (!jailId.empty())
{
diff --git a/loolwsd/MasterProcessSession.cpp b/loolwsd/MasterProcessSession.cpp
index 5c053c7..c2d4856 100644
--- a/loolwsd/MasterProcessSession.cpp
+++ b/loolwsd/MasterProcessSession.cpp
@@ -47,46 +47,8 @@ MasterProcessSession::~MasterProcessSession()
{
Log::info("~MasterProcessSession dtor [" + getName() + "].");
- try
- {
- // We could be unwinding because our peer's connection
- // died. Handle I/O errors in that case.
- disconnect();
- }
- catch (const std::exception& exc)
- {
- Log::error(std::string("MasterProcessSession::~MasterProcessSession:
Exception: ") + exc.what());
- }
-}
-
-void MasterProcessSession::disconnect()
-{
- if (!isDisconnected())
- {
- LOOLSession::disconnect();
-
- // Release the save-as queue.
- _saveAsQueue.put("");
-
- auto peer = _peer.lock();
- if (peer)
- {
- peer->disconnect();
- }
- }
-}
-
-bool MasterProcessSession::handleDisconnect()
-{
- Log::info("Graceful disconnect on " + getName() + ".");
-
- LOOLSession::handleDisconnect();
-
- auto peer = _peer.lock();
- if (peer)
- peer->disconnect();
-
- return false;
+ // Release the save-as queue.
+ _saveAsQueue.put("");
}
bool MasterProcessSession::_handleInput(const char *buffer, int length)
@@ -126,8 +88,7 @@ bool MasterProcessSession::_handleInput(const char *buffer,
int length)
{
if (!peer)
{
- LOOLSession::disconnect();
- return false;
+ throw Poco::ProtocolException("The session has not been
assigned a peer.");
}
if (tokens[0] == "unocommandresult:")
@@ -768,11 +729,25 @@ void MasterProcessSession::forwardToPeer(const char
*buffer, int length)
auto peer = _peer.lock();
if (!peer)
{
- Log::error(getName() + ": no peer to forward to.");
+ throw Poco::ProtocolException(getName() + ": no peer to forward to.");
+ }
+ else if (peer->isCloseFrame())
+ {
+ Log::trace(getName() + ": peer begin the closing handshake");
return;
}
peer->sendBinaryFrame(buffer, length);
}
+bool MasterProcessSession::shutdownPeer(Poco::UInt16 statusCode, const
std::string& message)
+{
+ auto peer = _peer.lock();
+ if (peer && !peer->isCloseFrame())
+ {
+ peer->_ws->shutdown(statusCode, message);
+ }
+ return peer != nullptr;
+}
+
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/MasterProcessSession.hpp b/loolwsd/MasterProcessSession.hpp
index 47b2d97..c38b20f 100644
--- a/loolwsd/MasterProcessSession.hpp
+++ b/loolwsd/MasterProcessSession.hpp
@@ -36,9 +36,6 @@ class MasterProcessSession final : public LOOLSession, public
std::enable_shared
virtual bool getPartPageRectangles(const char *buffer, int length)
override;
- virtual void disconnect() override;
- virtual bool handleDisconnect() override;
-
/**
* Return the URL of the saved-as document when it's ready. If called
* before it's ready, the call blocks till then.
@@ -55,6 +52,8 @@ class MasterProcessSession final : public LOOLSession, public
std::enable_shared
bool isEditLocked() const { return _bEditLock; }
+ bool shutdownPeer(Poco::UInt16 statusCode, const std::string& message);
+
public:
// Raise this flag on ToClient from ToPrisoner to let ToClient know of
load failures
bool _bLoadError = false;
diff --git a/loolwsd/test/httpwstest.cpp b/loolwsd/test/httpwstest.cpp
index 81d6a16..59dfda4 100644
--- a/loolwsd/test/httpwstest.cpp
+++ b/loolwsd/test/httpwstest.cpp
@@ -53,6 +53,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testBadRequest);
CPPUNIT_TEST(testHandShake);
+ CPPUNIT_TEST(testCloseAfterClose);
CPPUNIT_TEST(testLoad);
CPPUNIT_TEST(testBadLoad);
CPPUNIT_TEST(testReload);
@@ -76,6 +77,7 @@ class HTTPWSTest : public CPPUNIT_NS::TestFixture
void testCountHowManyLoolkits();
void testBadRequest();
void testHandShake();
+ void testCloseAfterClose();
void testLoad();
void testBadLoad();
void testReload();
@@ -272,6 +274,51 @@ void HTTPWSTest::testHandShake()
}
}
+void HTTPWSTest::testCloseAfterClose()
+{
+ try
+ {
+ int bytes;
+ int flags;
+ char buffer[READ_BUFFER_SIZE];
+
+ // Load a document and get its status.
+ const std::string documentPath = Util::getTempFilePath(TDOC,
"hello.odt");
+ const std::string documentURL = "file://" +
Poco::Path(documentPath).makeAbsolute().toString();
+
+ Poco::Net::HTTPRequest request(Poco::Net::HTTPRequest::HTTP_GET,
documentURL);
+ Poco::Net::WebSocket socket = *connectLOKit(request, _response);
+
+ sendTextFrame(socket, "load url=" + documentURL);
+ sendTextFrame(socket, "status");
+ CPPUNIT_ASSERT_MESSAGE("cannot load the document " + documentURL,
isDocumentLoaded(socket));
+
+ // send normal socket shutdown
+ socket.shutdown();
+
+ // 5 seconds timeout
+ socket.setReceiveTimeout(5000000);
+
+ // receive close frame handshake
+ do
+ {
+ bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
+ }
+ while ((flags & Poco::Net::WebSocket::FRAME_OP_BITMASK) !=
Poco::Net::WebSocket::FRAME_OP_CLOSE);
+
+ // no more messages is received.
+ bytes = socket.receiveFrame(buffer, sizeof(buffer), flags);
+ std::string received(buffer);
+ std::cout << received << "received " << bytes << " flags "<< flags <<
std::endl;
+ CPPUNIT_ASSERT_EQUAL(0, bytes);
+ CPPUNIT_ASSERT_EQUAL(0, flags);
+ }
+ catch (const Poco::Exception& exc)
+ {
+ CPPUNIT_FAIL(exc.displayText());
+ }
+}
+
void HTTPWSTest::loadDoc(const std::string& documentURL)
{
try
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
