Skip to content

Commit 8ba41f4

Browse files
committed
Fix pipe (double close)
1 parent 0797cb5 commit 8ba41f4

4 files changed

Lines changed: 40 additions & 6 deletions

File tree

coroio/base.hpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,9 @@ struct TTimer {
2525
};
2626

2727
struct THandlePair {
28-
THandle Read;
29-
THandle Write;
30-
THandle RHup;
28+
THandle Read = {};
29+
THandle Write = {};
30+
THandle RHup = {};
3131
};
3232

3333
struct TEvent {

coroio/pipe/pipe.cpp

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,6 @@ TPipe::TPipeLow::~TPipeLow() {
5050
int status = 0;
5151
kill(ChildPid, SIGKILL);
5252
waitpid(ChildPid, &status, 0);
53-
if (ReadFd != -1) { close(ReadFd); }
54-
if (WriteFd != -1) { close(WriteFd); }
55-
if (ErrFd != -1) { close(ErrFd); }
5653
}
5754
}
5855

coroio/pipe/pipe.hpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,8 @@ class TPipe {
2121
}
2222
}
2323

24+
int Pid() const { return PipeLow.ChildPid; }
25+
2426
void CloseRead() {
2527
ReadHandle.reset();
2628
}
@@ -49,6 +51,7 @@ class TPipe {
4951
std::string Exe;
5052
std::vector<std::string> Args;
5153

54+
// Descriptors are owned by TPipeFileHandle, do not close them!
5255
int ReadFd = -1;
5356
int WriteFd = -1;
5457
int ErrFd = -1;

tests/test_pipe.cpp

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include <stddef.h>
77
#include <setjmp.h>
88
#include <signal.h>
9+
#include <errno.h>
910
#include <iostream>
1011
#include <unordered_set>
1112

@@ -107,6 +108,38 @@ void test_pipe_merge_stderr_into_stdout(void**) {
107108
assert_true(memcmp(buffer, expected, sizeof(expected)-1) == 0);
108109
}
109110

111+
void test_pipe_kill_then_read(void**) {
112+
TLoop<TDefaultPoller> loop;
113+
auto& poller = loop.Poller();
114+
std::string sleepPath = "/bin/sleep";
115+
// Start a process that produces no output and runs for a while
116+
TPipe pipe(poller, sleepPath, {"5"});
117+
118+
int pid = pipe.Pid();
119+
assert_true(pid > 0);
120+
121+
int rc = kill(pid, SIGKILL);
122+
assert_true(rc == 0 || errno == ESRCH);
123+
124+
// Attempt to read from stdout; should observe EOF (0 bytes)
125+
char buffer[8] = {};
126+
size_t bytesRead = sizeof(buffer);
127+
auto reader = [](TPipe& pipe, char* buffer, size_t& size) -> TFuture<void> {
128+
try {
129+
size = co_await pipe.ReadSome(buffer, size);
130+
} catch (const std::exception& ex) {
131+
// Accept exceptions as valid outcome on abrupt termination
132+
size = 0;
133+
}
134+
}(pipe, buffer, bytesRead);
135+
136+
while (!reader.done()) {
137+
loop.Step();
138+
}
139+
140+
assert_true(bytesRead == 0);
141+
}
142+
110143
#endif // _WIN32
111144

112145
int main(int argc, char** argv) {
@@ -122,6 +155,7 @@ int main(int argc, char** argv) {
122155
ADD_TEST(cmocka_unit_test, test_pipe_basic_read_write);
123156
ADD_TEST(cmocka_unit_test, test_pipe_read_stderr);
124157
ADD_TEST(cmocka_unit_test, test_pipe_merge_stderr_into_stdout);
158+
ADD_TEST(cmocka_unit_test, test_pipe_kill_then_read);
125159
#endif
126160

127161
return _cmocka_run_group_tests("test_pipe", tests.data(), tests.size(), NULL, NULL);

0 commit comments

Comments
 (0)