Skip to content

Commit 728db27

Browse files
committed
fix: retain child bounds after captured stream EOF
1 parent d76ffb6 commit 728db27

4 files changed

Lines changed: 105 additions & 19 deletions

File tree

‎.agents/docs/2026-11-06-bmi-publication-and-compiler-cap-closeout.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,3 +199,25 @@ small synthetic compiler. The separate GCC mixed workload retains the real
199199
compiler cap, late failure, recovery, argv and no-op assertions. Selected-object
200200
controls establish causality; final qualification remains complete current-source
201201
CI and independent ecosystem review.
202+
203+
### Addendum, 2026-11-06: the final child wait retains its bounds
204+
205+
A final launcher control closes both captured streams before the child sleeps.
206+
The preceding correction still returned success after 2.002 seconds instead of
207+
observing the fifty-millisecond stop request: stream EOF had led to an unbounded
208+
child wait. The same boundary also bypassed an absolute deadline after EOF.
209+
210+
One shared predicate now applies during output reads, remaining error reads and
211+
the final child wait. It retains the original absolute deadline and stop request;
212+
observed output retains its idle budget after EOF. Activity on a separately
213+
captured error stream renews that observed-output budget. Inherited and discarded
214+
streams retain no inferred idle observation. Child completion is checked during
215+
the final bounded wait, preserving already completed child statuses.
216+
217+
A finite matrix covers all six stream modes, ordinary and signalled exits,
218+
completion with bounds configured, stop requests before and after one or both
219+
streams close, absolute deadlines after closure, captured idle expiry and live
220+
stderr after stdout closure. Its fifty-three cases pass, together with the
221+
original launcher tests and the direct EOF regression. Top-level signal behavior
222+
is additionally covered by the real-Ninja lifecycle control. No budget, compiler
223+
argument or output stream is changed to make a cancellation succeed.

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@
2727
another generation starts. Stable kernel permits retain live ownership across
2828
immediate rebuilds; runtime generations leave no-op build identities unchanged.
2929
- Child stop predicates apply with inherited and discarded streams, including
30-
executions without an absolute deadline.
30+
executions without an absolute deadline. Captured stream EOF retains stop,
31+
absolute and observed-output idle bounds until the child exits.
3132
- Queued downloads begin their progress budget when a worker starts them,
3233
rather than inheriting time spent waiting for a slot.
3334
- Provider progress timeouts identify the stalled download and source attempt,

