From faa1c3e80d95552bdc2c0e717065ebf8d510138f Mon Sep 17 00:00:00 2001 From: MarcoFalke <*~=`'#}+{/-|&$^_@721217.xyz> Date: Fri, 25 Jul 2025 15:32:48 +0200 Subject: [PATCH] Revert "Merge bitcoin/bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON" This reverts commit 31d3eebfb92ae0521e18225d69be95e78fb02672, reversing changes made to 4b26ca0e2f1ec6b68861f1e8c4fd932f8fd8a271. --- src/common/run_command.cpp | 2 +- src/util/subprocess.h | 58 -------------------------------------- 2 files changed, 1 insertion(+), 59 deletions(-) diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index 110ec416353..1f6d51b4f4b 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -24,7 +24,7 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& if (str_command.empty()) return UniValue::VNULL; - auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}, sp::close_fds{true}); + auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}); if (!str_std_in.empty()) { c.send(str_std_in); } diff --git a/src/util/subprocess.h b/src/util/subprocess.h index c45a570d5f1..be4b3e5e392 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -36,8 +36,6 @@ Documentation for C++ subprocessing library. #ifndef BITCOIN_UTIL_SUBPROCESS_H #define BITCOIN_UTIL_SUBPROCESS_H -#include -#include #include #include @@ -538,20 +536,6 @@ namespace util * ------------------------------- */ -/*! - * Option to close all file descriptors - * when the child process is spawned. - * The close fd list does not include - * input/output/error if they are explicitly - * set as part of the Popen arguments. - * - * Default value is false. - */ -struct close_fds { - explicit close_fds(bool c): close_all(c) {} - bool close_all = false; -}; - /*! * Base class for all arguments involving string value. */ @@ -749,7 +733,6 @@ struct ArgumentDeducer void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); - void set_option(close_fds&& cfds); private: Popen* popen_ = nullptr; @@ -1043,8 +1026,6 @@ private: std::future cleanup_future_; #endif - bool close_fds_ = false; - std::string exe_name_; // Command in string format @@ -1288,10 +1269,6 @@ namespace detail { if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; } - inline void ArgumentDeducer::set_option(close_fds&& cfds) { - popen_->close_fds_ = cfds.close_all; - } - inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1338,41 +1315,6 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) subprocess_close(stream.err_write_); - // Close all the inherited fd's except the error write pipe - if (parent_->close_fds_) { - // If possible, try to get the list of open file descriptors from the - // operating system. This is more efficient, but not guaranteed to be - // available. -#ifdef __linux__ - // For Linux, enumerate /proc//fd. - try { - std::vector fds_to_close; - for (const auto& it : fs::directory_iterator(strprintf("/proc/%d/fd", getpid()))) { - auto fd{ToIntegral(it.path().filename().native())}; - if (!fd || *fd > std::numeric_limits::max()) continue; - if (*fd <= 2) continue; // leave std{in,out,err} alone - if (*fd == static_cast(err_wr_pipe_)) continue; - fds_to_close.push_back(*fd); - } - for (const int fd : fds_to_close) { - close(fd); - } - } catch (const fs::filesystem_error &e) { - throw OSError("/proc//fd iteration failed", e.code().value()); - } -#else - // On other operating systems, iterate over all file descriptor slots - // and try to close them all. - int max_fd = sysconf(_SC_OPEN_MAX); - if (max_fd == -1) throw OSError("sysconf failed", errno); - - for (int i = 3; i < max_fd; i++) { - if (i == err_wr_pipe_) continue; - close(i); - } -#endif - } - // Replace the current image with the executable sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());