diff --git a/fastos/src/tests/processtest.cpp b/fastos/src/tests/processtest.cpp index 6b947523364f..204b87938dec 100644 --- a/fastos/src/tests/processtest.cpp +++ b/fastos/src/tests/processtest.cpp @@ -88,7 +88,7 @@ class ProcessTest : public BaseTest { TestHeader("PollWait Test"); - FastOS_Process *xproc = new FastOS_Process("sort", true); + FastOS_Process *xproc = new FastOS_Process("sort"); if(xproc->Create()) { @@ -168,7 +168,7 @@ class ProcessTest : public BaseTest for(j=0; j(this); } } else { rc = false; @@ -107,15 +107,14 @@ void FastOS_UNIX_Application::Cleanup () if(_ipcHelper != nullptr) _ipcHelper->Exit(); - if (_processStarter != nullptr) { + if (_processStarter) { { std::unique_lock guard; if (_processListMutex) { guard = getProcessGuard(); } } - delete _processStarter; - _processStarter = nullptr; + _processStarter.reset(); } FastOS_ApplicationInterface::Cleanup(); @@ -124,7 +123,7 @@ void FastOS_UNIX_Application::Cleanup () FastOS_UNIX_ProcessStarter * FastOS_UNIX_Application::GetProcessStarter () { - return _processStarter; + return _processStarter.get(); } void FastOS_UNIX_Application:: diff --git a/fastos/src/vespa/fastos/unix_app.h b/fastos/src/vespa/fastos/unix_app.h index 0c51b268299d..5bb6f14ad26a 100644 --- a/fastos/src/vespa/fastos/unix_app.h +++ b/fastos/src/vespa/fastos/unix_app.h @@ -11,6 +11,7 @@ #include "types.h" #include "app.h" +#include class FastOS_UNIX_ProcessStarter; class FastOS_UNIX_IPCHelper; @@ -25,7 +26,7 @@ class FastOS_UNIX_Application : public FastOS_ApplicationInterface FastOS_UNIX_Application(const FastOS_UNIX_Application&); FastOS_UNIX_Application& operator=(const FastOS_UNIX_Application&); - FastOS_UNIX_ProcessStarter *_processStarter; + std::unique_ptr _processStarter; FastOS_UNIX_IPCHelper *_ipcHelper; protected: diff --git a/fastos/src/vespa/fastos/unix_process.cpp b/fastos/src/vespa/fastos/unix_process.cpp index 7536ac74477c..cd29108c3d6b 100644 --- a/fastos/src/vespa/fastos/unix_process.cpp +++ b/fastos/src/vespa/fastos/unix_process.cpp @@ -7,7 +7,6 @@ #include #include #include -#include #include #ifndef __linux__ #include @@ -63,7 +62,6 @@ class FastOS_UNIX_RealProcess private: pid_t _pid; - bool _died; bool _terse; // Set if using direct fork (bypassing proxy process) int _streamMask; @@ -84,10 +82,6 @@ class FastOS_UNIX_RealProcess public: void SetRunDir(const char * runDir) { _runDir = runDir; } - int GetStdinDescriptor() const { return _stdinDes[1]; } - int GetStdoutDescriptor() const { return _stdoutDes[0]; } - int GetStderrDescriptor() const { return _stderrDes[0]; } - int HandoverStdinDescriptor() { int ret = _stdinDes[1]; _stdinDes[1] = -1; @@ -106,10 +100,6 @@ class FastOS_UNIX_RealProcess return ret; } - void CloseStdinDescriptor(); - void CloseStdoutDescriptor(); - void CloseStderrDescriptor(); - FastOS_UNIX_RealProcess *_prev, *_next; FastOS_UNIX_RealProcess (int streamMask); @@ -164,11 +154,9 @@ class FastOS_UNIX_RealProcess bool ForkAndExec(const char *command, char **environmentVariables, - FastOS_UNIX_Process *process, - FastOS_UNIX_ProcessStarter *processStarter); + FastOS_UNIX_Process *process); bool Setup(); - pid_t GetProcessId() const { return _pid; } void SetTerse() { _terse = true; } ssize_t HandshakeRead(void *buf, size_t len); void HandshakeWrite(int val); @@ -206,30 +194,8 @@ FastOS_UNIX_RealProcess::CloseDescriptors() } -void -FastOS_UNIX_RealProcess::CloseStdinDescriptor() -{ - CloseAndResetDescriptor(&_stdinDes[1]); -} - - -void -FastOS_UNIX_RealProcess::CloseStdoutDescriptor() -{ - CloseAndResetDescriptor(&_stdoutDes[0]); -} - - -void -FastOS_UNIX_RealProcess::CloseStderrDescriptor() -{ - CloseAndResetDescriptor(&_stderrDes[0]); -} - - FastOS_UNIX_RealProcess::FastOS_UNIX_RealProcess(int streamMask) : _pid(-1), - _died(false), _terse(false), _streamMask(streamMask), _runDir(), @@ -426,8 +392,7 @@ bool FastOS_UNIX_RealProcess:: ForkAndExec(const char *command, char **environmentVariables, - FastOS_UNIX_Process *process, - FastOS_UNIX_ProcessStarter *processStarter) + FastOS_UNIX_Process *process) { bool rc = false; int numArguments = 0; @@ -441,8 +406,7 @@ ForkAndExec(const char *command, for(int i=0; ; i++) { int length; - const char *arg = NextArgument(nextArg, &nextArg, - &length); + const char *arg = NextArgument(nextArg, &nextArg, &length); if (arg == nullptr) { // printf("ARG nullptr\n"); @@ -458,13 +422,7 @@ ForkAndExec(const char *command, PrepareExecVPE(execArgs[0]); } } - if (process == nullptr) { - processStarter->CloseProxyDescs(IsStdinPiped() ? _stdinDes[0] : -1, - IsStdoutPiped() ? _stdoutDes[1] : -1, - IsStderrPiped() ? _stderrDes[1] : -1, - _handshakeDes[0], - _handshakeDes[1]); - } + _pid = safe_fork(); if (_pid == static_cast(0)) { // Fork success, child side. @@ -511,8 +469,6 @@ ForkAndExec(const char *command, if (fd != _handshakeDes[1]) CloseDescriptor(fd); } - } else { - processStarter->CloseProxiedChildDescs(); } if (fcntl(_handshakeDes[1], F_SETFD, FD_CLOEXEC) != 0) _exit(127); @@ -728,12 +684,10 @@ FastOS_UNIX_RealProcess::Setup() FastOS_UNIX_Process:: -FastOS_UNIX_Process (const char *cmdLine, bool pipeStdin, +FastOS_UNIX_Process (const char *cmdLine, FastOS_ProcessRedirectListener *stdoutListener, - FastOS_ProcessRedirectListener *stderrListener, - int bufferSize) : - FastOS_ProcessInterface(cmdLine, pipeStdin, stdoutListener, - stderrListener, bufferSize), + FastOS_ProcessRedirectListener *stderrListener) : + FastOS_ProcessInterface(cmdLine, stdoutListener, stderrListener), _pid(0), _died(false), _returnCode(-1), @@ -744,10 +698,11 @@ FastOS_UNIX_Process (const char *cmdLine, bool pipeStdin, _killed(false), _closing(nullptr) { + constexpr uint32_t RING_BUFFER_SIZE = 0x10000; if (stdoutListener != nullptr) - _descriptor[TYPE_STDOUT]._readBuffer.reset(new FastOS_RingBuffer(bufferSize)); + _descriptor[TYPE_STDOUT]._readBuffer = std::make_unique(RING_BUFFER_SIZE); if (stderrListener != nullptr) - _descriptor[TYPE_STDERR]._readBuffer.reset(new FastOS_RingBuffer(bufferSize)); + _descriptor[TYPE_STDERR]._readBuffer = std::make_unique(RING_BUFFER_SIZE); { auto guard = _app->getProcessGuard(); @@ -786,7 +741,6 @@ FastOS_UNIX_Process::~FastOS_UNIX_Process () bool FastOS_UNIX_Process::CreateInternal (bool useShell) { return GetProcessStarter()->CreateProcess(this, useShell, - _pipeStdin, _stdoutListener != nullptr, _stderrListener != nullptr); } @@ -866,9 +820,7 @@ bool FastOS_UNIX_Process::PollWait (int *returnCode, bool *stillRunning) int FastOS_UNIX_Process::BuildStreamMask (bool useShell) { - int streamMask = 0; - - if (_pipeStdin) streamMask |= FastOS_UNIX_RealProcess::STREAM_STDIN; + int streamMask = FastOS_UNIX_RealProcess::STREAM_STDIN; if (_stdoutListener) streamMask |= FastOS_UNIX_RealProcess::STREAM_STDOUT; if (_stderrListener) streamMask |= FastOS_UNIX_RealProcess::STREAM_STDERR; if (useShell) streamMask |= FastOS_UNIX_RealProcess::EXEC_SHELL; @@ -877,83 +829,17 @@ int FastOS_UNIX_Process::BuildStreamMask (bool useShell) } -void FastOS_UNIX_ProcessStarter:: -AddChildProcess (FastOS_UNIX_RealProcess *node) -{ - node->_prev = nullptr; - node->_next = _processList; - - if (_processList != nullptr) - _processList->_prev = node; - _processList = node; -} - -void FastOS_UNIX_ProcessStarter:: -RemoveChildProcess (FastOS_UNIX_RealProcess *node) -{ - if (node->_prev) - node->_prev->_next = node->_next; - else - _processList = node->_next; - - if (node->_next) { - node->_next->_prev = node->_prev; - node->_next = nullptr; - } - - if (node->_prev != nullptr) - node->_prev = nullptr; -} - - FastOS_UNIX_ProcessStarter::FastOS_UNIX_ProcessStarter (FastOS_ApplicationInterface *app) : _app(app), - _processList(nullptr), - _pid(-1), - _closedProxyProcessFiles(false), _hasDirectChildren(false) { } -FastOS_UNIX_ProcessStarter::~FastOS_UNIX_ProcessStarter () -{ -} - - -void -FastOS_UNIX_ProcessStarter::CloseProxiedChildDescs() -{ -} - - -void -FastOS_UNIX_ProcessStarter::CloseProxyDescs(int stdinPipedDes, int stdoutPipedDes, int stderrPipedDes, - int handshakeDes0, int handshakeDes1) -{ - return; - if (_closedProxyProcessFiles) - return; - int fdlimit = sysconf(_SC_OPEN_MAX); - for(int fd = STDERR_FILENO + 1; fd < fdlimit; fd++) - { - if (fd != stdinPipedDes && - fd != stdoutPipedDes && - fd != stderrPipedDes && - fd != handshakeDes0 && - fd != handshakeDes1) - close(fd); - } - _closedProxyProcessFiles = true; -} - +FastOS_UNIX_ProcessStarter::~FastOS_UNIX_ProcessStarter () = default; bool FastOS_UNIX_ProcessStarter:: -CreateProcess (FastOS_UNIX_Process *process, - bool useShell, - bool pipeStdin, - bool pipeStdout, - bool pipeStderr) +CreateProcess (FastOS_UNIX_Process *process, bool useShell, bool pipeStdout, bool pipeStderr) { bool rc = false; @@ -977,14 +863,13 @@ CreateProcess (FastOS_UNIX_Process *process, } rprocess->SetTerse(); rprocess->Setup(); - if (pipeStdin) - process->SetDescriptor(FastOS_UNIX_Process::TYPE_STDIN, rprocess->HandoverStdinDescriptor()); + process->SetDescriptor(FastOS_UNIX_Process::TYPE_STDIN, rprocess->HandoverStdinDescriptor()); if (pipeStdout) process->SetDescriptor(FastOS_UNIX_Process::TYPE_STDOUT, rprocess->HandoverStdoutDescriptor()); if (pipeStderr) process->SetDescriptor(FastOS_UNIX_Process::TYPE_STDERR, rprocess->HandoverStderrDescriptor()); pid_t processId = -1; - if (rprocess->ForkAndExec(cmdLine, environ, process, this)) { + if (rprocess->ForkAndExec(cmdLine, environ, process)) { processId = rprocess->GetProcessID(); } if (processId != -1) { @@ -1093,8 +978,8 @@ FastOS_UNIX_Process::DescriptorHandle::CloseHandle() close(_fd); _fd = -1; } - if (_readBuffer.get() != nullptr) + if (_readBuffer) _readBuffer->Close(); - if (_writeBuffer.get() != nullptr) + if (_writeBuffer) _writeBuffer->Close(); } diff --git a/fastos/src/vespa/fastos/unix_process.h b/fastos/src/vespa/fastos/unix_process.h index a86474eccf70..43b4388e0c34 100644 --- a/fastos/src/vespa/fastos/unix_process.h +++ b/fastos/src/vespa/fastos/unix_process.h @@ -103,10 +103,9 @@ class FastOS_UNIX_Process : public FastOS_ProcessInterface } } - FastOS_UNIX_Process (const char *cmdLine, bool pipeStdin = false, + FastOS_UNIX_Process (const char *cmdLine, FastOS_ProcessRedirectListener *stdoutListener = nullptr, - FastOS_ProcessRedirectListener *stderrListener = nullptr, - int bufferSize = 65535); + FastOS_ProcessRedirectListener *stderrListener = nullptr); ~FastOS_UNIX_Process (); bool CreateInternal (bool useShell); bool Create () override { return CreateInternal(false); } @@ -140,7 +139,7 @@ class FastOS_UNIX_Process : public FastOS_ProcessInterface return _descriptor[type]; } - bool GetKillFlag () {return _killed; } + bool GetKillFlag () { return _killed; } const char *GetRunDir() const { return _runDir.c_str(); } const char *GetStdoutRedirName() const { return _stdoutRedirName.c_str(); } @@ -157,28 +156,16 @@ class FastOS_UNIX_ProcessStarter protected: FastOS_ApplicationInterface *_app; - - FastOS_UNIX_RealProcess *_processList; - - pid_t _pid; - bool _closedProxyProcessFiles; bool _hasDirectChildren; - void AddChildProcess (FastOS_UNIX_RealProcess *node); - void RemoveChildProcess (FastOS_UNIX_RealProcess *node); - void PollReapDirectChildren(); public: FastOS_UNIX_ProcessStarter (FastOS_ApplicationInterface *app); ~FastOS_UNIX_ProcessStarter (); - void CloseProxiedChildDescs(); - void CloseProxyDescs(int stdinPipedDes, int stdoutPipedDes, int stderrPipedDes, - int handshakeDes0, int handshakeDes1); - bool CreateProcess (FastOS_UNIX_Process *process, bool useShell, - bool pipeStdin, bool pipeStdout, bool pipeStderr); + bool pipeStdout, bool pipeStderr); bool Wait (FastOS_UNIX_Process *process, int timeOutSeconds, bool *pollStillRunning); }; diff --git a/vespalib/src/vespa/vespalib/util/child_process.cpp b/vespalib/src/vespa/vespalib/util/child_process.cpp index 694bed60f1b9..93db56cdf67d 100644 --- a/vespalib/src/vespa/vespalib/util/child_process.cpp +++ b/vespalib/src/vespa/vespalib/util/child_process.cpp @@ -207,7 +207,7 @@ ChildProcess::checkProc() ChildProcess::ChildProcess(const char *cmd) : _reader(1), - _proc(cmd, true, &_reader), + _proc(cmd, &_reader), _running(false), _failed(false), _exitCode(-918273645) @@ -218,7 +218,7 @@ ChildProcess::ChildProcess(const char *cmd) ChildProcess::ChildProcess(const char *cmd, capture_stderr_tag) : _reader(2), - _proc(cmd, true, &_reader, &_reader), + _proc(cmd, &_reader, &_reader), _running(false), _failed(false), _exitCode(-918273645)