loolwsd/DocumentBroker.cpp | 59 ++++++++++++++++++++++++---------------------
loolwsd/DocumentBroker.hpp | 20 ++++++---------
loolwsd/LOOLWSD.cpp | 22 ++++++----------
3 files changed, 50 insertions(+), 51 deletions(-)
New commits:
commit 372baaf427805f8cad79f3ef817c63d33ae20ce3
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date: Sat Apr 16 17:18:51 2016 -0400
loolwsd: cleanup of DocumentBroker session management
Improved session add/remove API.
Reduced mutex count.
Renamed members and variables.
Added documentation.
Change-Id: If15991971484d4d508714c9436a51b291f42079f
Reviewed-on: https://gerrit.libreoffice.org/24158
Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
Tested-by: Ashod Nakashian <ashnak...@gmail.com>
diff --git a/loolwsd/DocumentBroker.cpp b/loolwsd/DocumentBroker.cpp
index 0274cf0..85a8d17 100644
--- a/loolwsd/DocumentBroker.cpp
+++ b/loolwsd/DocumentBroker.cpp
@@ -151,8 +151,8 @@ bool DocumentBroker::save()
bool DocumentBroker::autoSave(const bool force)
{
- std::unique_lock<std::mutex> sessionsLock(_wsSessionsMutex);
- if (_wsSessions.empty())
+ std::unique_lock<std::mutex> lock(_mutex);
+ if (_sessions.empty())
{
// Shouldn't happen.
return false;
@@ -160,7 +160,7 @@ bool DocumentBroker::autoSave(const bool force)
// Find the most recent activity.
double inactivityTimeMs = std::numeric_limits<double>::max();
- for (auto& sessionIt: _wsSessions)
+ for (auto& sessionIt: _sessions)
{
inactivityTimeMs = std::min(sessionIt.second->getInactivityMS(),
inactivityTimeMs);
}
@@ -182,7 +182,7 @@ bool DocumentBroker::autoSave(const bool force)
// Save using session holding the edit-lock
bool sent = false;
- for (auto& sessionIt: _wsSessions)
+ for (auto& sessionIt: _sessions)
{
if (!sessionIt.second->isEditLocked())
continue;
@@ -231,8 +231,8 @@ std::string DocumentBroker::getJailRoot() const
void DocumentBroker::takeEditLock(const std::string& id)
{
- std::lock_guard<std::mutex> sessionsLock(_wsSessionsMutex);
- for (auto& it: _wsSessions)
+ std::lock_guard<std::mutex> lock(_mutex);
+ for (auto& it: _sessions)
{
if (it.first != id)
{
@@ -247,51 +247,56 @@ void DocumentBroker::takeEditLock(const std::string& id)
}
}
-void DocumentBroker::addWSSession(const std::string& id,
std::shared_ptr<MasterProcessSession>& ws)
+size_t DocumentBroker::addSession(std::shared_ptr<MasterProcessSession>&
session)
{
- std::lock_guard<std::mutex> sessionsLock(_wsSessionsMutex);
+ const auto id = session->getId();
+
+ std::lock_guard<std::mutex> lock(_mutex);
- auto ret = _wsSessions.emplace(id, ws);
+ auto ret = _sessions.emplace(id, session);
if (!ret.second)
{
Log::warn("DocumentBroker: Trying to add already existed session.");
}
- if (_wsSessions.size() == 1)
+ if (_sessions.size() == 1)
{
- ws->setEditLock(true);
- ws->sendTextFrame("editlock: 1");
+ session->setEditLock(true);
+ session->sendTextFrame("editlock: 1");
}
// Request a new session from the child kit.
const std::string aMessage = "session " + id + " " + _docKey + "\n";
Log::debug("DocBroker to Child: " + aMessage.substr(0, aMessage.length() -
1));
_childProcess->getWebSocket()->sendFrame(aMessage.data(), aMessage.size());
+
+ return _sessions.size();
}
-void DocumentBroker::removeWSSession(const std::string& id)
+size_t DocumentBroker::removeSession(const std::string& id)
{
- std::lock_guard<std::mutex> sessionsLock(_wsSessionsMutex);
+ std::lock_guard<std::mutex> lock(_mutex);
- bool haveEditLock = false;
- auto it = _wsSessions.find(id);
- if (it != _wsSessions.end())
+ auto it = _sessions.find(id);
+ if (it != _sessions.end())
{
- haveEditLock = it->second->isEditLocked();
+ const auto haveEditLock = it->second->isEditLocked();
it->second->setEditLock(false);
- _wsSessions.erase(it);
- }
+ _sessions.erase(it);
- if (haveEditLock)
- {
- // pass the edit lock to first session in map
- it = _wsSessions.begin();
- if (it != _wsSessions.end())
+ if (haveEditLock)
{
- it->second->setEditLock(true);
- it->second->sendTextFrame("editlock: 1");
+ // pass the edit lock to first session in map
+ it = _sessions.begin();
+ if (it != _sessions.end())
+ {
+ it->second->setEditLock(true);
+ it->second->sendTextFrame("editlock: 1");
+ }
}
}
+
+ return _sessions.size();
}
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index 599fb3f..df8287a 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -147,10 +147,10 @@ public:
const std::string& getDocKey() const { return _docKey; }
const std::string& getFilename() const { return _filename; };
TileCache& tileCache() { return *_tileCache; }
- unsigned getSessionsCount() const
+ size_t getSessionsCount() const
{
- std::lock_guard<std::mutex> sessionsLock(_wsSessionsMutex);
- return _wsSessions.size();
+ std::lock_guard<std::mutex> lock(_mutex);
+ return _sessions.size();
}
/// Returns the time in milliseconds since last save.
@@ -166,11 +166,10 @@ public:
/// except this one
void takeEditLock(const std::string& id);
- void addWSSession(const std::string& id,
std::shared_ptr<MasterProcessSession>& ws);
-
- void removeWSSession(const std::string& id);
-
- unsigned getWSSessionsCount() { return _wsSessions.size(); }
+ /// Add a new session. Returns the new number of sessions.
+ size_t addSession(std::shared_ptr<MasterProcessSession>& session);
+ /// Removes a session by ID. Returns the new number of sessions.
+ size_t removeSession(const std::string& id);
void kill() { _childProcess->close(true); };
@@ -183,12 +182,11 @@ private:
std::string _jailId;
std::string _filename;
std::chrono::steady_clock::time_point _lastSaveTime;
- std::map<std::string, std::shared_ptr<MasterProcessSession>> _wsSessions;
- mutable std::mutex _wsSessionsMutex;
+ std::map<std::string, std::shared_ptr<MasterProcessSession>> _sessions;
std::unique_ptr<StorageBase> _storage;
std::unique_ptr<TileCache> _tileCache;
std::shared_ptr<ChildProcess> _childProcess;
- std::mutex _mutex;
+ mutable std::mutex _mutex;
std::condition_variable _saveCV;
std::mutex _saveMutex;
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 99ab259..30c52e9 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -346,10 +346,9 @@ private:
// Load the document.
std::shared_ptr<WebSocket> ws;
auto session = std::make_shared<MasterProcessSession>(id,
LOOLSession::Kind::ToClient, ws, docBroker, nullptr);
- docBroker->addWSSession(id, session);
- auto wsSessionsCount = docBroker->getWSSessionsCount();
- Log::trace(docKey + ", ws_sessions++: " +
std::to_string(wsSessionsCount));
+ auto sessionsCount = docBroker->addSession(session);
lock.unlock();
+ Log::trace(docKey + ", ws_sessions++: " +
std::to_string(sessionsCount));
if (!waitBridgeCompleted(session, docBroker))
{
@@ -384,9 +383,8 @@ private:
}
lock.lock();
- docBroker->removeWSSession(id);
- wsSessionsCount = docBroker->getWSSessionsCount();
- if (wsSessionsCount == 0)
+ sessionsCount = docBroker->removeSession(id);
+ if (sessionsCount == 0)
{
Log::debug("Removing DocumentBroker for docKey [" +
docKey + "].");
docBrokers.erase(docKey);
@@ -523,10 +521,9 @@ private:
// "canceltiles" message.
auto queue = std::make_shared<BasicTileQueue>();
auto session = std::make_shared<MasterProcessSession>(id,
LOOLSession::Kind::ToClient, ws, docBroker, queue);
- docBroker->addWSSession(id, session);
- auto wsSessionsCount = docBroker->getWSSessionsCount();
- Log::trace(docKey + ", ws_sessions++: " +
std::to_string(wsSessionsCount));
+ auto sessionsCount = docBroker->addSession(session);
docBrokersLock.unlock();
+ Log::trace(docKey + ", ws_sessions++: " +
std::to_string(sessionsCount));
// indicator to a client that is waiting to connect to lokit process
status = "statusindicator: connect";
@@ -583,10 +580,9 @@ private:
queueHandlerThread.join();
docBrokersLock.lock();
- docBroker->removeWSSession(id);
- wsSessionsCount = docBroker->getWSSessionsCount();
- Log::trace(docKey + ", ws_sessions--: " +
std::to_string(wsSessionsCount));
- if (wsSessionsCount == 0)
+ sessionsCount = docBroker->removeSession(id);
+ Log::trace(docKey + ", ws_sessions--: " +
std::to_string(sessionsCount));
+ if (sessionsCount == 0)
{
Log::debug("Removing DocumentBroker for docKey [" + docKey + "].");
docBrokers.erase(docKey);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
