Skip to content

child_output_with_timeout never drains child pipes until exit, so a chatty child deadlocks until timeout #516

Closed

Description

@thcp

child_output_with_timeout does not drain the child's pipes until after the child has already exited, so a child that fills a pipe buffer deadlocks until the timeout kills it.

The defect

desktop/src-tauri/src/main.rs:1396-1430:

loop {
    if let Some(status) = child.try_wait()? {
        let mut stdout = Vec::new();
        if let Some(mut pipe) = child.stdout.take() { let _ = pipe.read_to_end(&mut stdout); }
        let mut stderr = Vec::new();
        if let Some(mut pipe) = child.stderr.take() { let _ = pipe.read_to_end(&mut stderr); }
        return Ok(Output { status, stdout, stderr });
    }
    if Instant::now() >= deadline { ... }
    thread::sleep(Duration::from_millis(100));
}

read_to_end is only reachable after try_wait() returns Some. While the child runs, nothing reads the pipes.

warmup_models sets both .stdout(Stdio::piped()) and .stderr(Stdio::piped()) (main.rs:1414-1415). Once the child fills the OS pipe buffer it blocks in write(), so it never exits, so try_wait() never returns Some -- and after the 30-minute timeout it is killed and warmup_models returns Err.

command_output_with_timeout (main.rs:4368) has the same shape.

How likely is the trigger

Being precise, because this was measured rather than assumed:

  • A real cold-cache warmup run on a fast connection emitted 8,360 bytes of stderr in total. Comfortably under any pipe buffer; it completed without deadlocking.
  • The volume is dominated by tqdm progress refreshes from torch.hub.download_url_to_file for Demucs, beat-this, allin1 and audio-separator. tqdm refresh count scales with download duration, not size, so a slow link produces many times more output for the same models.

So: the defect is confirmed, but I have not reproduced the deadlock. It requires enough output to fill the buffer, which a slow connection plausibly reaches and a fast one does not. That makes it a latent bug that bites the users least able to diagnose it -- the ones on slow links, whom warmup exists to help.

run_pip_install (main.rs:2254) is lower risk (--quiet, progress to a null'd stdout). verify_cuda_torch (main.rs:2407) is safe because Command::output() drains concurrently.

Fix

Use Command::output() where a timeout is not needed, and where it is, spawn reader threads for stdout and stderr (or use wait_with_output on a timer thread) so the pipes drain concurrently with execution.

Test

Spawn a child that writes several hundred KB to stderr and exits, and assert child_output_with_timeout returns promptly with the full output rather than timing out.

Metadata

Metadata

Assignees

Labels

bugSomething isn't workingreliabilityStability, resource management, race conditions

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions