From a02990beff549b9d768633eed1d81975a20ce35b Mon Sep 17 00:00:00 2001 From: SirLynix Date: Tue, 23 Jan 2024 15:10:27 +0100 Subject: [PATCH] Core/Posix: Rework implementation (using posix_spawn) --- src/Nazara/Core/Posix/PosixUtils.cpp | 6 +-- src/Nazara/Core/Posix/ProcessImpl.cpp | 56 ++++++++++----------------- 2 files changed, 24 insertions(+), 38 deletions(-) diff --git a/src/Nazara/Core/Posix/PosixUtils.cpp b/src/Nazara/Core/Posix/PosixUtils.cpp index 7bf84f354..69cf41651 100644 --- a/src/Nazara/Core/Posix/PosixUtils.cpp +++ b/src/Nazara/Core/Posix/PosixUtils.cpp @@ -24,7 +24,7 @@ namespace Nz::PlatformImpl { ret = ::close(fd); } - while (ret != -1 || errno == EINTR); + while (ret == -1 && errno == EINTR); return ret; #endif @@ -37,7 +37,7 @@ namespace Nz::PlatformImpl { ret = ::read(fd, buf, count); } - while (ret != -1 || errno == EINTR); + while (ret == -1 && errno == EINTR); return ret; } @@ -49,7 +49,7 @@ namespace Nz::PlatformImpl { ret = ::write(fd, buf, count); } - while (ret != -1 || errno == EINTR); + while (ret == -1 && errno == EINTR); return ret; } diff --git a/src/Nazara/Core/Posix/ProcessImpl.cpp b/src/Nazara/Core/Posix/ProcessImpl.cpp index 2b36dd711..2035c9886 100644 --- a/src/Nazara/Core/Posix/ProcessImpl.cpp +++ b/src/Nazara/Core/Posix/ProcessImpl.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -21,7 +22,7 @@ namespace Nz::PlatformImpl return Ok(true); if (errno == ESRCH) - return OK(false); + return Ok(false); return Err(Error::GetLastSystemError()); } @@ -43,9 +44,10 @@ namespace Nz::PlatformImpl if (!pipe) return Err("failed to create pipe: " + Error::GetLastSystemError()); + std::filesystem::path fullpath = std::filesystem::absolute(program); + // Double fork (see https://0xjet.github.io/3OHA/2022/04/11/post.html) // We will create a child and a grand-child process, using a pipe to retrieve the grand-child pid - // TODO: Maybe use vfork for the child process too? // TODO: Use posix_spawn if possible instead pid_t childPid = ::fork(); if (childPid == -1) @@ -71,40 +73,24 @@ namespace Nz::PlatformImpl } } - pid_t grandChildPid = ::vfork(); - if (grandChildPid == 0) + StackArray args = NazaraStackArrayNoInit(char*, arguments.size() + 2); + + // It's safe to const_cast here as we're using a copy of the memory (from child) from the original process + args[0] = const_cast(program.c_str()); + for (std::size_t i = 0; i < arguments.size(); ++i) + args[i + 1] = const_cast(arguments[i].data()); + + args.back() = nullptr; + + char* envs[] = { nullptr }; + + pid_t grandChildPid; + if (posix_spawn(&grandChildPid, fullpath.c_str(), nullptr, nullptr, args.data(), envs) == 0) { - // Grand-child process - StackArray argv = NazaraStackArrayNoInit(char*, arguments.size() + 2); + PidOrErr pid; + pid.pid = grandChildPid; - // It's safe to const_cast here as we're using a copy of the memory (from child) from the original process - argv[0] = const_cast(program.c_str()); - for (std::size_t i = 0; i < arguments.size(); ++i) - argv[i + 1] = const_cast(arguments[i].data()); - - argv[argv.size() - 1] = nullptr; - - char* envs[] = { nullptr }; - - ::execve(program.c_str(), argv.data(), envs); - - // If we get here, execve failed - // Remember we share the memory of our parent (vfork) so we need to exit using _exit() to avoid calling the parent exit handlers - - PidOrErr err; - err.pid = -1; - err.err = errno; - - pipe.Write(&err, sizeof(err)); - - _exit(1); - } - else if (grandChildPid != -1) - { - PidOrErr err; - err.pid = grandChildPid; - - pipe.Write(&err, sizeof(err)); + pipe.Write(&pid, sizeof(pid)); // Exits the child process, at this point the grand-child should have started std::exit(0); @@ -131,7 +117,7 @@ namespace Nz::PlatformImpl ::waitpid(childPid, &childStatus, 0); PidOrErr pidOrErr; - if (pipe.Read(&pidOrErr, sizeof(pidOrErr) != sizeof(pidOrErr))) + if (pipe.Read(&pidOrErr, sizeof(pidOrErr)) != sizeof(pidOrErr)) { // this should never happen return Err("failed to create child: couldn't retrieve status from pipe");