[Libreoffice-commits] online.git: common/Util.cpp common/Util.hpp wsd/DocumentBroker.cpp wsd/LOOLWSD.cpp
Michael Meeks
michael.meeks at collabora.com
Mon Jan 29 15:26:45 UTC 2018
common/Util.cpp | 92 +++++++++++++++++++++++++++++++++++++++++++++++++
common/Util.hpp | 5 ++
wsd/DocumentBroker.cpp | 8 +++-
wsd/LOOLWSD.cpp | 15 ++-----
4 files changed, 108 insertions(+), 12 deletions(-)
New commits:
commit d3c17510ed8b94a0d9d3af1aaaf88959432263f0
Author: Michael Meeks <michael.meeks at collabora.com>
Date: Mon Jan 29 15:13:54 2018 +0000
Implement an improved fork/exec wrapper.
* logs helpful messages for various error corner-cases.
* optimized file descriptor closing for large fd counts.
Change-Id: I8cba9ecb3d71ddc6e22e20d89368d8c6b9b5097f
diff --git a/common/Util.cpp b/common/Util.cpp
index f8c493b9..d42fcd62 100644
--- a/common/Util.cpp
+++ b/common/Util.cpp
@@ -18,7 +18,9 @@
#include <sys/stat.h>
#include <sys/uio.h>
#include <sys/vfs.h>
+#include <sys/types.h>
#include <unistd.h>
+#include <dirent.h>
#include <atomic>
#include <cassert>
@@ -114,6 +116,96 @@ namespace Util
}
}
+ // close what we have - far faster than going up to a 1m open_max eg.
+ static bool closeFdsFromProc()
+ {
+ DIR *fdDir = opendir("/proc/self/fd");
+ if (!fdDir)
+ return false;
+
+ struct dirent *i;
+
+ while ((i = readdir(fdDir))) {
+ if (i->d_name[0] == '.')
+ continue;
+
+ char *e = NULL;
+ errno = 0;
+ long fd = strtol(i->d_name, &e, 10);
+ if (errno != 0 || !e || *e)
+ continue;
+
+ if (fd == dirfd(fdDir))
+ continue;
+
+ if (fd < 3)
+ continue;
+
+ if (close(fd) < 0)
+ LOG_WRN("Unexpected failure to close fd " << fd);
+ else
+ LOG_TRC("Closed fd " << fd);
+ }
+
+ closedir(fdDir);
+ return true;
+ }
+
+ static void closeFds()
+ {
+ if (!closeFdsFromProc())
+ {
+ LOG_WRN("Couldn't close fds efficiently from /proc");
+ for (int fd = 3; fd < sysconf(_SC_OPEN_MAX); ++fd)
+ close(fd);
+ }
+ }
+
+ int spawnProcess(const std::string &cmd, const std::vector<std::string> &args, int *stdInput)
+ {
+ int pipeFds[2] = { -1, -1 };
+ if (stdInput)
+ {
+ if (pipe(pipeFds) < 0)
+ {
+ LOG_ERR("Out of file descriptors spawning " << cmd);
+ throw Poco::SystemException("Out of file descriptors");
+ }
+ }
+
+ std::vector<char *> params;
+ params.push_back(const_cast<char *>(cmd.c_str()));
+ for (auto i : args)
+ params.push_back(const_cast<char *>(i.c_str()));
+ params.push_back(nullptr);
+
+ int pid = fork();
+ if (pid < 0)
+ {
+ LOG_ERR("Failed to fork for command '" << cmd);
+ throw Poco::SystemException("Failed to fork for command ", cmd);
+ }
+ else if (pid == 0) // child
+ {
+ if (stdInput)
+ dup2(pipeFds[0], STDIN_FILENO);
+
+ closeFds();
+
+ int ret = execvp(params[0], ¶ms[0]);
+ if (ret < 0)
+ std::cerr << "Failed to exec command '" << cmd << "' with error '" << strerror(errno) << "'\n";
+ _exit(42);
+ }
+ // else spawning process still
+ if (stdInput)
+ {
+ close(pipeFds[0]);
+ *stdInput = pipeFds[1];
+ }
+ return pid;
+ }
+
bool dataFromHexString(const std::string& hexString, std::vector<unsigned char>& data)
{
if (hexString.length() % 2 != 0)
diff --git a/common/Util.hpp b/common/Util.hpp
index 5118efb6..a51e402e 100644
--- a/common/Util.hpp
+++ b/common/Util.hpp
@@ -44,6 +44,11 @@ namespace Util
std::string getFilename(const size_t length);
}
+ /// Spawn a process if stdInput is non-NULL it contains a writable descriptor
+ /// to send data to the child.
+ int spawnProcess(const std::string &cmd, const std::vector<std::string> &args,
+ int *stdInput = nullptr);
+
/// Hex to unsigned char
bool dataFromHexString(const std::string& hexString, std::vector<unsigned char>& data);
/// Encode an integral ID into a string, with padding support.
diff --git a/wsd/DocumentBroker.cpp b/wsd/DocumentBroker.cpp
index a5f89158..c499b6e4 100644
--- a/wsd/DocumentBroker.cpp
+++ b/wsd/DocumentBroker.cpp
@@ -38,6 +38,9 @@
#include <common/Protocol.hpp>
#include <common/Unit.hpp>
+#include <sys/types.h>
+#include <sys/wait.h>
+
using namespace LOOLProtocol;
using Poco::JSON::Object;
@@ -621,8 +624,9 @@ bool DocumentBroker::load(const std::shared_ptr<ClientSession>& session, const s
for (std::size_t i = 1; i < tokenizer.count(); ++i)
args.emplace_back(tokenizer[i]);
- Poco::ProcessHandle process = Poco::Process::launch(tokenizer[0], args);
- const int rc = process.wait();
+ int process = Util::spawnProcess(tokenizer[0], args);
+ int status = -1;
+ const int rc = ::waitpid(process, &status, 0);
if (rc != 0)
{
LOG_ERR("Conversion from " << extension << " to " << newExtension << " failed (" << rc << ").");
diff --git a/wsd/LOOLWSD.cpp b/wsd/LOOLWSD.cpp
index f96005ef..eecb793f 100644
--- a/wsd/LOOLWSD.cpp
+++ b/wsd/LOOLWSD.cpp
@@ -140,9 +140,6 @@ using Poco::Path;
using Poco::Pipe;
#endif
using Poco::Process;
-#ifndef KIT_IN_PROCESS
-using Poco::ProcessHandle;
-#endif
using Poco::StreamCopier;
using Poco::StringTokenizer;
using Poco::TemporaryFile;
@@ -1320,7 +1317,7 @@ bool LOOLWSD::createForKit()
std::unique_lock<std::mutex> newChildrenLock(NewChildrenMutex);
- Process::Args args;
+ std::vector<std::string> args;
args.push_back("--losubpath=" + std::string(LO_JAIL_SUBPATH));
args.push_back("--systemplate=" + SysTemplate);
args.push_back("--lotemplate=" + LoTemplate);
@@ -1376,13 +1373,11 @@ bool LOOLWSD::createForKit()
Poco::cat(std::string(" "), args.begin(), args.end()));
LastForkRequestTime = std::chrono::steady_clock::now();
- Pipe inPipe;
- ProcessHandle child = Process::launch(forKitPath, args, &inPipe, nullptr, nullptr);
-
- // The Pipe dtor closes the fd, so dup it.
- ForKitWritePipe = dup(inPipe.writeHandle());
+ int childStdin = -1;
+ int child = Util::spawnProcess(forKitPath, args, &childStdin);
- ForKitProcId = child.id();
+ ForKitWritePipe = childStdin;
+ ForKitProcId = child;
LOG_INF("Forkit process launched: " << ForKitProcId);
More information about the Libreoffice-commits
mailing list