loolwsd/Admin.cpp | 31 ++++++-----------------------
loolwsd/Admin.hpp | 29 +++++++++++++++++++--------
loolwsd/AdminModel.cpp | 6 -----
loolwsd/DocumentBroker.hpp | 6 -----
loolwsd/LOOLKit.cpp | 40 -------------------------------------
loolwsd/LOOLWSD.cpp | 48 +++++++++++++++++++++++++++++++++++----------
loolwsd/Util.cpp | 6 ++---
loolwsd/Util.hpp | 2 -
8 files changed, 71 insertions(+), 97 deletions(-)
New commits:
commit 6e5e9033f26c029e019ce43ccfaaadd2cd30582c
Author: Ashod Nakashian <ashod.nakash...@collabora.co.uk>
Date: Wed Apr 6 08:43:44 2016 -0400
loolwsd: removed Admin pipe
Admin no longer needs a pipe as it's notified
from WSD. It is now a singleton with improved
locking.
The tracking of documents and views still needs
improvement and corrections.
Change-Id: If614331de6dd595c6dd4443f480d4ab588ca4551
Reviewed-on: https://gerrit.libreoffice.org/23860
Reviewed-by: Ashod Nakashian <ashnak...@gmail.com>
Tested-by: Ashod Nakashian <ashnak...@gmail.com>
diff --git a/loolwsd/Admin.cpp b/loolwsd/Admin.cpp
index 572aba3..275434e 100644
--- a/loolwsd/Admin.cpp
+++ b/loolwsd/Admin.cpp
@@ -64,7 +64,7 @@ void AdminRequestHandler::handleWSRequests(HTTPServerRequest&
request, HTTPServe
auto ws = std::make_shared<WebSocket>(request, response);
{
- std::lock_guard<std::mutex> modelLock(_admin->_modelMutex);
+ std::unique_lock<std::mutex> modelLock(_admin->getLock());
// Subscribe the websocket of any AdminModel updates
AdminModel& model = _admin->getModel();
model.subscribe(nSessionId, ws);
@@ -120,7 +120,7 @@ void
AdminRequestHandler::handleWSRequests(HTTPServerRequest& request, HTTPServe
continue;
// Lock the model mutex before interacting with it
- std::lock_guard<std::mutex> modelLock(_admin->_modelMutex);
+ std::unique_lock<std::mutex> modelLock(_admin->getLock());
AdminModel& model = _admin->getModel();
if (tokens[0] == "stats")
@@ -396,11 +396,9 @@ void AdminRequestHandler::handleRequest(HTTPServerRequest&
request, HTTPServerRe
}
/// An admin command processor.
-Admin::Admin(const Poco::Process::PID brokerPid, const int notifyPipe) :
+Admin::Admin() :
_model(AdminModel())
{
- Admin::BrokerPid = brokerPid;
- Admin::AdminNotifyPipe = notifyPipe;
}
Admin::~Admin()
@@ -408,12 +406,7 @@ Admin::~Admin()
Log::info("~Admin dtor.");
}
-AdminRequestHandler* Admin::createRequestHandler()
-{
- return new AdminRequestHandler(this);
-}
-
-void Admin::handleInput(std::string& message)
+void Admin::update(const std::string& message)
{
std::lock_guard<std::mutex> modelLock(_modelMutex);
_model.update(message);
@@ -421,7 +414,7 @@ void Admin::handleInput(std::string& message)
void MemoryStats::run()
{
- std::lock_guard<std::mutex> modelLock(_admin->_modelMutex);
+ std::unique_lock<std::mutex> modelLock(_admin->getLock());
AdminModel& model = _admin->getModel();
unsigned totalMem = _admin->getTotalMemoryUsage(model);
@@ -432,7 +425,7 @@ void MemoryStats::run()
void CpuStats::run()
{
//TODO: Implement me
- //std::lock_guard<std::mutex> modelLock(_admin->_modelMutex);
+ //std::unique_lock<std::mutex> modelLock(_admin->getLock());
//model.addCpuStats(totalMem);
}
@@ -456,8 +449,7 @@ void Admin::rescheduleCpuTimer(unsigned interval)
unsigned Admin::getTotalMemoryUsage(AdminModel& model)
{
- Poco::Process::PID nBrokerPid = getBrokerPid();
- unsigned totalMem = Util::getMemoryUsage(nBrokerPid);
+ unsigned totalMem = Util::getMemoryUsage(_brokerPid);
totalMem += model.getTotalMemoryUsage();
totalMem += Util::getMemoryUsage(Poco::Process::id());
@@ -489,11 +481,6 @@ void Admin::run()
Log::info("Thread [" + thread_name + "] started.");
- // Start listening for data changes.
- IoUtil::PipeReader pipeReader(FIFO_ADMIN_NOTIFY, AdminNotifyPipe);
- pipeReader.process([this](std::string& message) { handleInput(message);
return true; },
- []() { return TerminationFlag; });
-
_memStatsTimer.cancel();
_cpuStatsTimer.cancel();
@@ -505,8 +492,4 @@ AdminModel& Admin::getModel()
return _model;
}
-//TODO: Clean up with something more elegant.
-Poco::Process::PID Admin::BrokerPid;
-int Admin::AdminNotifyPipe;
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/Admin.hpp b/loolwsd/Admin.hpp
index 5eef223..a235669 100644
--- a/loolwsd/Admin.hpp
+++ b/loolwsd/Admin.hpp
@@ -40,16 +40,24 @@ private:
class Admin : public Poco::Runnable
{
public:
- Admin(const Poco::Process::PID brokerPid, const int notifyPipe);
+ virtual ~Admin();
- ~Admin();
-
- static int getBrokerPid() { return Admin::BrokerPid; }
+ static Admin& instance()
+ {
+ static Admin admin;
+ return admin;
+ }
unsigned getTotalMemoryUsage(AdminModel&);
void run() override;
+ /// Update the Admin Model.
+ void update(const std::string& message);
+
+ /// Set the broker ProcessID.
+ void setBrokerPid(const int brokerPid) { _brokerPid = brokerPid; }
+
/// Callers must ensure that modelMutex is acquired
AdminModel& getModel();
@@ -61,16 +69,21 @@ public:
void rescheduleCpuTimer(unsigned interval);
- AdminRequestHandler* createRequestHandler();
+ static
+ AdminRequestHandler* createRequestHandler()
+ {
+ return new AdminRequestHandler(&instance());
+ }
-public:
- std::mutex _modelMutex;
+ std::unique_lock<std::mutex> getLock() { return
std::unique_lock<std::mutex>(_modelMutex); }
private:
- void handleInput(std::string& message);
+ Admin();
private:
AdminModel _model;
+ std::mutex _modelMutex;
+ int _brokerPid;
Poco::Util::Timer _memStatsTimer;
Poco::Util::TimerTask::Ptr _memStatsTask;
diff --git a/loolwsd/AdminModel.cpp b/loolwsd/AdminModel.cpp
index 7498fc9..debd8a8 100644
--- a/loolwsd/AdminModel.cpp
+++ b/loolwsd/AdminModel.cpp
@@ -280,11 +280,7 @@ void AdminModel::notify(const std::string& message)
void AdminModel::addDocument(Poco::Process::PID pid, std::string url)
{
- const auto ret = _documents.emplace(pid, Document(pid, url));
- if (!ret.second)
- {
- Log::warn() << "Document with PID [" + std::to_string(pid) + "]
already exists." << Log::end;
- }
+ _documents.emplace(pid, Document(pid, url));
}
void AdminModel::removeDocument(Poco::Process::PID pid)
diff --git a/loolwsd/DocumentBroker.hpp b/loolwsd/DocumentBroker.hpp
index f63c5a5..6ca9906 100644
--- a/loolwsd/DocumentBroker.hpp
+++ b/loolwsd/DocumentBroker.hpp
@@ -84,12 +84,6 @@ public:
Log::error("Cannot terminate lokit [" + std::to_string(_pid) +
"]. Abandoning.");
}
- //TODO: Notify Admin.
- std::ostringstream message;
- message << "rmdoc" << " "
- << _pid << " "
- << "\n";
- //IoUtil::writeFIFO(WriterNotify, message.str());
_pid = -1;
}
}
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index d89f315..468915c 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -54,8 +54,6 @@
typedef int (LokHookPreInit) (const char *install_path, const char
*user_profile_path);
-static int WriterNotify = -1;
-
using namespace LOOLProtocol;
using Poco::Exception;
@@ -736,13 +734,6 @@ private:
--_clientViews;
Log::info("Session " + sessionId + " is unloading. " +
std::to_string(_clientViews) + " views will remain.");
- std::ostringstream message;
- message << "rmview" << " "
- << Process::id() << " "
- << sessionId << " "
- << "\n";
- IoUtil::writeFIFO(WriterNotify, message.str());
-
if (_multiView && _loKitDocument)
{
Log::info() << "Document [" << _url << "] session ["
@@ -814,14 +805,6 @@ private:
return nullptr;
}
- // Notify the Admin thread
- std::ostringstream message;
- message << "document" << " "
- << Process::id() << " "
- << uri.substr(uri.find_last_of("/") + 1) << " "
- << "\n";
- IoUtil::writeFIFO(WriterNotify, message.str());
-
if (_multiView)
{
Log::info("Loading view to document from URI: [" + uri + "]
for session [" + sessionId + "].");
@@ -861,13 +844,6 @@ private:
}
}
- std::ostringstream message;
- message << "addview" << " "
- << Process::id() << " "
- << sessionId << " "
- << "\n";
- IoUtil::writeFIFO(WriterNotify, message.str());
-
return _loKitDocument;
}
@@ -935,15 +911,6 @@ void lokit_main(const std::string& childRoot,
try
{
- // Open notify pipe
- const Path pipePath = Path::forDirectory(childRoot + Path::separator()
+ FIFO_PATH);
- const std::string pipeNotify = Path(pipePath,
FIFO_ADMIN_NOTIFY).toString();
- if ((WriterNotify = open(pipeNotify.c_str(), O_WRONLY) ) < 0)
- {
- Log::error("Error: failed to open notify pipe [" +
std::string(FIFO_ADMIN_NOTIFY) + "] for writing.");
- exit(Application::EXIT_SOFTWARE);
- }
-
const Path jailPath = Path::forDirectory(childRoot + Path::separator()
+ jailId);
Log::info("Jail path: " + jailPath.toString());
File(jailPath).createDirectories();
@@ -1115,13 +1082,6 @@ void lokit_main(const std::string& childRoot,
Log::error(std::string("Exception: ") + exc.what());
}
- std::ostringstream message;
- message << "rmdoc" << " "
- << Process::id() << " "
- << "\n";
- IoUtil::writeFIFO(WriterNotify, message.str());
- close(WriterNotify);
-
Log::info("Process [" + process_name + "] finished.");
_exit(Application::EXIT_OK);
}
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index 3de2188..4a222e8 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -318,6 +318,12 @@ private:
{
Log::debug("Removing DocumentBroker for docKey [" +
docKey + "].");
docBrokers.erase(docKey);
+
+ std::ostringstream message;
+ message << "rmdoc" << " "
+ << docBroker->getJailId() << " "
+ << "\n";
+ Admin::instance().update(message.str());
}
}
@@ -669,11 +675,11 @@ public:
return;
}
+ std::string sessionId;
+ std::string jailId;
try
{
const auto params =
Poco::URI(request.getURI()).getQueryParameters();
- std::string sessionId;
- std::string jailId;
std::string docKey;
for (const auto& param : params)
{
@@ -738,6 +744,20 @@ public:
lock.unlock();
MasterProcessSession::AvailableChildSessionCV.notify_one();
+ const auto uri = request.getURI();
+ std::ostringstream message;
+ message << "document" << " "
+ << jailId << " "
+ << uri.substr(uri.find_last_of("/") + 1) << " "
+ << "\n";
+ Admin::instance().update(message.str());
+
+ message << "addview" << " "
+ << jailId << " "
+ << sessionId << " "
+ << "\n";
+ Admin::instance().update(message.str());
+
IoUtil::SocketProcessor(ws, response,
[&session](const std::vector<char>& payload)
{
@@ -762,6 +782,16 @@ public:
Log::error("PrisonerRequestHandler::handleRequest: Unexpected
exception");
}
+ if (!jailId.empty())
+ {
+ std::ostringstream message;
+ message << "rmview" << " "
+ << jailId << " "
+ << sessionId << " "
+ << "\n";
+ Admin::instance().update(message.str());
+ }
+
Log::debug("Thread [" + thread_name + "] finished.");
}
};
@@ -769,9 +799,8 @@ public:
class ClientRequestHandlerFactory: public HTTPRequestHandlerFactory
{
public:
- ClientRequestHandlerFactory(Admin& admin, FileServer& fileServer)
- : _admin(admin),
- _fileServer(fileServer)
+ ClientRequestHandlerFactory(FileServer& fileServer)
+ : _fileServer(fileServer)
{ }
HTTPRequestHandler* createRequestHandler(const HTTPServerRequest& request)
override
@@ -806,7 +835,7 @@ public:
// Admin WebSocket Connections
else if (reqPathSegs.size() >= 1 && reqPathSegs[0] == "adminws")
{
- requestHandler = _admin.createRequestHandler();
+ requestHandler = Admin::createRequestHandler();
}
// Client post and websocket connections
else
@@ -818,7 +847,6 @@ public:
}
private:
- Admin& _admin;
FileServer& _fileServer;
};
@@ -1254,7 +1282,7 @@ int LOOLWSD::main(const std::vector<std::string>&
/*args*/)
}
// Init the Admin manager
- Admin admin(brokerPid, notifyPipe);
+ Admin::instance().setBrokerPid(brokerPid);
// Init the file server
FileServer fileServer;
@@ -1274,7 +1302,7 @@ int LOOLWSD::main(const std::vector<std::string>&
/*args*/)
// Start a server listening on the port for clients
SecureServerSocket svs(ClientPortNumber);
ThreadPool threadPool(NumPreSpawnedChildren*6, MAX_SESSIONS * 2);
- HTTPServer srv(new ClientRequestHandlerFactory(admin, fileServer),
threadPool, svs, params1);
+ HTTPServer srv(new ClientRequestHandlerFactory(fileServer), threadPool,
svs, params1);
srv.start();
@@ -1291,7 +1319,7 @@ int LOOLWSD::main(const std::vector<std::string>&
/*args*/)
return Application::EXIT_SOFTWARE;
}
- threadPool.start(admin);
+ threadPool.start(Admin::instance());
TestInput input(*this, svs, srv);
Thread inputThread;
diff --git a/loolwsd/Util.cpp b/loolwsd/Util.cpp
index 5d6485d..caeab08 100644
--- a/loolwsd/Util.cpp
+++ b/loolwsd/Util.cpp
@@ -481,10 +481,9 @@ namespace Util
}
}
- unsigned getMemoryUsage(Poco::Process::PID nPid)
+ int getMemoryUsage(const Poco::Process::PID nPid)
{
//TODO: Instead of RSS, return PSS
- std::string sResponse;
const auto cmd = "ps o rss= -p " + std::to_string(nPid);
FILE* fp = popen(cmd.c_str(), "r");
if (fp == nullptr)
@@ -492,6 +491,7 @@ namespace Util
return 0;
}
+ std::string sResponse;
char cmdBuffer[1024];
while (fgets(cmdBuffer, sizeof(cmdBuffer) - 1, fp) != nullptr)
{
@@ -499,7 +499,7 @@ namespace Util
}
pclose(fp);
- unsigned nMem = 0;
+ int nMem = -1;
try
{
nMem = std::stoi(sResponse);
diff --git a/loolwsd/Util.hpp b/loolwsd/Util.hpp
index d02fa10..517724d 100644
--- a/loolwsd/Util.hpp
+++ b/loolwsd/Util.hpp
@@ -126,7 +126,7 @@ namespace Util
void requestTermination(const Poco::Process::PID& pid);
- unsigned getMemoryUsage(Poco::Process::PID nPid);
+ int getMemoryUsage(const Poco::Process::PID nPid);
std::string replace(const std::string& s, const std::string& a, const
std::string& b);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
