[Libreoffice-commits] online.git: 2 commits - loolwsd/IoUtil.cpp loolwsd/IoUtil.hpp loolwsd/LOOLBroker.cpp
Ashod Nakashian
ashod.nakashian at collabora.co.uk
Mon Apr 4 04:03:25 UTC 2016
loolwsd/IoUtil.cpp | 53 ++++++++++++++++++++-------------------
loolwsd/IoUtil.hpp | 11 ++++++++
loolwsd/LOOLBroker.cpp | 65 ++++++++++---------------------------------------
3 files changed, 52 insertions(+), 77 deletions(-)
New commits:
commit 285026b6f602b58d294a5768258346ed7ffe38c2
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sat Apr 2 17:22:40 2016 -0400
loolwsd: removed PipeRunnable thread from Broker and merged it with main
Change-Id: Ie10f046ed230d891eb0647e409756a34a4b146b8
Reviewed-on: https://gerrit.libreoffice.org/23776
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/IoUtil.cpp b/loolwsd/IoUtil.cpp
index f47f454..a01baba 100644
--- a/loolwsd/IoUtil.cpp
+++ b/loolwsd/IoUtil.cpp
@@ -352,37 +352,38 @@ int PipeReader::readLine(std::string& line,
return 0;
}
+bool PipeReader::processOnce(std::function<bool(std::string& message)> handler,
+ std::function<bool()> stopPredicate,
+ const size_t pollTimeoutMs)
+{
+ std::string line;
+ const auto ready = readLine(line, stopPredicate, pollTimeoutMs);
+ if (ready == 0)
+ {
+ // Timeout.
+ return true;
+ }
+ else if (ready < 0)
+ {
+ Log::error("Error reading from pipe [" + _name + "].");
+ return false;
+ }
+ else if (!handler(line))
+ {
+ Log::info("Pipe [" + _name + "] handler requested to finish.");
+ return false;
+ }
+
+ return true;
+}
+
void PipeReader::process(std::function<bool(std::string& message)> handler,
std::function<bool()> stopPredicate,
const size_t pollTimeoutMs)
{
- bool stop = false;
- for (;;)
+ while (processOnce(handler, stopPredicate, pollTimeoutMs))
{
- stop = stopPredicate();
- if (stop)
- {
- Log::info("Termination flagged for pipe [" + _name + "].");
- break;
- }
-
- std::string line;
- const auto ready = readLine(line, stopPredicate, pollTimeoutMs);
- if (ready == 0)
- {
- // Timeout.
- continue;
- }
- else if (ready < 0)
- {
- Log::error("Error reading from pipe [" + _name + "].");
- continue;
- }
- else if (!handler(line))
- {
- Log::info("Pipe [" + _name + "] handler requested to finish.");
- break;
- }
+ // readLine will call stopPredicate, no need to do it again here.
}
}
diff --git a/loolwsd/IoUtil.hpp b/loolwsd/IoUtil.hpp
index 016a808..aa67f21 100644
--- a/loolwsd/IoUtil.hpp
+++ b/loolwsd/IoUtil.hpp
@@ -54,6 +54,8 @@ namespace IoUtil
{
}
+ const std::string& getName() const { return _name; }
+
/// Reads a single line from the pipe.
/// Returns 0 for timeout, <0 for error, and >0 on success.
/// On success, line will contain the read message.
@@ -61,6 +63,15 @@ namespace IoUtil
std::function<bool()> stopPredicate,
const size_t timeoutMs = POLL_TIMEOUT_MS);
+ /// Processes a single line read and invoking stopPredicate
+ /// to check for termination condition.
+ /// Intended to be called from a polling loop.
+ bool processOnce(std::function<bool(std::string& message)> handler,
+ std::function<bool()> stopPredicate,
+ const size_t pollTimeoutMs = POLL_TIMEOUT_MS);
+
+ /// Designed to be called from a dedicated thread,
+ /// blocks and processes pipe messages in a loop.
void process(std::function<bool(std::string& message)> handler,
std::function<bool()> stopPredicate,
const size_t pollTimeoutMs = POLL_TIMEOUT_MS);
diff --git a/loolwsd/LOOLBroker.cpp b/loolwsd/LOOLBroker.cpp
index 4094c22..7b82a9e 100644
--- a/loolwsd/LOOLBroker.cpp
+++ b/loolwsd/LOOLBroker.cpp
@@ -160,7 +160,7 @@ namespace
}
}
-class PipeRunnable: public Runnable
+class PipeRunnable
{
public:
PipeRunnable() :
@@ -256,22 +256,6 @@ public:
}
}
- void run() override
- {
- static const std::string thread_name = "brk_pipe_reader";
-
- if (prctl(PR_SET_NAME, reinterpret_cast<unsigned long>(thread_name.c_str()), 0, 0, 0) != 0)
- Log::error("Cannot set thread name to " + thread_name + ".");
-
- Log::debug("Thread [" + thread_name + "] started.");
-
- IoUtil::PipeReader pipeReader(FIFO_LOOLWSD, readerBroker);
- pipeReader.process([this](std::string& message) { handleInput(message); return true; },
- []() { return TerminationFlag; });
-
- Log::debug("Thread [" + thread_name + "] finished.");
- }
-
bool waitForResponse()
{
std::string response;
@@ -665,18 +649,20 @@ int main(int argc, char** argv)
}
PipeRunnable pipeHandler;
- Poco::Thread pipeThread;
-
- pipeThread.start(pipeHandler);
-
Log::info("loolbroker is ready.");
Poco::Timestamp startTime;
+ IoUtil::PipeReader pipeReader(FIFO_LOOLWSD, readerBroker);
- int childExitCode = EXIT_SUCCESS;
- unsigned timeoutCounter = 0;
while (!TerminationFlag)
{
+ if (!pipeReader.processOnce([&pipeHandler](std::string& message) { pipeHandler.handleInput(message); return true; },
+ []() { return TerminationFlag; }))
+ {
+ Log::info("Reading pipe [" + pipeReader.getName() + "] flagged for termination.");
+ break;
+ }
+
int status;
const pid_t pid = waitpid(-1, &status, WUNTRACED | WNOHANG);
if (pid > 0)
@@ -684,7 +670,6 @@ int main(int argc, char** argv)
std::lock_guard<std::mutex> lock(forkMutex);
if (WIFEXITED(status))
{
- childExitCode = Util::getChildStatus(WEXITSTATUS(status));
Log::info() << "Child process [" << pid << "] exited with code: "
<< WEXITSTATUS(status) << "." << Log::end;
@@ -693,7 +678,6 @@ int main(int argc, char** argv)
else
if (WIFSIGNALED(status))
{
- childExitCode = Util::getSignalStatus(WTERMSIG(status));
std::string fate = "died";
#ifdef WCOREDUMP
if (WCOREDUMP(status))
@@ -731,25 +715,13 @@ int main(int argc, char** argv)
Log::info("Removing jail [" + childPath.toString() + "].");
Util::removeFile(childPath, true);
}
-
- timeoutCounter = 0;
}
else if (pid < 0)
{
// No child processes
if (errno == ECHILD)
{
- if (childExitCode == EXIT_SUCCESS)
- {
- Log::info("Last child exited successfully, fork new one.");
- ++forkCounter;
- }
- else
- {
- Log::error("Error: last child exited with error code. Terminating.");
- TerminationFlag = true; //FIXME: Why?
- continue;
- }
+ ++forkCounter;
}
else
{
@@ -757,7 +729,7 @@ int main(int argc, char** argv)
}
}
- if (forkCounter > 0 && childExitCode == EXIT_SUCCESS)
+ if (forkCounter > 0)
{
std::lock_guard<std::mutex> lock(forkMutex);
@@ -800,13 +772,6 @@ int main(int argc, char** argv)
}
}
- if (timeoutCounter++ == INTERVAL_PROBES)
- {
- timeoutCounter = 0;
- childExitCode = EXIT_SUCCESS;
- sleep(MAINTENANCE_INTERVAL);
- }
-
if (doBenchmark)
break;
}
@@ -867,7 +832,6 @@ int main(int argc, char** argv)
_childProcesses.clear();
_newChildProcesses.clear();
- pipeThread.join();
close(writerNotify);
close(readerChild);
close(readerBroker);
commit ba7db5535353a7e593f471a05b66fe514159a5f1
Author: Ashod Nakashian <ashod.nakashian at collabora.co.uk>
Date: Sat Apr 2 12:20:25 2016 -0400
loolwsd: cosmetics
Change-Id: I6c0c2e088e1e428d45cc56a31e7c1f3b8966dc2b
Reviewed-on: https://gerrit.libreoffice.org/23775
Reviewed-by: Ashod Nakashian <ashnakash at gmail.com>
Tested-by: Ashod Nakashian <ashnakash at gmail.com>
diff --git a/loolwsd/LOOLBroker.cpp b/loolwsd/LOOLBroker.cpp
index ef6a616..4094c22 100644
--- a/loolwsd/LOOLBroker.cpp
+++ b/loolwsd/LOOLBroker.cpp
@@ -289,11 +289,10 @@ private:
/// Initializes LibreOfficeKit for cross-fork re-use.
static bool globalPreinit(const std::string &loTemplate)
{
- void *handle;
- LokHookPreInit* preInit;
+ const std::string libSofficeapp = loTemplate + "/program/" LIB_SOFFICEAPP;
- std::string libSofficeapp = loTemplate + "/program/" LIB_SOFFICEAPP;
std::string loadedLibrary;
+ void *handle;
if (File(libSofficeapp).exists())
{
handle = dlopen(libSofficeapp.c_str(), RTLD_GLOBAL|RTLD_NOW);
@@ -324,7 +323,7 @@ static bool globalPreinit(const std::string &loTemplate)
}
}
- preInit = (LokHookPreInit *)dlsym(handle, "lok_preinit");
+ LokHookPreInit* preInit = (LokHookPreInit *)dlsym(handle, "lok_preinit");
if (!preInit)
{
Log::warn("Note: No lok_preinit hook in " + loadedLibrary);
More information about the Libreoffice-commits
mailing list