‎src/run/run.cpp‎

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,14 @@ Outcome launch(const Spec& s) {
241241
auto lastOutput = start;
242242
bool stop = false;
243243
const bool keepErr = s.out == Output::stdout_err_kept;
244+
const bool captured = s.out == Output::merged || s.out == Output::stdout_only
245+
|| s.out == Output::stdout_err_shown || keepErr;
246+
auto should_stop = [&] {
247+
const auto now = clock::now();
248+
return (s.total.count() > 0 && now - start >= s.total)
249+
|| (captured && s.idle.count() > 0 && now - lastOutput >= s.idle)
250+
|| (s.shouldStop && *s.shouldStop && (*s.shouldStop)());
251+
};
244252
bool errDone = !keepErr;
245253
// The error stream, read as it arrives so that a program writing much of
246254
// it is never stopped on a full pipe while its output is read.
@@ -250,17 +258,15 @@ Outcome launch(const Spec& s) {
250258
if (!e) { errDone = true; return; }
251259
if (!e->has_value()) return;
252260
if ((*e)->empty()) { errDone = true; return; }
261+
lastOutput = clock::now();
253262
out.error_output += **e;
254263
};
255-
if (s.out == Output::merged || s.out == Output::stdout_only || s.out == Output::stdout_err_shown
256-
|| keepErr) {
264+
if (captured) {
257265
std::string acc;
258266
std::optional<clock::time_point> endedAt;
259267
for (;;) {
260268
const auto now = clock::now();
261-
if ((s.total.count() > 0 && now - start >= s.total)
262-
|| (s.idle.count() > 0 && now - lastOutput >= s.idle)
263-
|| (s.shouldStop && *s.shouldStop && (*s.shouldStop)())) {
269+
if (should_stop()) {
264270
stop = true;
265271
break;
266272
}
@@ -290,7 +296,7 @@ Outcome launch(const Spec& s) {
290296
// bounds allow.
291297
while (!stop && !errDone) {
292298
const auto now = clock::now();
293-
if (s.total.count() > 0 && now - start >= s.total) { stop = true; break; }
299+
if (should_stop()) { stop = true; break; }
294300
if (!endedAt) {
295301
if (auto w = p->try_wait(); w && w->has_value()) endedAt = now;
296302
} else if (now - *endedAt > std::chrono::milliseconds(500)) {
@@ -299,22 +305,16 @@ Outcome launch(const Spec& s) {
299305
}
300306
drain_error(std::chrono::milliseconds(100));
301307
}
302-
} else if (s.shouldStop && *s.shouldStop) {
303-
// Inherited/discarded streams have no read loop. Cancellation still
304-
// belongs to the invocation, so poll the child without changing its
305-
// streams or waiting for a separate absolute deadline.
308+
}
309+
if (!stop && (s.total.count() > 0 || (captured && s.idle.count() > 0)
310+
|| (s.shouldStop && *s.shouldStop))) {
311+
// Stream EOF is independent of child completion. Keep the same bounds
312+
// through the final wait, including inherited and discarded streams.
306313
for (;;) {
307-
if ((*s.shouldStop)()
308-
|| (s.total.count() > 0 && clock::now() - start >= s.total)) {
309-
stop = true;
310-
break;
311-
}
312314
auto waited = p->wait_for(std::chrono::milliseconds(20));
313315
if (!waited || waited->has_value()) break;
316+
if (should_stop()) { stop = true; break; }
314317
}
315-
} else if (s.total.count() > 0) {
316-
auto w = p->wait_for(s.total);
317-
if (w && !w->has_value()) stop = true;
318318
}
319319
if (stop) {
320320
p->end_unit();

‎tests/unit/test_process_run_exec.cpp‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,69 @@ namespace process = mcpp::run;
1919
// change mcpp's own environment — that mutation is exactly what leaked into
2020
// /bin/sh and crashed it on newer-glibc hosts. The addition is the started
2121
// program's alone; this invocation's statement is what it was.
22+
TEST(RunExec, StreamCompletionAndChildWaitKeepTheirBounds) {
23+
struct Case {
24+
const char* name;
25+
const char* command;
26+
int total = 0;
27+
int idle = 0;
28+
bool stop = false;
29+
int exit = -1;
30+
};
31+
const std::vector<Case> cases{
32+
{"exit", "exit 7", 0, 0, false, 7},
33+
{"signal", "kill -TERM $$", 0, 0, false, 143},
34+
{"already-exited-bounded", "exit 7", 3000, 0, true, 7},
35+
{"open-stop", "sleep 2", 0, 0, true},
36+
{"both-EOF-stop", "exec 1>&- 2>&-; sleep 2", 0, 0, true},
37+
{"stdout-EOF-stop", "exec 1>&-; sleep 2", 0, 0, true},
38+
{"both-EOF-total", "exec 1>&- 2>&-; sleep 2", 50},
39+
{"stdout-EOF-total", "exec 1>&-; sleep 2", 50},
40+
};
41+
auto exercise = [](process::Streams streams, const Case& test) {
42+
SCOPED_TRACE(std::format("stream={} case={}", static_cast<int>(streams), test.name));
43+
const auto started = std::chrono::steady_clock::now();
44+
process::Options options;
45+
options.streams = streams;
46+
options.total = std::chrono::milliseconds(test.total);
47+
options.idle = std::chrono::milliseconds(test.idle);
48+
if (test.stop) options.should_stop = [&] {
49+
return std::chrono::steady_clock::now() - started > std::chrono::milliseconds(50);
50+
};
51+
const auto result = process::execute({"/bin/sh", "-c", test.command}, options);
52+
if (test.exit >= 0) {
53+
EXPECT_EQ(result.exit_code, test.exit);
54+
EXPECT_FALSE(result.timed_out);
55+
} else {
56+
EXPECT_NE(result.exit_code, 0);
57+
EXPECT_TRUE(result.timed_out);
58+
}
59+
EXPECT_LT(std::chrono::steady_clock::now() - started, std::chrono::seconds(1));
60+
};
61+
for (const auto streams : {process::Streams::inherit, process::Streams::discard,
62+
process::Streams::merged, process::Streams::output,
63+
process::Streams::output_error_shown, process::Streams::output_error_kept}) {
64+
for (const auto& test : cases) exercise(streams, test);
65+
if (streams != process::Streams::inherit && streams != process::Streams::discard)
66+
exercise(streams, {"both-EOF-idle", "exec 1>&- 2>&-; sleep 2", 0, 50});
67+
}
68+
exercise(process::Streams::output_error_kept,
69+
{"stderr-activity", "exec 1>&-; for i in 1 2 3 4 5 6 7 8; do printf x >&2; sleep .03; done; exit 7",
70+
0, 100, false, 7});
71+
}
72+
73+
TEST(RunExec, StopPredicateAppliesAfterCapturedStreamsClose) {
74+
const auto started = std::chrono::steady_clock::now();
75+
const auto result = process::execute({"/bin/sh", "-c", "exec 1>&- 2>&-; sleep 2"}, {
76+
.streams = process::Streams::merged,
77+
.should_stop = [&] {
78+
return std::chrono::steady_clock::now() - started > std::chrono::milliseconds(50);
79+
}});
80+
EXPECT_TRUE(result.timed_out);
81+
EXPECT_NE(result.exit_code, 0);
82+
EXPECT_LT(std::chrono::steady_clock::now() - started, std::chrono::seconds(1));
83+
}
84+
2285
TEST(RunExec, StopPredicateAppliesWithoutCapturedStreams) {
2386
for (const auto streams : {process::Streams::inherit, process::Streams::discard}) {
2487
const auto started = std::chrono::steady_clock::now();

0 commit comments

Comments
 (0)