loolwsd/ChildSession.cpp | 7 ------
loolwsd/ChildSession.hpp | 4 ++-
loolwsd/LOOLKit.cpp | 52 ++++++++++++++++++-----------------------------
3 files changed, 25 insertions(+), 38 deletions(-)
New commits:
commit a28b832309e30ea609df85582956643c1db29385
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date: Sun Aug 21 11:12:12 2016 -0400
loolwsd: proper ChildSession cleanup
ChildSession cleanup is tricky because it needs
to be cleaned up when the connection is dropped.
The ChildSession itself needs to initiate this
cleanup from Document.
A new approach simplifies the design and correctly
broadcasts remview to all other connections so
they would be able to cleanup visual elements.
Change-Id: I78fd01fb42b801913534c858324c16dd7ad6451d
Reviewed-on: https://gerrit.libreoffice.org/28302
Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
Tested-by: Ashod Nakashian <ashnak...@gmail.com>
diff --git a/loolwsd/ChildSession.cpp b/loolwsd/ChildSession.cpp
index 51b445f..d94b68b 100644
--- a/loolwsd/ChildSession.cpp
+++ b/loolwsd/ChildSession.cpp
@@ -58,14 +58,9 @@ void ChildSession::disconnect()
{
std::unique_lock<std::recursive_mutex> lock(Mutex);
- sendTextFrame("remview: " + std::to_string(_viewId));
-
if (_viewId >= 0)
{
- if (_multiView && _loKitDocument)
- _loKitDocument->setView(_viewId);
-
- _docManager.onUnload(getId());
+ _docManager.onUnload(*this);
}
else
{
diff --git a/loolwsd/ChildSession.hpp b/loolwsd/ChildSession.hpp
index e69c415..1f6d2eb 100644
--- a/loolwsd/ChildSession.hpp
+++ b/loolwsd/ChildSession.hpp
@@ -19,6 +19,8 @@
#include "LOOLSession.hpp"
#include "LibreOfficeKit.hpp"
+class ChildSession;
+
/// An abstract interface that defines the
/// DocumentManager interface and functionality.
class IDocumentManager
@@ -35,7 +37,7 @@ public:
/// Unload a client session, which unloads the document
/// if it is the last and only.
virtual
- void onUnload(const std::string& sessionId) = 0;
+ void onUnload(const ChildSession& session) = 0;
/// Get a list of all current view IDs.
virtual
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index a1df19f..897f033 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -248,6 +248,7 @@ public:
join();
}
+ const std::string& getSessionId() const { return _sessionId; };
std::shared_ptr<WebSocket> getWebSocket() const { return _ws; }
std::shared_ptr<ChildSession> getSession() { return _session; }
@@ -325,8 +326,6 @@ public:
Log::error("Connection::run:: Unexpected exception");
}
- // Release the session and unload view.
- _session.reset();
Log::debug("Thread finished.");
}
@@ -882,55 +881,46 @@ private:
return _loKitDocument;
}
- void onUnload(const std::string& sessionId) override
+ void onUnload(const ChildSession& session) override
{
+ const auto sessionId = session.getId();
Log::info("Unloading [" + sessionId + "].");
- const unsigned intSessionId = Util::decodeId(sessionId);
- if (_loKitDocument == nullptr)
- {
- Log::error("Unloading session [" + sessionId + "] without
loKitDocument.");
- return;
- }
-
- // Find this session connection.
- int sessionViewId = -1;
+ // Broadcast the demise and removal of session.
{
std::unique_lock<std::mutex> lock(_mutex);
- const auto it = _connections.find(intSessionId);
- if (it == _connections.end() || !it->second ||
!it->second->getSession())
+ // We should be removed by this point, otherwise
+ // our closed connection will throw, if not segfault.
+ for (const auto& pair : _connections)
{
- Log::error("Session [" + sessionId + "] not found to unload.");
- return;
+ assert(sessionId != pair.second->getSessionId() && "Unloading
connection still lingering.");
+ pair.second->getSession()->sendTextFrame("remview: " +
sessionId);
}
- sessionViewId = it->second->getSession()->getViewId();
+ if (_loKitDocument == nullptr)
+ {
+ Log::error("Unloading session [" + sessionId + "] without
loKitDocument.");
+ return;
+ }
}
--_clientViews;
- Log::info("Session " + sessionId + " is unloading. " +
std::to_string(_clientViews) +
- " view" + (_clientViews != 1 ? "s" : "") + " remain.");
+ Log::info() << "Document [" << _url << "] session ["
+ << sessionId << "] unloaded, " << _clientViews
+ << " view" << (_clientViews != 1 ? "s" : "")
+ << Log::end;
if (_multiView)
{
- Log::info() << "Document [" << _url << "] session ["
- << sessionId << "] unloaded, leaving "
- << _clientViews << " views." << Log::end;
-
std::unique_lock<std::mutex> lock(_loKitDocument->getLock());
- const auto viewId = _loKitDocument->getView();
- if (viewId != sessionViewId)
- {
- Log::error() << "Unloading view [" << sessionViewId
- << "] from view [" << viewId << "]." << Log::end;
- return;
- }
-
+ const auto viewId = session.getViewId();
_viewIdToCallbackDescr.erase(viewId);
+ _loKitDocument->setView(viewId);
_loKitDocument->registerCallback(nullptr, nullptr);
_loKitDocument->destroyView(viewId);
+ Log::debug("Destroyed view " + std::to_string(viewId));
}
}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
