loolwsd/Common.hpp | 2 --
loolwsd/LOOLForKit.cpp | 21 +++------------------
loolwsd/LOOLWSD.cpp | 33 +++++++--------------------------
3 files changed, 10 insertions(+), 46 deletions(-)
New commits:
commit 58f78f8734b908c738eab8ec6a7eccaebd5ba2ee
Author: Tor Lillqvist <t...@collabora.com>
Date: Wed Oct 12 18:15:13 2016 +0300
Don't use named pipes (FIFOs)
Use a plain pipe, as created by Poco::Process::launch() when you pass
a non-nullptr inPipe pointer to it. Leads to simpler code. No need to
explicitly create a FIFO and open it. In loolforkit, the read fd of
the pipe is now always 0 (stdin).
Creating the pipe ourselves in loolwsd using pipe() and passing the fd
of the read end of the pipe on the loolforkit command-line would not
be much more complex, and was what I first tried. But it did not work,
as Poco::ProcessImpl::launchByForkExecImpl() closes all file
descriptors >= 3. Which is a bit rude, and not documented, but there
you go.
diff --git a/loolwsd/Common.hpp b/loolwsd/Common.hpp
index 3e5d5e9..68530e8 100644
--- a/loolwsd/Common.hpp
+++ b/loolwsd/Common.hpp
@@ -32,8 +32,6 @@ constexpr int READ_BUFFER_SIZE = 2048;
/// size are considered small messages.
constexpr int SMALL_MESSAGE_SIZE = READ_BUFFER_SIZE / 2;
-constexpr auto FIFO_LOOLWSD = "loolwsdfifo";
-constexpr auto FIFO_PATH = "pipe";
constexpr auto JAILED_DOCUMENT_ROOT = "/user/docs/";
constexpr auto CHILD_URI = "/loolws/child?";
constexpr auto NEW_CHILD_URI = "/loolws/newchild?";
diff --git a/loolwsd/LOOLForKit.cpp b/loolwsd/LOOLForKit.cpp
index 2122444..c27e001 100644
--- a/loolwsd/LOOLForKit.cpp
+++ b/loolwsd/LOOLForKit.cpp
@@ -55,8 +55,6 @@ static std::map<Process::PID, std::string> childJails;
int ClientPortNumber = DEFAULT_CLIENT_PORT_NUMBER;
int MasterPortNumber = DEFAULT_MASTER_PORT_NUMBER;
-static int pipeFd = -1;
-
/// Dispatcher class to demultiplex requests from
/// WSD and handles them.
class CommandDispatcher : public IoUtil::PipeReader
@@ -209,9 +207,8 @@ static int createLibreOfficeKit(const std::string&
childRoot,
Process::PID pid;
if (!(pid = fork()))
{
- // quicker than a generic socket closing approach.
- // (but pipeFd is a pipe, not a socket...?)
- close(pipeFd);
+ // Close the pipe from loolwsd
+ close(0);
UnitKit::get().postFork();
@@ -366,16 +363,6 @@ int main(int argc, char** argv)
if (!std::getenv("LD_BIND_NOW"))
Log::info("Note: LD_BIND_NOW is not set.");
- // Open read fifo pipe with WSD.
- const Path pipePath = Path::forDirectory(childRoot + "/" + FIFO_PATH);
- const std::string pipeLoolwsd = Path(pipePath, FIFO_LOOLWSD).toString();
- if ( (pipeFd = open(pipeLoolwsd.c_str(), O_RDONLY) ) < 0 )
- {
- Log::syserror("Failed to open pipe [" + pipeLoolwsd + "] for reading.
Exiting.");
- std::_Exit(Application::EXIT_SOFTWARE);
- }
- Log::debug("open(" + pipeLoolwsd + ", RDONLY) = " +
std::to_string(pipeFd));
-
if (!haveCorrectCapabilities())
return Application::EXIT_SOFTWARE;
@@ -393,7 +380,7 @@ int main(int argc, char** argv)
std::_Exit(Application::EXIT_SOFTWARE);
}
- CommandDispatcher commandDispatcher(pipeFd);
+ CommandDispatcher commandDispatcher(0);
Log::info("ForKit process is ready.");
while (!TerminationFlag)
@@ -434,8 +421,6 @@ int main(int argc, char** argv)
}
}
- close(pipeFd);
-
int returnValue = Application::EXIT_OK;
UnitKit::get().returnValue(returnValue);
diff --git a/loolwsd/LOOLWSD.cpp b/loolwsd/LOOLWSD.cpp
index cb5e94f..8b3cec2 100644
--- a/loolwsd/LOOLWSD.cpp
+++ b/loolwsd/LOOLWSD.cpp
@@ -78,6 +78,7 @@
#include <Poco/Net/SocketAddress.h>
#include <Poco/Net/WebSocket.h>
#include <Poco/Path.h>
+#include <Poco/Pipe.h>
#include <Poco/Process.h>
#include <Poco/SAX/InputSource.h>
#include <Poco/StreamCopier.h>
@@ -133,6 +134,7 @@ using Poco::Net::ServerSocket;
using Poco::Net::SocketAddress;
using Poco::Net::WebSocket;
using Poco::Path;
+using Poco::Pipe;
using Poco::Process;
using Poco::ProcessHandle;
using Poco::StreamCopier;
@@ -1848,7 +1850,11 @@ Process::PID LOOLWSD::createForKit()
Poco::cat(std::string(" "), args.begin(), args.end()));
lastForkRequestTime = std::chrono::steady_clock::now();
- ProcessHandle child = Process::launch(forKitPath, args);
+ Pipe inPipe;
+ ProcessHandle child = Process::launch(forKitPath, args, &inPipe, nullptr,
nullptr);
+
+ // The Pipe dtor closes the fd, so dup it.
+ ForKitWritePipe = dup(inPipe.writeHandle());
return child.id();
}
@@ -1906,23 +1912,6 @@ int LOOLWSD::main(const std::vector<std::string>&
/*args*/)
if (ClientPortNumber == MasterPortNumber)
throw IncompatibleOptionsException("port");
- // Create the directory where the fifo pipe with ForKit will be.
- const Path pipePath = Path::forDirectory(ChildRoot + "/" + FIFO_PATH);
- if (!File(pipePath).exists() && !File(pipePath).createDirectory())
- {
- Log::error("Failed to create pipe directory [" + pipePath.toString() +
"].");
- return Application::EXIT_SOFTWARE;
- }
-
- // Create the fifo with ForKit.
- const std::string pipeLoolwsd = Path(pipePath, FIFO_LOOLWSD).toString();
- Log::debug("mkfifo(" + pipeLoolwsd + ")");
- if (mkfifo(pipeLoolwsd.c_str(), 0666) < 0 && errno != EEXIST)
- {
- Log::syserror("Failed to create fifo [" + pipeLoolwsd + "].");
- return Application::EXIT_SOFTWARE;
- }
-
// Configure the Server.
// Note: TCPServer internally uses a ThreadPool to
// dispatch connections (the default if not given).
@@ -1960,14 +1949,6 @@ int LOOLWSD::main(const std::vector<std::string>&
/*args*/)
return Application::EXIT_SOFTWARE;
}
- // Open write fifo pipe with ForKit.
- if ( (ForKitWritePipe = open(pipeLoolwsd.c_str(), O_WRONLY) ) < 0 )
- {
- Log::syserror("Failed to open pipe [" + pipeLoolwsd + "] for
writing.");
- return Application::EXIT_SOFTWARE;
- }
- Log::debug("open(" + pipeLoolwsd + ", WRONLY) = " +
std::to_string(ForKitWritePipe));
-
// Init the Admin manager
Admin::instance().setForKitPid(forKitPid);
_______________________________________________
Libreoffice-commits mailing list
libreoffice-comm...@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/libreoffice-commits
