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 <[email protected]> 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); _______________________________________________ Libreoffice-commits mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
