[Libreoffice-commits] online.git: 2 commits - loolwsd/ChildSession.cpp loolwsd/ClientSession.cpp loolwsd/ClientSession.hpp loolwsd/DocumentBroker.cpp loolwsd/DocumentBroker.hpp loolwsd/LOOLWSD.cpp loolwsd/test loolwsd/TileCache.cpp loolwsd/TileCache.hpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Thu Sep 1 12:37:56 UTC 2016
loolwsd/ChildSession.cpp | 6 ++++
loolwsd/ClientSession.cpp | 8 ++++--
loolwsd/ClientSession.hpp | 9 +++++++
loolwsd/DocumentBroker.cpp | 14 ++---------
loolwsd/DocumentBroker.hpp | 2 -
loolwsd/LOOLWSD.cpp | 28 +++++++++++++++++-----
loolwsd/TileCache.cpp | 49 +++++-----------------------------------
loolwsd/TileCache.hpp | 3 --
loolwsd/test/Makefile.am | 1
loolwsd/test/TileCacheTests.cpp | 14 -----------
loolwsd/test/helpers.hpp | 9 -------
11 files changed, 53 insertions(+), 90 deletions(-)
New commits:
commit 16605b928999d10c8e75c3d9c0a959573f7ef55b
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Sep 1 08:27:18 2016 -0400
Revert "loolwsd: canceltiles re-designed using tilecache instead of queue"
This reverts commit 571ff06906960c9840cd65a35914ca0607f72a11.
Change-Id: Icf9caeafc640b815f64211f240cfdac8e91694a1
Reviewed-on: https://gerrit.libreoffice.org/28595
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index 88ece98..93a2312 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -119,6 +119,12 @@ bool ChildSession::_handleInput(const char *buffer, int length)
// Just to update the activity of a view-only client.
return true;
}
+ else if (tokens[0] == "canceltiles")
+ {
+ // This command makes sense only on the command queue level.
+ // Shouldn't get this here.
+ return true;
+ }
else if (tokens[0] == "commandvalues")
{
return getCommandValues(buffer, length, tokens);
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index 40deeee..901d138 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -147,8 +147,10 @@ bool ClientSession::_handleInput(const char *buffer, int length)
}
else if (tokens[0] == "canceltiles")
{
- _docBroker->cancelTileRequests(shared_from_this());
- return true;
+ if (!_peer.expired())
+ {
+ return forwardToPeer(_peer, buffer, length, false);
+ }
}
else if (tokens[0] == "commandvalues")
{
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index e313ae3..a89b9d3 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -616,13 +616,6 @@ void DocumentBroker::handleTileCombinedRequest(TileCombined& tileCombined,
}
}
-void DocumentBroker::cancelTileRequests(const std::shared_ptr<ClientSession>& session)
-{
- std::unique_lock<std::mutex> lock(_mutex);
-
- tileCache().cancelTiles(session);
-}
-
void DocumentBroker::handleTileResponse(const std::vector<char>& payload)
{
const std::string firstLine = getFirstLine(payload);
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 80bf0a3..54ca138 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -214,8 +214,6 @@ public:
void handleTileCombinedRequest(TileCombined& tileCombined,
const std::shared_ptr<ClientSession>& session);
- void cancelTileRequests(const std::shared_ptr<ClientSession>& session);
-
void handleTileResponse(const std::vector<char>& payload);
void handleTileCombinedResponse(const std::vector<char>& payload);
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index 8f05a14..9b76a15 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -461,26 +461,4 @@ int TileCache::subscribeToTileRendering(const TileDesc& tile, const std::shared_
}
}
-void TileCache::cancelTiles(const std::shared_ptr<ClientSession> &subscriber)
-{
- std::unique_lock<std::mutex> lock(_tilesBeingRenderedMutex);
-
- const auto sub = subscriber.get();
-
- Log::trace("Cancelling tiles for " + subscriber->getName());
-
- for (auto it = _tilesBeingRendered.begin(); it != _tilesBeingRendered.end(); )
- {
- auto& subscribers = it->second->_subscribers;
- Log::trace("Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers.");
- subscribers.erase(std::remove_if(subscribers.begin(), subscribers.end(),
- [sub](std::weak_ptr<ClientSession>& ptr){ return ptr.lock().get() == sub; }),
- subscribers.end());
- Log::trace(" Tile " + it->first + " has " + std::to_string(subscribers.size()) + " subscribers.");
-
- // Remove if there are no more subscribers on this tile.
- it = (subscribers.empty() ? _tilesBeingRendered.erase(it) : ++it);
- }
-}
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/TileCache.hpp b/loolwsd/TileCache.hpp
index 5703227..bbe5aa0 100644
--- a/loolwsd/TileCache.hpp
+++ b/loolwsd/TileCache.hpp
@@ -42,9 +42,6 @@ public:
/// Otherwise returns 0 to signify a subscription exists.
int subscribeToTileRendering(const TileDesc& tile, const std::shared_ptr<ClientSession> &subscriber);
- /// Cancels all tile requests by the given subscriber.
- void cancelTiles(const std::shared_ptr<ClientSession> &subscriber);
-
std::unique_ptr<std::fstream> lookupTile(const TileDesc& tile);
void saveTileAndNotify(const TileDesc& tile, const char *data, const size_t size, const bool priority);
diff --git a/loolwsd/test/TileCacheTests.cpp b/loolwsd/test/TileCacheTests.cpp
index d2bfa5e..e44a18d 100644
--- a/loolwsd/test/TileCacheTests.cpp
+++ b/loolwsd/test/TileCacheTests.cpp
@@ -33,7 +33,6 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
CPPUNIT_TEST(testSimple);
CPPUNIT_TEST(testSimpleCombine);
CPPUNIT_TEST(testPerformance);
- CPPUNIT_TEST(testCancelTiles);
CPPUNIT_TEST(testUnresponsiveClient);
CPPUNIT_TEST(testClientPartImpress);
CPPUNIT_TEST(testClientPartCalc);
@@ -49,7 +48,6 @@ class TileCacheTests : public CPPUNIT_NS::TestFixture
void testSimple();
void testSimpleCombine();
void testPerformance();
- void testCancelTiles();
void testUnresponsiveClient();
void testClientPartImpress();
void testClientPartCalc();
@@ -210,18 +208,6 @@ void TileCacheTests::testPerformance()
socket.shutdown();
}
-void TileCacheTests::testCancelTiles()
-{
- const auto testName = "cancelTiles ";
- auto socket = *loadDocAndGetSocket("load12.ods", _uri, testName);
-
- // Request a huge tile, and cancel immediately.
- sendTextFrame(socket, "tilecombine part=0 width=2560 height=2560 tileposx=0 tileposy=0 tilewidth=38400 tileheight=38400");
- sendTextFrame(socket, "canceltiles");
-
- assertNotInResponse(socket, "tile:", testName);
-}
-
void TileCacheTests::testUnresponsiveClient()
{
const std::string docFilename = "hello.odt";
diff --git a/loolwsd/test/helpers.hpp b/loolwsd/test/helpers.hpp
index 7933beb..0a7d003 100644
--- a/loolwsd/test/helpers.hpp
+++ b/loolwsd/test/helpers.hpp
@@ -330,15 +330,6 @@ std::string assertResponseLine(T& ws, const std::string& prefix, const std::stri
return res;
}
-/// Assert that we don't get a response with the given prefix.
-template <typename T>
-std::string assertNotInResponse(T& ws, const std::string& prefix, const std::string name = "")
-{
- const auto res = getResponseLine(ws, prefix, name);
- CPPUNIT_ASSERT_MESSAGE("Did not expect getting message [" + res + "].", res.empty());
- return res;
-}
-
inline
void getResponseMessage(const std::shared_ptr<Poco::Net::WebSocket>& ws, const std::string& prefix, std::string& response, const bool isLine)
{
commit 77a693c353a35607742572e241be9324f5a345d5
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Thu Sep 1 08:25:32 2016 -0400
Revert "loolwsd: remove tile queue and simplify tile response"
This reverts commit 59eaacd2f87f46c4c4d2963ef54f4d20d346b2d0.
Change-Id: Ieba9bbaaa6406e3e685b46ce12a44a0766127815
Reviewed-on: https://gerrit.libreoffice.org/28594
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/ClientSession.cpp b/loolwsd/ClientSession.cpp
index c61c574..40deeee 100644
--- a/loolwsd/ClientSession.cpp
+++ b/loolwsd/ClientSession.cpp
@@ -37,9 +37,11 @@ using Poco::StringTokenizer;
ClientSession::ClientSession(const std::string& id,
std::shared_ptr<Poco::Net::WebSocket> ws,
std::shared_ptr<DocumentBroker> docBroker,
+ std::shared_ptr<BasicTileQueue> queue,
bool readOnly) :
LOOLSession(id, Kind::ToClient, ws),
_docBroker(std::move(docBroker)),
+ _queue(std::move(queue)),
_haveEditLock(std::getenv("LOK_VIEW_CALLBACK")),
_isReadOnly(readOnly),
_loadFailed(false),
diff --git a/loolwsd/ClientSession.hpp b/loolwsd/ClientSession.hpp
index 3a9b1a1..dc531c8 100644
--- a/loolwsd/ClientSession.hpp
+++ b/loolwsd/ClientSession.hpp
@@ -23,6 +23,7 @@ public:
ClientSession(const std::string& id,
std::shared_ptr<Poco::Net::WebSocket> ws,
std::shared_ptr<DocumentBroker> docBroker,
+ std::shared_ptr<BasicTileQueue> queue,
bool isReadOnly = false);
virtual ~ClientSession();
@@ -59,6 +60,11 @@ public:
_loadFailed = true;
}
+ void sendToInputQueue(const std::string& message)
+ {
+ _queue->put(message);
+ }
+
std::shared_ptr<DocumentBroker> getDocumentBroker() const { return _docBroker; }
private:
@@ -79,6 +85,9 @@ private:
std::shared_ptr<DocumentBroker> _docBroker;
+ /// The incoming message queue.
+ std::shared_ptr<BasicTileQueue> _queue;
+
// If this document holds the edit lock.
// An edit lock will only allow the current session to make edits,
// while other session opening the same document can only see
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 2ae259f..e313ae3 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -317,6 +317,7 @@ bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
_lastFileModifiedTime.fromEpochTime(0);
// We do not want save to terminate editing mode if we are in edit mode now
+
std::ostringstream oss;
// arguments init
oss << "{";
@@ -342,10 +343,8 @@ bool DocumentBroker::sendUnoSave(const bool dontSaveIfUnmodified)
// arguments end
oss << "}";
- const auto saveArgs = oss.str();
- Log::trace(".uno:Save arguments: " + saveArgs);
- const auto command = "uno .uno:Save " + saveArgs;
- sessionIt.second->handleInput(command.data(), command.size());
+ Log::debug(".uno:Save arguments: " + oss.str());
+ sessionIt.second->sendToInputQueue("uno .uno:Save " + oss.str());
return true;
}
}
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 0637bf3..db7f135 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -399,7 +399,7 @@ private:
// Load the document.
std::shared_ptr<WebSocket> ws;
- auto session = std::make_shared<ClientSession>(id, ws, docBroker);
+ auto session = std::make_shared<ClientSession>(id, ws, docBroker, nullptr);
// Request the child to connect to us and add this session.
auto sessionsCount = docBroker->addSession(session);
@@ -662,7 +662,10 @@ private:
std::shared_ptr<ClientSession> session;
try
{
- session = std::make_shared<ClientSession>(id, ws, docBroker, isReadOnly);
+ // For ToClient sessions, we store incoming messages in a queue and have a separate
+ // thread to pump them. This is to empty the queue when we get a "canceltiles" message.
+ auto queue = std::make_shared<BasicTileQueue>();
+ session = std::make_shared<ClientSession>(id, ws, docBroker, queue, isReadOnly);
if (!fileinfo._userName.empty())
{
Log::debug(uriPublic.toString() + " requested with username [" + fileinfo._userName + "]");
@@ -689,13 +692,18 @@ private:
ws->sendFrame(status.data(), (int) status.size());
// Let messages flow
+ QueueHandler handler(queue, session, "wsd_queue_" + session->getId());
+ Thread queueHandlerThread;
+ queueHandlerThread.start(handler);
+
IoUtil::SocketProcessor(ws,
- [&session](const std::vector<char>& payload)
+ [&queue](const std::vector<char>& payload)
{
- return session->handleInput(payload.data(), payload.size());
+ queue->put(payload);
+ return true;
},
[&session]() { session->closeFrame(); },
- []() { return !!TerminationFlag; });
+ [&queueHandlerThread]() { return TerminationFlag || !queueHandlerThread.isRunning(); });
{
std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
@@ -735,6 +743,12 @@ private:
}
}
+ if (session->isLoadFailed())
+ {
+ Log::info("Clearing the queue.");
+ queue->clear();
+ }
+
if (sessionsCount == 0)
{
std::unique_lock<std::mutex> docBrokersLock(docBrokersMutex);
@@ -748,7 +762,9 @@ private:
}
LOOLWSD::dumpEventTrace(docBroker->getJailId(), id, "EndSession: " + uri);
- Log::info("Finishing GET request handler for session [" + id + "].");
+ Log::info("Finishing GET request handler for session [" + id + "]. Joining the queue.");
+ queue->put("eof");
+ queueHandlerThread.join();
}
catch (const std::exception& exc)
{
diff --git a/loolwsd/TileCache.cpp b/loolwsd/TileCache.cpp
index ca6af9b..8f05a14 100644
--- a/loolwsd/TileCache.cpp
+++ b/loolwsd/TileCache.cpp
@@ -166,32 +166,19 @@ void TileCache::saveTileAndNotify(const TileDesc& tile, const char *data, const
{
if (!tileBeingRendered->_subscribers.empty())
{
- std::string response = tile.serialize("tile:");
- Log::debug("Sending tile message to subscribers: " + response);
- response += '\n';
-
- std::vector<char> output;
- output.reserve(static_cast<size_t>(4) * tile.getWidth() * tile.getHeight());
- output.resize(response.size());
- std::memcpy(output.data(), response.data(), response.size());
-
- const auto pos = output.size();
- output.resize(pos + size);
- std::memcpy(output.data() + pos, data, size);
+ const std::string message = tile.serialize("tile");
+ Log::debug("Sending tile message to subscribers: " + message);
for (const auto& i: tileBeingRendered->_subscribers)
{
auto subscriber = i.lock();
if (subscriber)
{
- try
- {
- subscriber->sendBinaryFrame(output.data(), output.size());
- }
- catch (const std::exception& ex)
- {
- Log::warn("Failed to send tile to " + subscriber->getName() + ": " + ex.what());
- }
+ //FIXME: This is inefficient; should just send directly to each client (although that is risky as well!
+ // Re-emit the tile command in the other thread(s) to re-check and hit
+ // the cache. Construct the message from scratch to contain only the
+ // mandatory parts of the message.
+ subscriber->sendToInputQueue(message);
}
}
}
diff --git a/loolwsd/test/Makefile.am b/loolwsd/test/Makefile.am
index 53f7d97..91401dc 100644
--- a/loolwsd/test/Makefile.am
+++ b/loolwsd/test/Makefile.am
@@ -21,7 +21,6 @@ wsd_sources = \
../IoUtil.cpp \
../Log.cpp \
../LOOLProtocol.cpp \
- ../LOOLSession.cpp \
../TileCache.cpp \
../MessageQueue.cpp \
../Unit.cpp \
More information about the Libreoffice-commits
mailing list