From 87bd880b431965781e3e0b3024d5b6d85a2184e9 Mon Sep 17 00:00:00 2001 From: falkTX Date: Sat, 13 Jun 2020 01:22:58 +0100 Subject: [PATCH] Cleanup ChildProcess code Signed-off-by: falkTX --- source/modules/water/threads/ChildProcess.cpp | 224 ++++++------------ source/modules/water/threads/ChildProcess.h | 24 +- 2 files changed, 76 insertions(+), 172 deletions(-) diff --git a/source/modules/water/threads/ChildProcess.cpp b/source/modules/water/threads/ChildProcess.cpp index 58a2f3cb8..0a92b7dbc 100644 --- a/source/modules/water/threads/ChildProcess.cpp +++ b/source/modules/water/threads/ChildProcess.cpp @@ -3,7 +3,7 @@ This file is part of the Water library. Copyright (c) 2016 ROLI Ltd. - Copyright (C) 2017-2018 Filipe Coelho + Copyright (C) 2017-2020 Filipe Coelho Permission is granted to use this software under the terms of the ISC license http://www.isc.org/downloads/software-support-policy/isc-license/ @@ -26,7 +26,6 @@ #include "ChildProcess.h" #include "../files/File.h" #include "../misc/Time.h" -#include "../streams/MemoryOutputStream.h" #ifndef CARLA_OS_WIN # include @@ -40,30 +39,16 @@ namespace water { class ChildProcess::ActiveProcess { public: - ActiveProcess (const String& command, int streamFlags) - : ok (false), readPipe (0), writePipe (0) + ActiveProcess (const String& command) + : ok (false) { - SECURITY_ATTRIBUTES securityAtts; - carla_zeroStruct(securityAtts); - securityAtts.nLength = sizeof (securityAtts); - securityAtts.bInheritHandle = TRUE; + STARTUPINFO startupInfo; + carla_zeroStruct(startupInfo); + startupInfo.cb = sizeof (startupInfo); - if (CreatePipe (&readPipe, &writePipe, &securityAtts, 0) - && SetHandleInformation (readPipe, HANDLE_FLAG_INHERIT, 0)) - { - STARTUPINFO startupInfo; - carla_zeroStruct(startupInfo); - startupInfo.cb = sizeof (startupInfo); - - startupInfo.hStdOutput = (streamFlags & wantStdOut) != 0 ? writePipe : 0; - startupInfo.hStdError = (streamFlags & wantStdErr) != 0 ? writePipe : 0; - startupInfo.dwFlags = STARTF_USESTDHANDLES; - - ok = CreateProcess (nullptr, const_cast(command.toRawUTF8()), - nullptr, nullptr, TRUE, CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT, - nullptr, nullptr, &startupInfo, &processInfo) != FALSE; - } - } + ok = CreateProcess (nullptr, const_cast(command.toRawUTF8()), + nullptr, nullptr, TRUE, CREATE_NO_WINDOW | CREATE_UNICODE_ENVIRONMENT, + nullptr, nullptr, &startupInfo, &processInfo) != FALSE; ~ActiveProcess() { @@ -72,12 +57,6 @@ public: CloseHandle (processInfo.hThread); CloseHandle (processInfo.hProcess); } - - if (readPipe != 0) - CloseHandle (readPipe); - - if (writePipe != 0) - CloseHandle (writePipe); } bool isRunning() const noexcept @@ -85,39 +64,15 @@ public: return WaitForSingleObject (processInfo.hProcess, 0) != WAIT_OBJECT_0; } - int read (void* dest, int numNeeded) const noexcept + bool checkRunningAndUnsetPID() noexcept { - int total = 0; - - while (ok && numNeeded > 0) - { - DWORD available = 0; - - if (! PeekNamedPipe ((HANDLE) readPipe, nullptr, 0, nullptr, &available, nullptr)) - break; - - const int numToDo = jmin ((int) available, numNeeded); - - if (available == 0) - { - if (! isRunning()) - break; - - Sleep(5); - } - else - { - DWORD numRead = 0; - if (! ReadFile ((HANDLE) readPipe, dest, numToDo, &numRead, nullptr)) - break; - - total += numRead; - dest = addBytesToPointer (dest, numRead); - numNeeded -= numRead; - } - } + if (isRunning()) + return true; - return total; + ok = false; + CloseHandle (processInfo.hThread); + CloseHandle (processInfo.hProcess); + return false; } bool killProcess() const noexcept @@ -145,7 +100,6 @@ public: bool ok; private: - HANDLE readPipe, writePipe; PROCESS_INFORMATION processInfo; CARLA_DECLARE_NON_COPY_CLASS (ActiveProcess) @@ -154,8 +108,8 @@ private: class ChildProcess::ActiveProcess { public: - ActiveProcess (const StringArray& arguments, int streamFlags) - : childPID (0), pipeHandle (0), readHandle (0) + ActiveProcess (const StringArray& arguments) + : childPID (0) { String exe (arguments[0].unquoted()); @@ -164,49 +118,35 @@ public: wassert (File::getCurrentWorkingDirectory().getChildFile (exe).existsAsFile() || ! exe.containsChar (File::separator)); - int pipeHandles[2] = { 0 }; + Array argv; + for (int i = 0; i < arguments.size(); ++i) + if (arguments[i].isNotEmpty()) + argv.add (const_cast (arguments[i].toRawUTF8())); + + argv.add (nullptr); - if (pipe (pipeHandles) == 0) + const pid_t result = fork(); + + if (result < 0) + { + // error + } + else if (result == 0) { - Array argv; - for (int i = 0; i < arguments.size(); ++i) - if (arguments[i].isNotEmpty()) - argv.add (const_cast (arguments[i].toRawUTF8())); - - argv.add (nullptr); - - const pid_t result = vfork(); - - if (result < 0) - { - close (pipeHandles[0]); - close (pipeHandles[1]); - } - else if (result == 0) - { - if (execvp (exe.toRawUTF8(), argv.getRawDataPointer())) - _exit (-1); - } - else - { - // we're the parent process.. - childPID = result; - pipeHandle = pipeHandles[0]; - close (pipeHandles[1]); // close the write handle - } - - // FIXME - (void)streamFlags; + // child process + if (execvp (exe.toRawUTF8(), argv.getRawDataPointer())) + _exit (-1); + } + else + { + // we're the parent process.. + childPID = result; } } ~ActiveProcess() { - if (readHandle != 0) - fclose (readHandle); - - if (pipeHandle != 0) - close (pipeHandle); + CARLA_SAFE_ASSERT_INT(childPID == 0, childPID); } bool isRunning() const noexcept @@ -221,26 +161,33 @@ public: return false; } - int read (void* const dest, const int numBytes) noexcept + bool checkRunningAndUnsetPID() noexcept { - wassert (dest != nullptr); - - #ifdef fdopen - #error // the zlib headers define this function as NULL! - #endif - - if (readHandle == 0 && childPID != 0) - readHandle = fdopen (pipeHandle, "r"); + if (childPID != 0) + { + int childState = 0; + const int pid = waitpid (childPID, &childState, WNOHANG|WUNTRACED); + if (pid == 0) + return true; + if ( ! (WIFEXITED (childState) || WIFSIGNALED (childState) || WIFSTOPPED (childState))) + return true; - if (readHandle != 0) - return (int) fread (dest, 1, (size_t) numBytes, readHandle); + childPID = 0; + return false; + } - return 0; + return false; } - bool killProcess() const noexcept + bool killProcess() noexcept { - return ::kill (childPID, SIGKILL) == 0; + if (::kill (childPID, SIGKILL) == 0) + { + childPID = 0; + return true; + } + + return false; } bool terminateProcess() const noexcept @@ -269,10 +216,6 @@ public: int childPID; -private: - int pipeHandle; - FILE* readHandle; - CARLA_DECLARE_NON_COPY_CLASS (ActiveProcess) }; #endif @@ -287,11 +230,6 @@ bool ChildProcess::isRunning() const return activeProcess != nullptr && activeProcess->isRunning(); } -int ChildProcess::readProcessOutput (void* dest, int numBytes) -{ - return activeProcess != nullptr ? activeProcess->read (dest, numBytes) : 0; -} - bool ChildProcess::kill() { return activeProcess == nullptr || activeProcess->killProcess(); @@ -307,13 +245,15 @@ uint32 ChildProcess::getExitCode() const return activeProcess != nullptr ? activeProcess->getExitCode() : 0; } -bool ChildProcess::waitForProcessToFinish (const int timeoutMs) const +bool ChildProcess::waitForProcessToFinish (const int timeoutMs) { const uint32 timeoutTime = Time::getMillisecondCounter() + (uint32) timeoutMs; do { - if (! isRunning()) + if (activeProcess == nullptr) + return true; + if (! activeProcess->checkRunningAndUnsetPID()) return true; carla_msleep(5); @@ -323,24 +263,6 @@ bool ChildProcess::waitForProcessToFinish (const int timeoutMs) const return false; } -String ChildProcess::readAllProcessOutput() -{ - MemoryOutputStream result; - - for (;;) - { - char buffer [512]; - const int num = readProcessOutput (buffer, sizeof (buffer)); - - if (num <= 0) - break; - - result.write (buffer, (size_t) num); - } - - return result.toString(); -} - uint32 ChildProcess::getPID() const noexcept { return activeProcess != nullptr ? activeProcess->getPID() : 0; @@ -349,9 +271,9 @@ uint32 ChildProcess::getPID() const noexcept //===================================================================================================================== #ifdef CARLA_OS_WIN -bool ChildProcess::start (const String& command, int streamFlags) +bool ChildProcess::start (const String& command) { - activeProcess = new ActiveProcess (command, streamFlags); + activeProcess = new ActiveProcess (command); if (! activeProcess->ok) activeProcess = nullptr; @@ -359,7 +281,7 @@ bool ChildProcess::start (const String& command, int streamFlags) return activeProcess != nullptr; } -bool ChildProcess::start (const StringArray& args, int streamFlags) +bool ChildProcess::start (const StringArray& args) { String escaped; @@ -378,20 +300,20 @@ bool ChildProcess::start (const StringArray& args, int streamFlags) escaped << ' '; } - return start (escaped.trim(), streamFlags); + return start (escaped.trim()); } #else -bool ChildProcess::start (const String& command, int streamFlags) +bool ChildProcess::start (const String& command) { - return start (StringArray::fromTokens (command, true), streamFlags); + return start (StringArray::fromTokens (command, true)); } -bool ChildProcess::start (const StringArray& args, int streamFlags) +bool ChildProcess::start (const StringArray& args) { if (args.size() == 0) return false; - activeProcess = new ActiveProcess (args, streamFlags); + activeProcess = new ActiveProcess (args); if (activeProcess->childPID == 0) activeProcess = nullptr; diff --git a/source/modules/water/threads/ChildProcess.h b/source/modules/water/threads/ChildProcess.h index b45e89d13..314a5db89 100644 --- a/source/modules/water/threads/ChildProcess.h +++ b/source/modules/water/threads/ChildProcess.h @@ -53,13 +53,6 @@ public: */ ~ChildProcess(); - /** These flags are used by the start() methods. */ - enum StreamFlags - { - wantStdOut = 1, - wantStdErr = 2 - }; - /** Attempts to launch a child process command. The command should be the name of the executable file, followed by any arguments @@ -69,7 +62,7 @@ public: The streamFlags is a combinations of values to indicate which of the child's output streams should be read and returned by readProcessOutput(). */ - bool start (const String& command, int streamFlags = wantStdOut | wantStdErr); + bool start (const String& command); /** Attempts to launch a child process command. @@ -80,24 +73,13 @@ public: The streamFlags is a combinations of values to indicate which of the child's output streams should be read and returned by readProcessOutput(). */ - bool start (const StringArray& arguments, int streamFlags = wantStdOut | wantStdErr); + bool start (const StringArray& arguments); /** Returns true if the child process is alive. */ bool isRunning() const; - /** Attempts to read some output from the child process. - This will attempt to read up to the given number of bytes of data from the - process. It returns the number of bytes that were actually read. - */ - int readProcessOutput (void* destBuffer, int numBytesToRead); - - /** Blocks until the process has finished, and then returns its complete output - as a string. - */ - String readAllProcessOutput(); - /** Blocks until the process is no longer running. */ - bool waitForProcessToFinish (int timeoutMs) const; + bool waitForProcessToFinish (int timeoutMs); /** If the process has finished, this returns its exit code. */ uint32 getExitCode() const;