daemon: node stdout reader loops instead of terminating on a read error (asymmetric with stderr reader) → busy-loop / log flood on a persistent read error

[phil-opp]

Note: This issue was created by a scheduled automated Claude code-review check. It was found by randomly selecting a component (daemon/src/spawn) and reviewing it for correctness issues. Please treat the severity assessment as a starting point for human triage, not a confirmed defect.

Summary

The per-node stdout log reader and stderr log reader in binaries/daemon/src/spawn/prepared.rs are meant to be symmetric, but they handle a read error differently:

• the stderr reader sets finished = true on Err(...), so the loop terminates;
• the stdout reader sets finished = false on Err(...), so the loop keeps going.

If a node's stdout pipe ever returns a persistent (non-EOF) read error, the stdout task spins in a tight loop: each iteration read_until returns Err immediately (leaving raw empty), the task logs a Warn, pushes an empty LogLine onto the log channel, and loops again — a CPU busy-loop that also floods the daemon log and the log channel. The stderr reader, given the same condition, exits cleanly.

Affected code

binaries/daemon/src/spawn/prepared.rs

stdout reader (loops on error):

// prepared.rs:582-596
finished = match child_stdout
    .read_until(b'\n', &mut raw)
    .await
    .wrap_err_with(|| {
        format!("failed to read stdout line from spawned node {node_id}")
    }) {
    Ok(0) => true,
    Ok(_) => false,
    Err(err) => {
        logger_c
            .log(LogLevel::Warn, Some("daemon".into()), format!("{err:?}"))
            .await;
        false   // <-- keeps looping on a read error
    }
};

stderr reader (terminates on error — the intended behavior):

// prepared.rs:646-658
finished = match child_stderr
    .read_until(b'\n', &mut raw)
    .await
    .wrap_err_with(|| {
        format!("failed to read stderr line from spawned node {node_id}")
    }) {
    Ok(0) => true,
    Ok(_) => false,
    Err(err) => {
        tracing::warn!("{err:?}");
        true    // <-- terminates the loop
    }
};

Impact

• On a persistent stdout read error: an unbounded busy-loop in the stdout logging task — pegging a core, spamming the daemon log with repeated Warn lines, and flooding the log channel with empty LogLines. The task never exits until the process does.
• Severity is low in practice: most terminal conditions on a child pipe surface as Ok(0) (EOF), and transient errors like EINTR are retried by tokio internally, so a repeating non-EOF error is uncommon. But the two readers are clearly intended to behave identically, and the stdout side lacks the loop-terminating guard the stderr side has.

Suggested fix

Make the stdout error arm match the stderr arm — terminate the loop after logging:

    Err(err) => {
        logger_c
            .log(LogLevel::Warn, Some("daemon".into()), format!("{err:?}"))
            .await;
        true    // terminate, matching the stderr reader
    }

(Alternatively, if the intent is to tolerate a transient error and keep reading, both readers should share that policy with a bounded retry / backoff rather than an unconditional immediate re-read — but the simplest correct fix is to make stdout symmetric with stderr.)

Affected files

• binaries/daemon/src/spawn/prepared.rs (stdout reader error arm, ~line 590-595; cf. stderr reader ~line 654-657)

[gaumrab]

@phil-opp I went through the issue and understand the root cause now. The stdout reader keeps looping after a persistent read error, unlike the stderr reader which exits, leading to a potential busy loop and log flooding. I'd like to work on this issue and proceed with implementing the fix if that's okay.

[phil-opp]

Triage: this appears to be already fixed on current main.

In binaries/daemon/src/spawn/prepared.rs, the stdout reader's error arm now terminates the loop (returns true), symmetric with the stderr reader — exactly the suggested fix:

Err(err) => {
    logger_c
        .log(LogLevel::Warn, Some("daemon".into()), format!("{err:?}"))
        .await;
    // Stop on a persistent read error instead of looping
    // (which would busy-spin and flood the log channel).
    true
}

Both readers were also refactored to share read_capped_line, which returns the EOF flag directly, so the two arms are now structurally identical on the error path. No further code change looks necessary; this can likely be closed.

---

Generated by Claude Code

[phil-opp]

This looks already resolved on main.

Since this issue was filed (2026-07-19), the stdout/stderr log readers in binaries/daemon/src/spawn/prepared.rs were refactored (commit 0532878, merged 2026-07-24) to share a read_capped_line helper, and the stdout error arm now terminates the loop — symmetric with stderr:

// stdout reader — binaries/daemon/src/spawn/prepared.rs (~641-655)
finished = match read_capped_line(&mut child_stdout, &mut raw)
    .await
    .wrap_err_with(|| format!("failed to read stdout line from spawned node {node_id}"))
{
    Ok(eof) => eof,
    Err(err) => {
        logger_c
            .log(LogLevel::Warn, Some("daemon".into()), format!("{err:?}"))
            .await;
        // Stop on a persistent read error instead of looping
        // (which would busy-spin and flood the log channel).
        true
    }
};

The stderr reader (~706-716) returns true on Err in the same way. The busy-loop / log-flood described here no longer reproduces, so no code change is needed. Suggest closing.

---

Generated by Claude Code