loolwsd/LOOLBroker.cpp | 115 ++++++++++++----------------------------
loolwsd/LOOLKit.cpp | 83 ----------------------------
loolwsd/Makefile.am | 18 ++----
loolwsd/README.vars | 19 ++----
loolwsd/debian/loolwsd.postinst | 1
loolwsd/loolwsd.spec.in | 2
6 files changed, 50 insertions(+), 188 deletions(-)
New commits:
commit a132e06409353b374e754a9aecf6b55b06778118
Author: Tor Lillqvist <t...@collabora.com>
Date: Mon Apr 4 08:26:05 2016 +0300
Bin the non-preinit and non-fork code paths
Preiniting LibreOfficeKit and forking kit processes (instead of
spawning) has worked fine for a while, and has been the default way
this works.
No 'loolkit' program gets built any more.
diff --git a/loolwsd/LOOLBroker.cpp b/loolwsd/LOOLBroker.cpp
index c37116a..defcfae 100644
--- a/loolwsd/LOOLBroker.cpp
+++ b/loolwsd/LOOLBroker.cpp
@@ -21,7 +21,6 @@
// we can avoid execve and share lots of memory here. We
// can't link to a non-PIC translation unit though, so
// include to share.
-#define LOOLKIT_NO_MAIN 1
#include "LOOLKit.cpp"
#define LIB_SOFFICEAPP "lib" "sofficeapp" ".so"
@@ -34,7 +33,6 @@ const std::string BROKER_PREFIX = "lokit";
static int ReaderBroker = -1;
-static std::string LoolkitPath;
static std::atomic<unsigned> ForkCounter;
static unsigned int ChildCounter = 0;
static int NumPreSpawnedChildren = 1;
@@ -74,7 +72,7 @@ private:
};
/// Initializes LibreOfficeKit for cross-fork re-use.
-static bool globalPreinit(const std::string &loTemplate)
+static void globalPreinit(const std::string &loTemplate)
{
const std::string libSofficeapp = loTemplate + "/program/" LIB_SOFFICEAPP;
@@ -85,8 +83,8 @@ static bool globalPreinit(const std::string &loTemplate)
handle = dlopen(libSofficeapp.c_str(), RTLD_GLOBAL|RTLD_NOW);
if (!handle)
{
- Log::warn("Failed to load " + libSofficeapp + ": " +
std::string(dlerror()));
- return false;
+ Log::error("Failed to load " + libSofficeapp + ": " +
std::string(dlerror()));
+ _exit(Application::EXIT_SOFTWARE);
}
loadedLibrary = libSofficeapp;
}
@@ -98,30 +96,33 @@ static bool globalPreinit(const std::string &loTemplate)
handle = dlopen(libMerged.c_str(), RTLD_GLOBAL|RTLD_NOW);
if (!handle)
{
- Log::warn("Failed to load " + libMerged + ": " +
std::string(dlerror()));
- return false;
+ Log::error("Failed to load " + libMerged + ": " +
std::string(dlerror()));
+ _exit(Application::EXIT_SOFTWARE);
}
loadedLibrary = libMerged;
}
else
{
- Log::warn("Neither " + libSofficeapp + " or " + libMerged + "
exist.");
- return false;
+ Log::error("Neither " + libSofficeapp + " or " + libMerged + "
exist.");
+ _exit(Application::EXIT_SOFTWARE);
}
}
LokHookPreInit* preInit = (LokHookPreInit *)dlsym(handle, "lok_preinit");
if (!preInit)
{
- Log::warn("Note: No lok_preinit hook in " + loadedLibrary);
- return false;
+ Log::error("No lok_preinit symbol in " + loadedLibrary);
+ _exit(Application::EXIT_SOFTWARE);
}
- return preInit((loTemplate + "/program").c_str(), "file:///user") == 0;
+ if (preInit((loTemplate + "/program").c_str(), "file:///user") != 0)
+ {
+ Log::error("lok_preinit() in " + loadedLibrary + " failed");
+ _exit(Application::EXIT_SOFTWARE);
+ }
}
-static int createLibreOfficeKit(const bool sharePages,
- const std::string& childRoot,
+static int createLibreOfficeKit(const std::string& childRoot,
const std::string& sysTemplate,
const std::string& loTemplate,
const std::string& loSubPath)
@@ -137,56 +138,28 @@ static int createLibreOfficeKit(const bool sharePages,
return -1;
}
- if (sharePages)
- {
- Log::debug("Forking LibreOfficeKit.");
+ Log::debug("Forking LibreOfficeKit.");
- Process::PID pid;
- if (!(pid = fork()))
- {
- // child
- if (std::getenv("SLEEPKITFORDEBUGGER"))
- {
- std::cerr << "Sleeping " << std::getenv("SLEEPKITFORDEBUGGER")
- << " seconds to give you time to attach debugger to
process "
- << Process::id() << std::endl;
- Thread::sleep(std::stoul(std::getenv("SLEEPKITFORDEBUGGER")) *
1000);
- }
-
- lokit_main(childRoot, sysTemplate, loTemplate, loSubPath, pipeKit);
- _exit(Application::EXIT_OK);
- }
- else
+ Process::PID pid;
+ if (!(pid = fork()))
+ {
+ // child
+ if (std::getenv("SLEEPKITFORDEBUGGER"))
{
- // parent
- childPID = pid; // (somehow - switch the hash to use real pids or
?) ...
- Log::info("Forked kit [" + std::to_string(childPID) + "].");
+ std::cerr << "Sleeping " << std::getenv("SLEEPKITFORDEBUGGER")
+ << " seconds to give you time to attach debugger to
process "
+ << Process::id() << std::endl;
+ Thread::sleep(std::stoul(std::getenv("SLEEPKITFORDEBUGGER")) *
1000);
}
+
+ lokit_main(childRoot, sysTemplate, loTemplate, loSubPath, pipeKit);
+ _exit(Application::EXIT_OK);
}
else
{
- Process::Args args;
- args.push_back("--childroot=" + childRoot);
- args.push_back("--systemplate=" + sysTemplate);
- args.push_back("--lotemplate=" + loTemplate);
- args.push_back("--losubpath=" + loSubPath);
- args.push_back("--pipe=" + pipeKit);
- args.push_back("--clientport=" + std::to_string(ClientPortNumber));
-
- Log::info("Launching LibreOfficeKit #" + std::to_string(ChildCounter) +
- ": " + LoolkitPath + " " +
- Poco::cat(std::string(" "), args.begin(), args.end()));
-
- Poco::ProcessHandle procChild = Process::launch(LoolkitPath, args);
- childPID = procChild.id();
- Log::info("Spawned kit [" + std::to_string(childPID) + "].");
-
- if (!Process::isRunning(procChild))
- {
- // This can happen if we fail to copy it, or bad chroot etc.
- Log::error("Error: loolkit [" + std::to_string(childPID) + "] was
stillborn.");
- return -1;
- }
+ // parent
+ childPID = pid; // (somehow - switch the hash to use real pids or ?)
...
+ Log::info("Forked kit [" + std::to_string(childPID) + "].");
}
Log::info() << "Created Kit #" << ChildCounter << ", PID: " << childPID <<
Log::end;
@@ -289,8 +262,6 @@ int main(int argc, char** argv)
}
}
- LoolkitPath = Poco::Path(argv[0]).parent().toString() + "loolkit";
-
if (loSubPath.empty() || sysTemplate.empty() ||
loTemplate.empty() || childRoot.empty() ||
NumPreSpawnedChildren < 1)
@@ -307,19 +278,13 @@ int main(int argc, char** argv)
setupPipes(childRoot);
- // Initialize LoKit and hope we can fork and save memory by sharing pages.
- const bool sharePages = std::getenv("LOK_NO_PREINIT") == nullptr
- ? globalPreinit(loTemplate)
- : std::getenv("LOK_FORK") != nullptr;
+ // Initialize LoKit
+ globalPreinit(loTemplate);
- if (!sharePages)
- Log::warn("Cannot fork, will spawn instead.");
- else
- Log::info("Preinit stage OK.");
+ Log::info("Preinit stage OK.");
// We must have at least one child, more is created dynamically.
- if (createLibreOfficeKit(sharePages, childRoot, sysTemplate,
- loTemplate, loSubPath) < 0)
+ if (createLibreOfficeKit(childRoot, sysTemplate, loTemplate, loSubPath) <
0)
{
Log::error("Error: failed to create children.");
std::exit(Application::EXIT_SOFTWARE);
@@ -328,13 +293,6 @@ int main(int argc, char** argv)
if (NumPreSpawnedChildren > 1)
ForkCounter = NumPreSpawnedChildren - 1;
- if (!sharePages)
- {
- dropCapability(CAP_SYS_CHROOT);
- dropCapability(CAP_MKNOD);
- dropCapability(CAP_FOWNER);
- }
-
ChildDispatcher childDispatcher;
Log::info("loolbroker is ready.");
@@ -357,8 +315,7 @@ int main(int argc, char** argv)
size_t newInstances = 0;
do
{
- if (createLibreOfficeKit(sharePages, childRoot, sysTemplate,
- loTemplate, loSubPath) < 0)
+ if (createLibreOfficeKit(childRoot, sysTemplate, loTemplate,
loSubPath) < 0)
{
Log::error("Error: fork failed.");
}
diff --git a/loolwsd/LOOLKit.cpp b/loolwsd/LOOLKit.cpp
index 23b6063..c45aebc 100644
--- a/loolwsd/LOOLKit.cpp
+++ b/loolwsd/LOOLKit.cpp
@@ -8,7 +8,7 @@
*/
/*
- * NB. this file is compiled both standalone, and as part of the LOOLBroker.
+ * NB. this file is compiled as part of the LOOLBroker.
*/
#include <sys/prctl.h>
@@ -855,11 +855,9 @@ void lokit_main(const std::string& childRoot,
const std::string& pipe,
bool doBenchmark = false)
{
-#ifdef LOOLKIT_NO_MAIN
// Reinitialize logging when forked.
Log::initialize("kit");
Util::rng::reseed();
-#endif
assert(!childRoot.empty());
assert(!sysTemplate.empty());
@@ -907,7 +905,6 @@ void lokit_main(const std::string& childRoot,
File(jailPath).createDirectories();
-#ifdef LOOLKIT_NO_MAIN
// Create a symlink inside the jailPath so that the absolute pathname
loTemplate, when
// interpreted inside a chroot at jailPath, points to loSubPath
(relative to the chroot).
Path symlinkSource(jailPath, Path(loTemplate.substr(1)));
@@ -925,7 +922,6 @@ void lokit_main(const std::string& childRoot,
Log::error("Error: symlink(\"" + symlinkTarget + "\",\"" +
symlinkSource.toString() + "\") failed");
throw Poco::Exception("symlink() failed");
}
-#endif
Path jailLOInstallation(jailPath, loSubPath);
jailLOInstallation.makeDirectory();
@@ -1108,81 +1104,4 @@ void lokit_main(const std::string& childRoot,
Log::info("Process [" + process_name + "] finished.");
}
-#ifndef LOOLKIT_NO_MAIN
-
-/// Simple argument parsing wrapper / helper for the above.
-int main(int argc, char** argv)
-{
- if (std::getenv("SLEEPFORDEBUGGER"))
- {
- std::cerr << "Sleeping " << std::getenv("SLEEPFORDEBUGGER")
- << " seconds to attach debugger to process "
- << Process::id() << std::endl;
- Thread::sleep(std::stoul(std::getenv("SLEEPFORDEBUGGER")) * 1000);
- }
-
- Log::initialize("kit");
-
- std::string childRoot;
- std::string sysTemplate;
- std::string loTemplate;
- std::string loSubPath;
- std::string pipe;
-
- for (int i = 1; i < argc; ++i)
- {
- char *cmd = argv[i];
- char *eq;
-
- if (std::strstr(cmd, "--childroot=") == cmd)
- {
- eq = std::strchr(cmd, '=');
- childRoot = std::string(eq+1);
- }
- else if (std::strstr(cmd, "--systemplate=") == cmd)
- {
- eq = std::strchr(cmd, '=');
- sysTemplate = std::string(eq+1);
- }
- else if (std::strstr(cmd, "--lotemplate=") == cmd)
- {
- eq = std::strchr(cmd, '=');
- loTemplate = std::string(eq+1);
- }
- else if (std::strstr(cmd, "--losubpath=") == cmd)
- {
- eq = std::strchr(cmd, '=');
- loSubPath = std::string(eq+1);
- }
- else if (std::strstr(cmd, "--pipe=") == cmd)
- {
- eq = std::strchr(cmd, '=');
- pipe = std::string(eq+1);
- }
- else if (std::strstr(cmd, "--clientport=") == cmd)
- {
- eq = std::strchr(cmd, '=');
- ClientPortNumber = std::stoll(std::string(eq+1));
- }
- }
-
- if (loSubPath.empty())
- {
- Log::error("Error: --losubpath is empty");
- std::exit(Application::EXIT_SOFTWARE);
- }
-
- if (pipe.empty())
- {
- Log::error("Error: --pipe is empty");
- std::exit(Application::EXIT_SOFTWARE);
- }
-
- lokit_main(childRoot, sysTemplate, loTemplate, loSubPath, pipe);
-
- return Application::EXIT_OK;
-}
-
-#endif
-
/* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/loolwsd/Makefile.am b/loolwsd/Makefile.am
index 7056792..4a0d753 100644
--- a/loolwsd/Makefile.am
+++ b/loolwsd/Makefile.am
@@ -1,6 +1,6 @@
SUBDIRS = test
-bin_PROGRAMS = loolwsd loolbroker loolkit loolmap loolmount
+bin_PROGRAMS = loolwsd loolbroker loolmap loolmount
dist_bin_SCRIPTS = loolwsd-systemplate-setup discovery.xml
@@ -47,9 +47,6 @@ lokitclient_SOURCES = IoUtil.cpp \
broker_shared_sources = ChildProcessSession.cpp \
$(shared_sources)
-loolkit_SOURCES = LOOLKit.cpp \
- $(broker_shared_sources)
-
loolbroker_SOURCES = LOOLBroker.cpp \
$(broker_shared_sources)
@@ -92,16 +89,15 @@ clean-cache:
# Intentionally don't use "*" below... Avoid risk of accidentally running rm
-rf /*
test -n "@LOOLWSD_CACHEDIR@" && rm -rf "@LOOLWSD_CACHEDIR@"/[0-9a-f]
-# After building loolbroker and loolkit, set their capabilities as
-# required. Do it already after a plain 'make' to allow for testing
-# without installing. When building for packaging, no need for this,
-# as the capabilities won't survive packaging anyway. Instead, handle
-# it when installing the RPM or Debian package.
+# After building loolbroker, set its capabilities as required. Do it
+# already after a plain 'make' to allow for testing without
+# installing. When building for packaging, no need for this, as the
+# capabilities won't survive packaging anyway. Instead, handle it when
+# installing the RPM or Debian package.
-all-local: loolbroker loolkit certificates
+all-local: loolbroker certificates
if test "$$BUILDING_FROM_RPMBUILD" != yes; then \
sudo @SETCAP@ cap_fowner,cap_mknod,cap_sys_chroot=ep loolbroker; \
- sudo @SETCAP@ cap_fowner,cap_mknod,cap_sys_chroot=ep loolkit; \
sudo @SETCAP@ cap_sys_admin=ep loolmount; \
echo "Set required capabilities"; \
else \
diff --git a/loolwsd/README.vars b/loolwsd/README.vars
index 59108ab..e87f3ab 100644
--- a/loolwsd/README.vars
+++ b/loolwsd/README.vars
@@ -16,18 +16,11 @@ LOOL_LOGLEVEL <level>
error, warning, notice, information, debug,
trace
-LOK_NO_PREINIT <set/unset>
- set this to disable pre-initialization of LOK instances.
-
-LOK_FORK <set/unset>
- set this to enable forking instead of execve'ing of kit
- process instances even if LOK_NO_PREINIT is set.
-
SLEEPFORDEBUGGER <seconds to sleep>
- sleep <n> seconds while launching processes in order to
- allow a 'sudo gdb' session to 'attach <pid>' to them.
+ sleep <n> seconds in the broken process after starting in
+ order to allow a 'sudo gdb' session to 'attach <pid>' to them.
-SLEEPKITFORDEBUGGER <seconds to sleep>
- sleep <n> seconds after launching (or forking) each
- kit process instance, to allow a 'sudo gdb' session
- to attach and debug that process.
+SLEEPKITFORDEBUGGER <seconds to sleep>
+ sleep <n> seconds in each kit process instance after forking,
+ to allow a 'sudo gdb' session to attach and debug that
+ process.
diff --git a/loolwsd/debian/loolwsd.postinst b/loolwsd/debian/loolwsd.postinst
index bb4f6da..d2b50ec 100755
--- a/loolwsd/debian/loolwsd.postinst
+++ b/loolwsd/debian/loolwsd.postinst
@@ -4,7 +4,6 @@ set -e
case "$1" in
configure)
- setcap cap_fowner,cap_mknod,cap_sys_chroot=ep /usr/bin/loolkit || true
setcap cap_fowner,cap_mknod,cap_sys_chroot=ep /usr/bin/loolbroker ||
true
adduser --quiet --system --group --home /opt/lool lool
diff --git a/loolwsd/loolwsd.spec.in b/loolwsd/loolwsd.spec.in
index 9d06a2f..9c0e198 100644
--- a/loolwsd/loolwsd.spec.in
+++ b/loolwsd/loolwsd.spec.in
@@ -57,7 +57,6 @@ echo "0 0 */1 * * root find /var/cache/loolwsd -name
\"*.png\" -a -atime +10 -ex
/usr/bin/loolwsd
/usr/bin/loolwsd-systemplate-setup
/usr/bin/loolmap
-/usr/bin/loolkit
/usr/bin/loolbroker
/usr/bin/discovery.xml
%{_unitdir}/loolwsd.service
@@ -71,7 +70,6 @@ echo "0 0 */1 * * root find /var/cache/loolwsd -name
\"*.png\" -a -atime +10 -ex
%post
setcap cap_fowner,cap_mknod,cap_sys_chroot=ep /usr/bin/loolbroker
-setcap cap_fowner,cap_mknod,cap_sys_chroot=ep /usr/bin/loolkit
getent group %{group} >/dev/null || groupadd -r %{group}
getent passwd %{owner} >/dev/null || useradd -g %{group} -r %{owner}
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
