[Libreoffice-commits] online.git: 7 commits - common/Session.cpp common/Session.hpp net/Socket.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
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Thu Mar 30 06:22:00 UTC 2017
common/Session.cpp | 6 --
common/Session.hpp | 11 ++++
net/Socket.hpp | 5 ++
net/WebSocketHandler.hpp | 6 +-
wsd/Admin.cpp | 8 +--
wsd/AdminModel.cpp | 2
wsd/ClientSession.cpp | 18 +++----
wsd/ClientSession.hpp | 7 +-
wsd/DocumentBroker.cpp | 51 ++++++++++++---------
wsd/DocumentBroker.hpp | 19 ++++---
wsd/LOOLWSD.cpp | 112 ++++++++++++++++++++---------------------------
11 files changed, 126 insertions(+), 119 deletions(-)
New commits:
commit 7d3f5c0d2b64bd6562f168a3784842e3cc097e63
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 22:50:29 2017 -0400
wsd: terminate DocBrokers asynchronously
Remove locks and replace with isCorrectThread
assertions instead.
Crash recovery still needs some work, but
otherwise tests are clean (91/94 pass).
Change-Id: I9ac3e21854447d19a8e6106487dfd8be00fcf5ef
diff --git a/wsd/ClientSession.cpp b/wsd/ClientSession.cpp
index 10a11343..9178dff7 100644
--- a/wsd/ClientSession.cpp
+++ b/wsd/ClientSession.cpp
@@ -611,8 +611,7 @@ bool ClientSession::handleKitToClientMessage(const char* buffer, const int lengt
docBroker->removeSession(getId());
// Now terminate.
- auto lock = docBroker->getLock();
- docBroker->terminateChild(lock, "", true);
+ docBroker->stop();
}
return true;
@@ -731,6 +730,7 @@ void ClientSession::onDisconnect()
const auto docBroker = getDocumentBroker();
LOG_CHECK_RET(docBroker && "Null DocumentBroker instance", );
+ assert(docBroker->isCorrectThread());
const auto docKey = docBroker->getDocKey();
try
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index c92c1d8f..6d87fb7b 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -259,6 +259,10 @@ void DocumentBroker::pollThread()
}
}
+ // Terminate properly while we can.
+ //TODO: pass some sensible reason.
+ terminateChild("", false);
+
// Flush socket data.
const int flushTimeoutMs = POLL_TIMEOUT_MS * 2; // ~1000ms
const auto flushStartTime = std::chrono::steady_clock::now();
@@ -272,9 +276,6 @@ void DocumentBroker::pollThread()
_poll->poll(std::min(flushTimeoutMs - elapsedMs, POLL_TIMEOUT_MS / 5));
}
- // Terminate properly while we can.
- auto lock = getLock();
- terminateChild(lock, "", false);
LOG_INF("Finished docBroker polling thread for docKey [" << _docKey << "].");
}
@@ -501,6 +502,8 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
bool DocumentBroker::saveToStorage(const std::string& sessionId,
bool success, const std::string& result)
{
+ assert(isCorrectThread());
+
const bool res = saveToStorageInternal(sessionId, success, result);
// If marked to destroy, then this was the last session.
@@ -537,8 +540,6 @@ 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())
{
@@ -805,7 +806,7 @@ size_t DocumentBroker::addSession(const std::shared_ptr<ClientSession>& session)
size_t DocumentBroker::removeSession(const std::string& id, bool destroyIfLast)
{
- auto guard = getLock();
+ assert(isCorrectThread());
if (destroyIfLast)
destroyIfLastEditor(id);
@@ -828,6 +829,7 @@ size_t DocumentBroker::removeSession(const std::string& id, bool destroyIfLast)
size_t DocumentBroker::removeSessionInternal(const std::string& id)
{
+ assert(isCorrectThread());
try
{
Admin::instance().rmDoc(_docKey, id);
@@ -1139,7 +1141,7 @@ void DocumentBroker::handleTileCombinedResponse(const std::vector<char>& payload
void DocumentBroker::destroyIfLastEditor(const std::string& id)
{
- Util::assertIsLocked(_mutex);
+ assert(isCorrectThread());
const auto currentSession = _sessions.find(id);
if (currentSession == _sessions.end())
@@ -1275,10 +1277,9 @@ void DocumentBroker::childSocketTerminated()
}
}
-void DocumentBroker::terminateChild(std::unique_lock<std::mutex>& lock, const std::string& closeReason, const bool rude)
+void DocumentBroker::terminateChild(const std::string& closeReason, const bool rude)
{
- Util::assertIsLocked(_mutex);
- Util::assertIsLocked(lock);
+ assert(isCorrectThread());
LOG_INF("Terminating doc [" << _docKey << "].");
@@ -1309,26 +1310,20 @@ void DocumentBroker::terminateChild(std::unique_lock<std::mutex>& lock, const st
_childProcess->stop();
}
- // Release the lock and wait for the thread to finish.
- lock.unlock();
-
_childProcess->close(rude);
}
// Stop the polling thread.
_poll->stop();
_stop = true;
-
- // Trigger cleanup.
- LOOLWSD::triggerChildAndDocHousekeeping();
}
void DocumentBroker::closeDocument(const std::string& reason)
{
- auto lock = getLock();
+ assert(isCorrectThread());
LOG_DBG("Closing DocumentBroker for docKey [" << _docKey << "] with reason: " << reason);
- terminateChild(lock, reason, true);
+ terminateChild(reason, true);
}
void DocumentBroker::updateLastActivityTime()
diff --git a/wsd/DocumentBroker.hpp b/wsd/DocumentBroker.hpp
index ceb3db48..a7473531 100644
--- a/wsd/DocumentBroker.hpp
+++ b/wsd/DocumentBroker.hpp
@@ -224,6 +224,10 @@ public:
/// Start processing events
void startThread();
+ /// Flag for termination.
+ //TODO: Take reason to broadcast to clients.
+ void stop() { _stop = true; }
+
/// 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; }
@@ -319,13 +323,6 @@ public:
/// or upon failing to process an incoming message.
void childSocketTerminated();
- /// This gracefully terminates the connection
- /// with the child and cleans up ChildProcess etc.
- /// We must be called under lock and it must be
- /// passed to us so we unlock before waiting on
- /// the ChildProcess thread, which can take our lock.
- void terminateChild(std::unique_lock<std::mutex>& lock, const std::string& closeReason, const bool rude);
-
/// Get the PID of the associated child process
Poco::Process::PID getPid() const { return _childProcess->getPid(); }
@@ -341,6 +338,10 @@ public:
}
private:
+ /// This gracefully terminates the connection
+ /// with the child and cleans up ChildProcess etc.
+ void terminateChild(const std::string& closeReason, const bool rude);
+
/// Sends the .uno:Save command to LoKit.
bool sendUnoSave(const bool dontSaveIfUnmodified);
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index ffd7c612..b5491e51 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -248,7 +248,7 @@ bool cleanupDocBrokers()
{
LOG_INF("Terminating " << (idle ? "idle" : "dead") <<
" DocumentBroker for docKey [" << it->first << "].");
- docBroker->terminateChild(lock, idle ? "idle" : "", true);
+ docBroker->stop();
// Remove only when not alive.
if (!docBroker->isAlive())
@@ -1351,8 +1351,7 @@ public:
if (docBroker)
{
// FIXME: No need to notify if asked to stop.
- auto lock = docBroker->getLock();
- docBroker->terminateChild(lock, "Service unavailable", true);
+ docBroker->stop();
}
}
@@ -1379,7 +1378,7 @@ private:
{
auto lock = docBroker->getLock();
assert(docBroker->isCorrectThread());
- docBroker->terminateChild(lock, "Service unavailable", false);
+ docBroker->stop();
}
}
commit 00f823265221e243ff906089d92d3804b30ea3f7
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Wed Mar 29 23:11:34 2017 -0400
wsd: simplify getNewChild
Change-Id: Id939025fabdcd81ee3ab5ba3ae6c4ed446944a2a
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a0cf1067..c92c1d8f 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -185,7 +185,17 @@ void DocumentBroker::pollThread()
_threadStart = std::chrono::steady_clock::now();
// Request a kit process for this doc.
- _childProcess = getNewChild_Blocks();
+ do
+ {
+ static const int timeoutMs = COMMAND_TIMEOUT_MS * 5;
+ _childProcess = getNewChild_Blocks();
+ if (_childProcess ||
+ std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() -
+ _threadStart).count() > timeoutMs)
+ break;
+ }
+ while (!_stop && _poll->continuePolling() && !TerminationFlag && !ShutdownRequestFlag);
+
if (!_childProcess)
{
// Let the client know we can't serve now.
@@ -200,6 +210,8 @@ void DocumentBroker::pollThread()
#endif
// FIXME: return something good down the websocket ...
_stop = true;
+
+ LOG_INF("Finished docBroker polling thread for docKey [" << _docKey << "].");
return;
}
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index 455e03e5..ffd7c612 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -403,69 +403,54 @@ static size_t addNewChild(const std::shared_ptr<ChildProcess>& child)
std::shared_ptr<ChildProcess> getNewChild_Blocks()
{
- std::unique_lock<std::mutex> locka(DocBrokersMutex);
- std::unique_lock<std::mutex> lockb(NewChildrenMutex);
+ std::unique_lock<std::mutex> lock(NewChildrenMutex);
+
+ const auto startTime = std::chrono::steady_clock::now();
- namespace chrono = std::chrono;
- const auto startTime = chrono::steady_clock::now();
- do
+ LOG_DBG("getNewChild: Rebalancing children.");
+ int numPreSpawn = LOOLWSD::NumPreSpawnedChildren;
+ ++numPreSpawn; // Replace the one we'll dispatch just now.
+ if (rebalanceChildren(numPreSpawn) < 0)
{
- LOG_DBG("getNewChild: Rebalancing children.");
- int numPreSpawn = LOOLWSD::NumPreSpawnedChildren;
- ++numPreSpawn; // Replace the one we'll dispatch just now.
- if (rebalanceChildren(numPreSpawn) < 0)
- {
- // Fatal. Let's fail and retry at a higher level.
- LOG_DBG("getNewChild: rebalancing of children failed. Checking and restoring forkit.");
-
- lockb.unlock();
- locka.unlock();
- LOOLWSD::checkAndRestoreForKit();
- if (chrono::duration_cast<chrono::milliseconds>(chrono::steady_clock::now() - startTime).count() <
- CHILD_TIMEOUT_MS * 4)
- {
- // Try again.
- locka.lock();
- lockb.lock();
- continue;
- }
+ LOG_DBG("getNewChild: rebalancing of children failed. Checking and restoring forkit.");
- return nullptr;
- }
+ LOOLWSD::checkAndRestoreForKit();
- // With valgrind we need extended time to spawn kits.
-#ifdef KIT_IN_PROCESS
- const auto timeoutMs = CHILD_TIMEOUT_MS;
-#else
- const auto timeoutMs = CHILD_TIMEOUT_MS * (LOOLWSD::NoCapsForKit ? 100 : 1);
-#endif
- LOG_TRC("Waiting for a new child for a max of " << timeoutMs << " ms.");
- const auto timeout = chrono::milliseconds(timeoutMs);
- // FIXME: blocks ...
- if (NewChildrenCV.wait_for(lockb, timeout, []() { return !NewChildren.empty(); }))
- {
- auto child = NewChildren.back();
- NewChildren.pop_back();
- const auto available = NewChildren.size();
+ // Let the caller retry after a while.
+ return nullptr;
+ }
- // Validate before returning.
- if (child && child->isAlive())
- {
- LOG_DBG("getNewChild: Have " << available << " spare " <<
- (available == 1 ? "child" : "children") <<
- " after poping [" << child->getPid() << "] to return.");
- return child;
- }
+ // With valgrind we need extended time to spawn kits.
+ const size_t timeoutMs = CHILD_TIMEOUT_MS / 2;
+ LOG_TRC("Waiting for a new child for a max of " << timeoutMs << " ms.");
+ const auto timeout = std::chrono::milliseconds(timeoutMs);
+ // FIXME: blocks ...
+ // Unfortunately we need to wait after spawning children to avoid bombing the system.
+ // If we fail fast and return, the next document will spawn more children without knowing
+ // there are some on the way already. And if the system is slow already, that wouldn't help.
+ if (NewChildrenCV.wait_for(lock, timeout, []() { return !NewChildren.empty(); }))
+ {
+ auto child = NewChildren.back();
+ NewChildren.pop_back();
+ const auto available = NewChildren.size();
- LOG_WRN("getNewChild: popped dead child, need to find another.");
- }
- else
+ // Validate before returning.
+ if (child && child->isAlive())
{
- LOG_WRN("getNewChild: No available child. Sending spawn request to forkit and failing.");
+ LOG_DBG("getNewChild: Have " << available << " spare " <<
+ (available == 1 ? "child" : "children") <<
+ " after poping [" << child->getPid() << "] to return in " <<
+ std::chrono::duration_cast<std::chrono::milliseconds>(std::chrono::steady_clock::now() -
+ startTime).count() << "ms.");
+ return child;
}
+
+ LOG_WRN("getNewChild: popped dead child, need to find another.");
+ }
+ else
+ {
+ LOG_WRN("getNewChild: No child available. Sending spawn request to forkit and failing.");
}
- while (chrono::duration_cast<chrono::milliseconds>(chrono::steady_clock::now() - startTime).count() <
- CHILD_TIMEOUT_MS * 4);
LOG_DBG("getNewChild: Timed out while waiting for new child.");
return nullptr;
commit 2fa34440c53778747a17b5c4ad7c3ce638e8da25
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
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 492b818022c766bab123706d0cec12905f4fa8cf
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
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 48ad9a247297f9b52b05c63f6eb0a02e81ad6dcc
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
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 30d58f96a433b4b10d0f6a357a4fd19f2b5df852
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
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 d77ede8954b88a0ba63e0c71e20a6429e59a2290
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
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);
More information about the Libreoffice-commits
mailing list