Skip to content

Commit

Permalink
Merge pull request #21481 from vespa-engine/balder/gc-unused-code-1
Browse files Browse the repository at this point in the history
GC unused code.
baldersheim authored Mar 1, 2022

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
2 parents 470b59d + 5c079f8 commit 6dd5ec3
Showing 8 changed files with 38 additions and 178 deletions.
4 changes: 2 additions & 2 deletions fastos/src/tests/processtest.cpp
Original file line number Diff line number Diff line change
@@ -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<numEachTime; j++)
{
FastOS_ProcessInterface *xproc =
new FastOS_Process("sort", true,
new FastOS_Process("sort",
new MyListener("STDOUT"),
new MyListener("STDERR"));

6 changes: 1 addition & 5 deletions fastos/src/vespa/fastos/process.cpp
Original file line number Diff line number Diff line change
@@ -3,15 +3,11 @@
#include "process.h"

FastOS_ProcessInterface::FastOS_ProcessInterface (const char *cmdLine,
bool pipeStdin,
FastOS_ProcessRedirectListener *stdoutListener,
FastOS_ProcessRedirectListener *stderrListener,
int bufferSize) :
FastOS_ProcessRedirectListener *stderrListener) :
_cmdLine(cmdLine),
_pipeStdin(pipeStdin),
_stdoutListener(stdoutListener),
_stderrListener(stderrListener),
_bufferSize(bufferSize),
_next(nullptr),
_prev(nullptr)
{
14 changes: 3 additions & 11 deletions fastos/src/vespa/fastos/process.h
Original file line number Diff line number Diff line change
@@ -47,19 +47,12 @@ class FastOS_ApplicationInterface;
*/
class FastOS_ProcessInterface
{
private:
FastOS_ProcessInterface(const FastOS_ProcessInterface&);
FastOS_ProcessInterface &operator=(const FastOS_ProcessInterface &);

protected:

std::string _cmdLine;
bool _pipeStdin;

FastOS_ProcessRedirectListener *_stdoutListener;
FastOS_ProcessRedirectListener *_stderrListener;

int _bufferSize;
public:
FastOS_ProcessInterface *_next, *_prev;
static FastOS_ApplicationInterface *_app;
@@ -74,17 +67,16 @@ class FastOS_ProcessInterface
* Constructor. Does not start the process, use @ref Create or
* @ref CreateWithShell to actually start the process.
* @param cmdLine Command line
* @param pipeStdin set to true in order to redirect stdin
* @param stdoutListener non-nullptr to redirect stdout
* @param stderrListener non-nullptr to redirect stderr
* @param bufferSize Size of redirect buffers
*/
FastOS_ProcessInterface (const char *cmdLine,
bool pipeStdin = false,
FastOS_ProcessRedirectListener *stdoutListener = nullptr,
FastOS_ProcessRedirectListener *stderrListener = nullptr,
int bufferSize = 65535);
FastOS_ProcessRedirectListener *stderrListener = nullptr);

FastOS_ProcessInterface(const FastOS_ProcessInterface&) = delete;
FastOS_ProcessInterface &operator=(const FastOS_ProcessInterface &) = delete;
/**
* Destructor.
* If @ref Wait has not been called yet, it is called here.
15 changes: 7 additions & 8 deletions fastos/src/vespa/fastos/unix_app.cpp
Original file line number Diff line number Diff line change
@@ -17,7 +17,7 @@


FastOS_UNIX_Application::FastOS_UNIX_Application ()
: _processStarter(nullptr),
: _processStarter(),
_ipcHelper(nullptr)
{
}
@@ -31,8 +31,8 @@ extern char **environ;

int
FastOS_UNIX_Application::GetOpt (const char *optionsString,
const char* &optionArgument,
int &optionIndex)
const char* &optionArgument,
int &optionIndex)
{
int rc = getopt(_argc, _argv, optionsString);
optionArgument = optarg;
@@ -74,7 +74,7 @@ bool FastOS_UNIX_Application::PreThreadInit ()
sigaction(SIGPIPE, &act, nullptr);

if (useProcessStarter()) {
_processStarter = new FastOS_UNIX_ProcessStarter(this);
_processStarter = std::make_unique<FastOS_UNIX_ProcessStarter>(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<std::mutex> 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::
3 changes: 2 additions & 1 deletion fastos/src/vespa/fastos/unix_app.h
Original file line number Diff line number Diff line change
@@ -11,6 +11,7 @@

#include "types.h"
#include "app.h"
#include <memory>

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<FastOS_UNIX_ProcessStarter> _processStarter;
FastOS_UNIX_IPCHelper *_ipcHelper;

protected:
149 changes: 17 additions & 132 deletions fastos/src/vespa/fastos/unix_process.cpp
Original file line number Diff line number Diff line change
@@ -7,7 +7,6 @@
#include <cstdlib>
#include <unistd.h>
#include <fcntl.h>
#include <sys/socket.h>
#include <sys/wait.h>
#ifndef __linux__
#include <signal.h>
@@ -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<pid_t>(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<FastOS_RingBuffer>(RING_BUFFER_SIZE);
if (stderrListener != nullptr)
_descriptor[TYPE_STDERR]._readBuffer.reset(new FastOS_RingBuffer(bufferSize));
_descriptor[TYPE_STDERR]._readBuffer = std::make_unique<FastOS_RingBuffer>(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();
}
21 changes: 4 additions & 17 deletions fastos/src/vespa/fastos/unix_process.h
Original file line number Diff line number Diff line change
@@ -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);
};

4 changes: 2 additions & 2 deletions vespalib/src/vespa/vespalib/util/child_process.cpp
Original file line number Diff line number Diff line change
@@ -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)

0 comments on commit 6dd5ec3

Please sign in to comment.