Skip to content

Commit faa1c3e

Browse files
author
MarcoFalke
committed
Revert "Merge bitcoin#32343: common: Close non-std fds before exec in RunCommandJSON"
This reverts commit 31d3eeb, reversing changes made to 4b26ca0.
1 parent 31d3eeb commit faa1c3e

File tree

2 files changed

+1
-59
lines changed

2 files changed

+1
-59
lines changed

src/common/run_command.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string&
2424

2525
if (str_command.empty()) return UniValue::VNULL;
2626

27-
auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}, sp::close_fds{true});
27+
auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE});
2828
if (!str_std_in.empty()) {
2929
c.send(str_std_in);
3030
}

src/util/subprocess.h

Lines changed: 0 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,6 @@ Documentation for C++ subprocessing library.
3636
#ifndef BITCOIN_UTIL_SUBPROCESS_H
3737
#define BITCOIN_UTIL_SUBPROCESS_H
3838

39-
#include <util/fs.h>
40-
#include <util/strencodings.h>
4139
#include <util/syserror.h>
4240

4341
#include <algorithm>
@@ -538,20 +536,6 @@ namespace util
538536
* -------------------------------
539537
*/
540538

541-
/*!
542-
* Option to close all file descriptors
543-
* when the child process is spawned.
544-
* The close fd list does not include
545-
* input/output/error if they are explicitly
546-
* set as part of the Popen arguments.
547-
*
548-
* Default value is false.
549-
*/
550-
struct close_fds {
551-
explicit close_fds(bool c): close_all(c) {}
552-
bool close_all = false;
553-
};
554-
555539
/*!
556540
* Base class for all arguments involving string value.
557541
*/
@@ -749,7 +733,6 @@ struct ArgumentDeducer
749733
void set_option(input&& inp);
750734
void set_option(output&& out);
751735
void set_option(error&& err);
752-
void set_option(close_fds&& cfds);
753736

754737
private:
755738
Popen* popen_ = nullptr;
@@ -1043,8 +1026,6 @@ class Popen
10431026
std::future<void> cleanup_future_;
10441027
#endif
10451028

1046-
bool close_fds_ = false;
1047-
10481029
std::string exe_name_;
10491030

10501031
// Command in string format
@@ -1288,10 +1269,6 @@ namespace detail {
12881269
if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_;
12891270
}
12901271

1291-
inline void ArgumentDeducer::set_option(close_fds&& cfds) {
1292-
popen_->close_fds_ = cfds.close_all;
1293-
}
1294-
12951272

12961273
inline void Child::execute_child() {
12971274
#ifndef __USING_WINDOWS__
@@ -1338,41 +1315,6 @@ namespace detail {
13381315
if (stream.err_write_ != -1 && stream.err_write_ > 2)
13391316
subprocess_close(stream.err_write_);
13401317

1341-
// Close all the inherited fd's except the error write pipe
1342-
if (parent_->close_fds_) {
1343-
// If possible, try to get the list of open file descriptors from the
1344-
// operating system. This is more efficient, but not guaranteed to be
1345-
// available.
1346-
#ifdef __linux__
1347-
// For Linux, enumerate /proc/<pid>/fd.
1348-
try {
1349-
std::vector<int> fds_to_close;
1350-
for (const auto& it : fs::directory_iterator(strprintf("/proc/%d/fd", getpid()))) {
1351-
auto fd{ToIntegral<uint64_t>(it.path().filename().native())};
1352-
if (!fd || *fd > std::numeric_limits<int>::max()) continue;
1353-
if (*fd <= 2) continue; // leave std{in,out,err} alone
1354-
if (*fd == static_cast<uint64_t>(err_wr_pipe_)) continue;
1355-
fds_to_close.push_back(*fd);
1356-
}
1357-
for (const int fd : fds_to_close) {
1358-
close(fd);
1359-
}
1360-
} catch (const fs::filesystem_error &e) {
1361-
throw OSError("/proc/<pid>/fd iteration failed", e.code().value());
1362-
}
1363-
#else
1364-
// On other operating systems, iterate over all file descriptor slots
1365-
// and try to close them all.
1366-
int max_fd = sysconf(_SC_OPEN_MAX);
1367-
if (max_fd == -1) throw OSError("sysconf failed", errno);
1368-
1369-
for (int i = 3; i < max_fd; i++) {
1370-
if (i == err_wr_pipe_) continue;
1371-
close(i);
1372-
}
1373-
#endif
1374-
}
1375-
13761318
// Replace the current image with the executable
13771319
sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data());
13781320

0 commit comments

Comments
 (0)