From 0d3ce06dd9c0de06e2afdeac9471a05267e5b49f Mon Sep 17 00:00:00 2001 From: trcrsired Date: Thu, 12 Dec 2024 02:59:20 -0500 Subject: [PATCH 1/6] Remove error pipe and use {fd errno} for error propagation in posix --- .../platforms/systemcall_details.h | 11 +-- .../fast_io_hosted/process/process/posix.h | 80 +++++-------------- 2 files changed, 24 insertions(+), 67 deletions(-) diff --git a/include/fast_io_hosted/platforms/systemcall_details.h b/include/fast_io_hosted/platforms/systemcall_details.h index 9b45758d..c4fe2697 100644 --- a/include/fast_io_hosted/platforms/systemcall_details.h +++ b/include/fast_io_hosted/platforms/systemcall_details.h @@ -75,8 +75,9 @@ inline int sys_dup2(int old_fd, int new_fd) struct return_code { - int code{}; - bool error{}; + using errno_number_type = ::std::remove_cvref_t; + int fd{-1}; + errno_number_type error_number{}; }; inline return_code sys_dup2_nothrow(int old_fd, int new_fd) noexcept @@ -85,7 +86,7 @@ inline return_code sys_dup2_nothrow(int old_fd, int new_fd) noexcept int fd{system_call<__NR_dup2, int>(old_fd, new_fd)}; if (linux_system_call_fails(fd)) { - return {-fd, true}; + return {-1, static_cast(-fd)}; } #else auto fd{noexcept_call( @@ -98,10 +99,10 @@ inline return_code sys_dup2_nothrow(int old_fd, int new_fd) noexcept old_fd, new_fd)}; if (fd == -1) { - return {errno, true}; + return {-1, errno}; } #endif - return {fd}; + return {fd, 0}; } inline int sys_close(int fd) noexcept diff --git a/include/fast_io_hosted/process/process/posix.h b/include/fast_io_hosted/process/process/posix.h index a79985da..c7a1e8c9 100644 --- a/include/fast_io_hosted/process/process/posix.h +++ b/include/fast_io_hosted/process/process/posix.h @@ -144,17 +144,22 @@ inline void posix_waitpid_noexcept(pid_t pid) noexcept #endif } -inline int posix_execveat(int dirfd, char const *cstr, char const *const *args, char const *const *envp) noexcept +inline return_code posix_execveat(int dirfd, char const *cstr, char const *const *args, char const *const *envp) noexcept { #if defined(__linux__) && defined(__NR_execveat) - return -(system_call<__NR_execveat, int>(dirfd, cstr, args, envp, AT_SYMLINK_NOFOLLOW)); + auto ret{system_call<__NR_execveat, int>(dirfd, cstr, args, envp, AT_SYMLINK_NOFOLLOW)}; + if (linux_system_call_fails(ret)) + { + return {-1, -ret}; + } + return {0, ret}; #else int fd{::fast_io::details::my_posix_openat_noexcept(dirfd, cstr, O_RDONLY | O_NOFOLLOW, 0644)}; if (fd != -1) [[likely]] { ::fast_io::posix::libc_fexecve(fd, const_cast(args), const_cast(envp)); } - return errno; + return {fd, errno}; #endif } @@ -178,12 +183,12 @@ struct io_redirector { return_code rc; rc = redirect(0, pio.in); - if (rc.error) + if (rc.fd == -1) { return rc; } rc = redirect(1, pio.out); - if (rc.error) + if (rc.fd == -1) { return rc; } @@ -204,7 +209,7 @@ struct io_redirector // the read/write ends of pipe are all open // the user shouldn't close them if they pass entire pipe as argument rc = sys_dup2_nothrow(d.pipe_fds[is_stdin ? 0 : 1], target_fd); - if (rc.error) + if (rc.fd == -1) { return rc; } @@ -219,11 +224,7 @@ struct io_redirector { rc = sys_dup2_nothrow(d.fd, target_fd); } - if (rc.error) - { - return rc; - } - return {}; + return rc; } // only used by parent process @@ -260,34 +261,19 @@ struct io_redirector inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char const *const *args, char const *const *envp, posix_process_io const &pio) { - posix_pipe error_pipe; pid_t pid = posix_fork(); if (pid == 0) { // subprocess - error_pipe.in().close(); - - int t_errno{}; + return_code rc; // io redirection - { - io_redirector r; - auto rc = r.redirect_all(pio); - if (rc.error) - { - t_errno = rc.code; - } - } + io_redirector r; + rc = r.redirect_all(pio); - if (t_errno == 0) + if (rc.fd != -1) { - t_errno = posix_execveat(dirfd, cstr, args, envp); + posix_execveat(dirfd, cstr, args, envp); } - // execve only return on error, so t_errno always contains an error code - // send error code back to parent process - auto err_buffer = reinterpret_cast(&t_errno); - ::fast_io::operations::write_all(error_pipe.out(), err_buffer, err_buffer + sizeof(t_errno)); - // _Exit() won't destruct c++ objects, we must close it manually - error_pipe.out().close(); // special exit code 127 indicates error of exec #if defined(__linux__) #ifdef __NR_exit_group @@ -299,37 +285,7 @@ inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char con ::fast_io::posix::libc_exit(127); #endif } - // parent process - // currently parent process never close pipes - // uncomment those lines to enable automatically closing pipe ends -#if 0 - io_redirector::close_pipe_ends(0, pio.in); - io_redirector::close_pipe_ends(1, pio.out); - io_redirector::close_pipe_ends(2, pio.err); -#endif - - error_pipe.out().close(); - int errno_from_subproc{}; - auto err_buffer = reinterpret_cast(&errno_from_subproc); - auto err_buffer_end = err_buffer + sizeof(errno_from_subproc); - auto n = ::fast_io::operations::read_some(error_pipe.in(), err_buffer, err_buffer_end); - if (n == err_buffer) - { - return pid; - } - else - { - posix_waitpid_noexcept(pid); - if (n == err_buffer_end) - { - throw_posix_error(errno_from_subproc); - } - else [[unlikely]] - { - // sub process died before sending error code, assume it an io error - throw_posix_error(EIO); - } - } + posix_waitpid(pid); } template From 699e06f32a599562134ab81b9a712e2dd98bbdd6 Mon Sep 17 00:00:00 2001 From: trcrsired Date: Thu, 12 Dec 2024 03:09:22 -0500 Subject: [PATCH 2/6] this code is junk --- include/fast_io_hosted/process/process/posix.h | 1 + 1 file changed, 1 insertion(+) diff --git a/include/fast_io_hosted/process/process/posix.h b/include/fast_io_hosted/process/process/posix.h index c7a1e8c9..568228ce 100644 --- a/include/fast_io_hosted/process/process/posix.h +++ b/include/fast_io_hosted/process/process/posix.h @@ -286,6 +286,7 @@ inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char con #endif } posix_waitpid(pid); + return 0; } template From b862dc9af8824d9cfa9047a2d25797fd09214650 Mon Sep 17 00:00:00 2001 From: trcrsired Date: Thu, 12 Dec 2024 03:17:08 -0500 Subject: [PATCH 3/6] remove error pipe in fork_execveat chain --- include/fast_io_hosted/process/process/posix.h | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/include/fast_io_hosted/process/process/posix.h b/include/fast_io_hosted/process/process/posix.h index 568228ce..039f1c98 100644 --- a/include/fast_io_hosted/process/process/posix.h +++ b/include/fast_io_hosted/process/process/posix.h @@ -285,8 +285,7 @@ inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char con ::fast_io::posix::libc_exit(127); #endif } - posix_waitpid(pid); - return 0; + return pid; } template From e7ded8a872a794ba6989ef0702feb45cfc98f14d Mon Sep 17 00:00:00 2001 From: trcrsired Date: Thu, 12 Dec 2024 03:57:23 -0500 Subject: [PATCH 4/6] Revert "remove error pipe in fork_execveat chain" This reverts commit b862dc9af8824d9cfa9047a2d25797fd09214650. --- include/fast_io_hosted/process/process/posix.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/fast_io_hosted/process/process/posix.h b/include/fast_io_hosted/process/process/posix.h index 039f1c98..568228ce 100644 --- a/include/fast_io_hosted/process/process/posix.h +++ b/include/fast_io_hosted/process/process/posix.h @@ -285,7 +285,8 @@ inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char con ::fast_io::posix::libc_exit(127); #endif } - return pid; + posix_waitpid(pid); + return 0; } template From e3fc95aad29253c7d28e6157c6594c9a15d9b584 Mon Sep 17 00:00:00 2001 From: trcrsired Date: Thu, 12 Dec 2024 03:57:42 -0500 Subject: [PATCH 5/6] Revert "this code is junk" This reverts commit 699e06f32a599562134ab81b9a712e2dd98bbdd6. --- include/fast_io_hosted/process/process/posix.h | 1 - 1 file changed, 1 deletion(-) diff --git a/include/fast_io_hosted/process/process/posix.h b/include/fast_io_hosted/process/process/posix.h index 568228ce..c7a1e8c9 100644 --- a/include/fast_io_hosted/process/process/posix.h +++ b/include/fast_io_hosted/process/process/posix.h @@ -286,7 +286,6 @@ inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char con #endif } posix_waitpid(pid); - return 0; } template From 3ea2a7fd01791140cc9bc4f8fb92de36867ad652 Mon Sep 17 00:00:00 2001 From: trcrsired Date: Thu, 12 Dec 2024 03:58:07 -0500 Subject: [PATCH 6/6] Revert "Remove error pipe and use {fd errno} for error propagation in posix" This reverts commit 0d3ce06dd9c0de06e2afdeac9471a05267e5b49f. --- .../platforms/systemcall_details.h | 11 ++- .../fast_io_hosted/process/process/posix.h | 80 ++++++++++++++----- 2 files changed, 67 insertions(+), 24 deletions(-) diff --git a/include/fast_io_hosted/platforms/systemcall_details.h b/include/fast_io_hosted/platforms/systemcall_details.h index c4fe2697..9b45758d 100644 --- a/include/fast_io_hosted/platforms/systemcall_details.h +++ b/include/fast_io_hosted/platforms/systemcall_details.h @@ -75,9 +75,8 @@ inline int sys_dup2(int old_fd, int new_fd) struct return_code { - using errno_number_type = ::std::remove_cvref_t; - int fd{-1}; - errno_number_type error_number{}; + int code{}; + bool error{}; }; inline return_code sys_dup2_nothrow(int old_fd, int new_fd) noexcept @@ -86,7 +85,7 @@ inline return_code sys_dup2_nothrow(int old_fd, int new_fd) noexcept int fd{system_call<__NR_dup2, int>(old_fd, new_fd)}; if (linux_system_call_fails(fd)) { - return {-1, static_cast(-fd)}; + return {-fd, true}; } #else auto fd{noexcept_call( @@ -99,10 +98,10 @@ inline return_code sys_dup2_nothrow(int old_fd, int new_fd) noexcept old_fd, new_fd)}; if (fd == -1) { - return {-1, errno}; + return {errno, true}; } #endif - return {fd, 0}; + return {fd}; } inline int sys_close(int fd) noexcept diff --git a/include/fast_io_hosted/process/process/posix.h b/include/fast_io_hosted/process/process/posix.h index c7a1e8c9..a79985da 100644 --- a/include/fast_io_hosted/process/process/posix.h +++ b/include/fast_io_hosted/process/process/posix.h @@ -144,22 +144,17 @@ inline void posix_waitpid_noexcept(pid_t pid) noexcept #endif } -inline return_code posix_execveat(int dirfd, char const *cstr, char const *const *args, char const *const *envp) noexcept +inline int posix_execveat(int dirfd, char const *cstr, char const *const *args, char const *const *envp) noexcept { #if defined(__linux__) && defined(__NR_execveat) - auto ret{system_call<__NR_execveat, int>(dirfd, cstr, args, envp, AT_SYMLINK_NOFOLLOW)}; - if (linux_system_call_fails(ret)) - { - return {-1, -ret}; - } - return {0, ret}; + return -(system_call<__NR_execveat, int>(dirfd, cstr, args, envp, AT_SYMLINK_NOFOLLOW)); #else int fd{::fast_io::details::my_posix_openat_noexcept(dirfd, cstr, O_RDONLY | O_NOFOLLOW, 0644)}; if (fd != -1) [[likely]] { ::fast_io::posix::libc_fexecve(fd, const_cast(args), const_cast(envp)); } - return {fd, errno}; + return errno; #endif } @@ -183,12 +178,12 @@ struct io_redirector { return_code rc; rc = redirect(0, pio.in); - if (rc.fd == -1) + if (rc.error) { return rc; } rc = redirect(1, pio.out); - if (rc.fd == -1) + if (rc.error) { return rc; } @@ -209,7 +204,7 @@ struct io_redirector // the read/write ends of pipe are all open // the user shouldn't close them if they pass entire pipe as argument rc = sys_dup2_nothrow(d.pipe_fds[is_stdin ? 0 : 1], target_fd); - if (rc.fd == -1) + if (rc.error) { return rc; } @@ -224,7 +219,11 @@ struct io_redirector { rc = sys_dup2_nothrow(d.fd, target_fd); } - return rc; + if (rc.error) + { + return rc; + } + return {}; } // only used by parent process @@ -261,19 +260,34 @@ struct io_redirector inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char const *const *args, char const *const *envp, posix_process_io const &pio) { + posix_pipe error_pipe; pid_t pid = posix_fork(); if (pid == 0) { // subprocess - return_code rc; + error_pipe.in().close(); + + int t_errno{}; // io redirection - io_redirector r; - rc = r.redirect_all(pio); + { + io_redirector r; + auto rc = r.redirect_all(pio); + if (rc.error) + { + t_errno = rc.code; + } + } - if (rc.fd != -1) + if (t_errno == 0) { - posix_execveat(dirfd, cstr, args, envp); + t_errno = posix_execveat(dirfd, cstr, args, envp); } + // execve only return on error, so t_errno always contains an error code + // send error code back to parent process + auto err_buffer = reinterpret_cast(&t_errno); + ::fast_io::operations::write_all(error_pipe.out(), err_buffer, err_buffer + sizeof(t_errno)); + // _Exit() won't destruct c++ objects, we must close it manually + error_pipe.out().close(); // special exit code 127 indicates error of exec #if defined(__linux__) #ifdef __NR_exit_group @@ -285,7 +299,37 @@ inline pid_t pipefork_execveat_common_impl(int dirfd, char const *cstr, char con ::fast_io::posix::libc_exit(127); #endif } - posix_waitpid(pid); + // parent process + // currently parent process never close pipes + // uncomment those lines to enable automatically closing pipe ends +#if 0 + io_redirector::close_pipe_ends(0, pio.in); + io_redirector::close_pipe_ends(1, pio.out); + io_redirector::close_pipe_ends(2, pio.err); +#endif + + error_pipe.out().close(); + int errno_from_subproc{}; + auto err_buffer = reinterpret_cast(&errno_from_subproc); + auto err_buffer_end = err_buffer + sizeof(errno_from_subproc); + auto n = ::fast_io::operations::read_some(error_pipe.in(), err_buffer, err_buffer_end); + if (n == err_buffer) + { + return pid; + } + else + { + posix_waitpid_noexcept(pid); + if (n == err_buffer_end) + { + throw_posix_error(errno_from_subproc); + } + else [[unlikely]] + { + // sub process died before sending error code, assume it an io error + throw_posix_error(EIO); + } + } } template