Skip to content

A build that never finishes is stopped at its timeout, and lets go of the build lock - #114

Open
Bluesfire wants to merge 1 commit into
n-delic:mainfrom
Bluesfire:fix/build-timeout
Open

Bluesfire wants to merge 1 commit into
n-delic:mainfrom
Bluesfire:fix/build-timeout

Conversation

@Bluesfire

Copy link
Copy Markdown

Requested by Bluesfire

Type: bug fix

Summary

ForkProject.run read the build's output to the end, and only then called proc.wait(timeout=...). A compiler
that never finishes keeps the pipe open, so the loop never ended and the timeout was never looked at. On a GameCube
game, an agent compiled a function with a #pragma scheduling value that CodeWarrior 2.5 loops on (100% CPU, no
end). That build held the project's build lock for 8½ hours: every compile_and_diff after it waited behind it, and
the agents' MCP calls timed out at 300 s one after another. That made 61 helper sessions with 0 compiles, and about
$7.50 of Claude usage plus 14M Codex tokens spent on nothing. The change was never undone either, so after a restart
the next compile of that file froze the app again, two minutes in.

 ForkProject.run(cmd, timeout)
-  for line in proc.stdout: ...            # until the build closes its output
-  proc.wait(timeout)                      # only reached once it has
-  timed out: proc.kill(), "timed out"     # ninja only; its compiler keeps the pipe
+  a thread reads proc.stdout, as before (log, failed, quiet)
+  proc.wait(timeout)                      # counts while the build runs
+  timed out: stop_process_tree(proc)      # ninja and the compiler it started
+             the output so far + "stopped: the build didn't finish in 600 seconds, ... change the code that was
+             just added."
+  ended: wait up to BUILD_OUTPUT_GRACE (5 s) for the rest of its output

compile_and_diff already treats a build that fails as a BUILD ERROR and undoes the change, so the agent's code
goes and the lock is free again.

Evidence

  • Before (upstream main): last night's file, built by a parent process that starts mwcceppc.exe as ninja
    does, with timeout=20: run() had not returned after 88.8 s, when I stopped it from outside, and nothing said
    why.

    After:

    same file, timeout=20 -> returned after 20.7 s, ok=False
      tail: "stopped: the build didn't finish in 20 seconds, so it was stopped with everything it started. ..."
      mwcceppc.exe processes left running: 0
    tests/test_forks.py::test_a_build_that_never_finishes_is_stopped_at_its_timeout: fails before (returns ok after
      the 60 s sleep), passes after (about 4 s)
    
  • Full suite on Windows: per-test outcomes identical to upstream main apart from the new test (735 passed, 230
    skipped, 2 failed; the same 2 fail on main: test_when_a_usage_limit_resets_is_read_from_what_the_agent_said
    and test_the_stubs_visual_cpp_makes_are_found[7.1]).

  • uv run ruff check src tests projects: passes.

Merge Danger

Door: two-way

Blast Radius: every build ForkProject.run starts (a unit's compile, build_all, a project's setup)

A build that used to finish late but still finish now fails at its timeout (600 s for a unit, 1800 s for
build_all, 3600 s by default). One that went past those used to hang without end, so that's the only case that
changes. A Claude Code helper still gives up on its own call at 300 s, but the next compile goes through once the
600 s are up, where before none ever did. On Linux stop_process_tree sends SIGTERM, which ninja passes on to the
commands it runs. A program a build leaves running after it ends (a compiler's helper server) can't hold run() for
more than 5 s.

🤖 Generated with Claude Code

… the build lock

ForkProject.run read the build's output to the end before it looked at the
timeout. A compiler that never finished on an agent's code (CodeWarrior on a
GameCube project, looping on one #pragma) kept ninja and the pipe open, so the
timeout was never reached: the build held the project's build lock, every
compile_and_diff after it waited behind it, and the agent's change was never
undone. Agents' calls timed out one after another for hours with no compile.

The output is now read on a thread beside the build, so the timeout counts while
the build runs. At the timeout the build is stopped with everything it started
(stop_process_tree: killing ninja alone leaves the compiler holding the pipe),
and it fails with a sentence saying so; the compile path already undoes a change
whose build fails.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the bug Something isn't working label Oct 25, 2026
@n-delic

n-delic commented Oct 26, 2026

Copy link
Copy Markdown
Owner

Thanks, this fixes the important part: a build that never finishes no longer holds the build lock, and compile_and_diff turns the timeout into a BUILD ERROR and undoes the change. It merges cleanly onto current main. A review turned up a few things to fix before it goes in:

1. On Linux and macOS the compiler survives the timeout. stop_process_tree (hostenv.py:95-108) only does proc.terminate() on POSIX. The build isn't started in a process group of its own (forks/project.py:333), so only the direct child gets the signal, and a compiler that ignores SIGTERM keeps running. Probe: a Makefile recipe starting a Python "compiler" that ignores SIGTERM, then ForkProject.run(["make"], timeout=2): run returned ok=False after 9 s, and the compiler and the build-output thread were both still alive. The timeout message says "stopped with everything it started", which is only true on Windows (taskkill /T). Suggestion: start the build with start_new_session=True and kill the group on POSIX (os.killpg). capped uses systemd-run --user --scope, which runs the command in place, so the group kill reaches it there too.

2. The reader thread can outlive run. If something the build started keeps stdout open, reader.join(BUILD_OUTPUT_GRACE) gives up, but the thread keeps running. It keeps appending to out, calling log, and extending the caller's failed list after run has returned (failed_units in app/forkengine.py:846 reads that list). Probe: a recipe that starts a helper holding stdout and exits. run returned ok=True after 5 s, and failed grew from 10 to 13 entries afterwards. Suggestion: collect failed locally and copy it out only once the reader is done, and close or stop the reader after the grace period so nothing logs after run returns.

3. The test doesn't cover "everything it started". build.py is one sleeping process, so the test passes with a plain proc.kill() and wouldn't catch 1. Having the test's build start a child that holds stdout, and asserting both PIDs are gone, would. (The test does fail on main as it should: 60 s, then assert not True.)

Smaller, fine as follow-ups:

  • The timeout sentence ("change the code that was just added") is also what reconfigure, build_all and setup return, where the last code change may not be the cause. A neutral sentence, or one per caller, would fit better.
  • Setup, Check again and link ignore build_all's result (this was already the case before this PR). A timeout there now passes silently, because the sentence is only in the returned tail and never logged.
  • Cancelling a fork job now waits for the build or its timeout, since check_cancel raises on the reader thread while the job's thread sits in proc.wait.

Same "read to EOF, then wait with no timeout" pattern elsewhere, not for this PR: app/updates.py:239, app/routes/setup.py:64, app/agents.py:1110, app/winhelper.py:17, rom/export_build.py:65, rom/export.py:130.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants