Skip to content

Commit a54a21d

Browse files
committed
Fixed SIGPIPE handling for pipelines that terminate early
1 parent f537926 commit a54a21d

File tree

2 files changed

+23
-7
lines changed

2 files changed

+23
-7
lines changed

include/subprocess/subprocess.hpp

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -425,7 +425,7 @@ class file_descriptor : public virtual descriptor
425425
* modified by umask like normal.
426426
*/
427427
file_descriptor(std::string path, int flags, mode_t mode = 0666)
428-
: path_{std::move(path)}, flags_{flags}, mode_{mode}
428+
: path_{std::move(path)}, flags_{O_CLOEXEC | flags}, mode_{mode}
429429
{
430430
}
431431
file_descriptor(const file_descriptor&) = default;
@@ -540,6 +540,11 @@ inline void opipe_descriptor::open()
540540
{
541541
throw exceptions::os_error{"pipe"};
542542
}
543+
//! Required because MacOS doesn't have ::pipe2
544+
for (auto pipe_fd : fd)
545+
{
546+
::fcntl(pipe_fd, F_SETFD, FD_CLOEXEC);
547+
}
543548
linked_fd_->fd_ = fd[0];
544549
fd_ = fd[1];
545550
}
@@ -555,6 +560,11 @@ inline void ipipe_descriptor::open()
555560
{
556561
throw exceptions::os_error{"pipe"};
557562
}
563+
//! Required because MacOS doesn't have ::pipe2
564+
for (auto pipe_fd : fd)
565+
{
566+
::fcntl(pipe_fd, F_SETFD, FD_CLOEXEC);
567+
}
558568
fd_ = fd[0];
559569
linked_fd_->fd_ = fd[1];
560570
}
@@ -617,7 +627,7 @@ inline void ivariable_descriptor::open()
617627
* any one of the resulting objects to initialize the pipe.
618628
*
619629
* @return std::pair<descriptor_ptr_t<ipipe_descriptor>, descriptor_ptr_t<opipe_descriptor>> A pair of linked
620-
* file_descriptos [read_fd, write_fd]
630+
* pipe_descriptors [read_fd, write_fd]
621631
*/
622632
inline std::pair<descriptor_ptr_t<ipipe_descriptor>, descriptor_ptr_t<opipe_descriptor>> create_pipe()
623633
{
@@ -744,8 +754,8 @@ class posix_spawn_file_actions
744754
* - All descriptors are opened with a call to descriptor::open().
745755
* - descriptor::fd() is used to get descriptors for the child process and
746756
* dup them to stdin, stdout, and stderr
747-
* - In the child process, the descriptors returned by descriptor::fd() would
748-
* be closed.
757+
* - In the child process, the descriptors would be duped to the their std counterparts
758+
* and all the fd's marked with O_CLOEXEC would be automatically closed.
749759
* - After the process is spawned, the parent process closes each descriptor with
750760
* a call to descriptor::close.
751761
*
@@ -806,9 +816,6 @@ inline void posix_process::execute()
806816
process_fds(action, stdin_fd_, posix_util::standard_filenos::standard_in);
807817
process_fds(action, stdout_fd_, posix_util::standard_filenos::standard_out);
808818
process_fds(action, stderr_fd_, posix_util::standard_filenos::standard_error);
809-
action.close(stdin_fd_);
810-
action.close(stdout_fd_);
811-
action.close(stderr_fd_);
812819

813820
int pid{};
814821
if (int err{::posix_spawnp(&pid, sh.argv()[0], action.get(), nullptr, sh.argv(), nullptr)}; err != 0)

test/source/subprocess_test.cpp

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,15 @@ TEST_CASE("bash-like redirection")
7070
REQUIRE(not output.empty());
7171
}
7272

73+
TEST_CASE("SIGPIPE handling for child processes")
74+
{
75+
using namespace subprocess::literals;
76+
std::string output;
77+
int errc = ("yes"_cmd | "head -n1"_cmd > output).run();
78+
REQUIRE_EQ(errc, 0);
79+
REQUIRE_EQ(output, "y\n");
80+
}
81+
7382
TEST_CASE("pipe_descriptors double linking not allowed")
7483
{
7584
auto [read_desc, write_desc] = subprocess::create_pipe();

0 commit comments

Comments
 (0)