python3.13 likely remains stuck in a busy-poll loop. A diagnosis report and patch proposal were created locally.
Published session
Prompt
## Plan Pass
You are planning a fixer patch before any edits happen.
Read the evidence bundle at `./evidence.json`. The prepared workspace is `./workspace` and it was acquired via `debian-source`. The original pre-edit snapshot is available at `./source` if you need to inspect it. For interpreter processes, plan from the script/application entrypoint evidence first and include the runtime only as a second investigation target unless the evidence proves a runtime bug.
Upstream-style expectation: before planning or editing, check for contribution/style docs (`CONTRIBUTING`, `HACKING`, `README-hacking`, `README.md`, `docs/`, `dev-docs/`) and scan the touched subsystem for local helpers. If the project has wrappers for file IO, path-relative IO, process spawning, memory allocation, logging, locking, or platform compatibility, prefer those wrappers over generic libc/std APIs. Do not invent a reproducer or user-visible failure that is not in the evidence bundle; if the evidence is profiler-only or indirect, describe it as a targeted mitigation or stop with a diagnosis instead of presenting a speculative patch as a confirmed bug fix. In the plan and final validation, name any such helper, convention, or evidence limit you found, or say that no relevant local helper was found. Treat this as a `supervisor` upstream patch, not just a Debian-local workaround. Inspect the relevant code, nearby callers, project contribution docs, and local helper/compat APIs, but do not edit files in this pass.
Return a short markdown plan with these exact sections:
## Problem
## Evidence Confidence
## Proposed Subject
## Patch Plan
## Risks
## Validation
Classify `## Evidence Confidence` as exactly one of `reproduced`, `observed`, or `inferred`. Use `inferred` only for a no-patch diagnosis/report plan unless you can name the extra evidence you will collect before editing; inferred source patches are blocked by Fixer because they are not pull-request-ready. For `observed` source-patch plans, plan to say in the final `## Issue Connection` that the failure was observed by Fixer and not independently reproduced. The plan must explain how the proposed code change addresses the observed issue evidence, call out any prior Fixer patch that should be improved or replaced, reject awkward control flow such as avoidable `goto` if there is a cleaner bounded alternative, name any local helper APIs or maintainer conventions the patch should follow, and keep the intended maintainer-facing explanation clear enough that someone unfamiliar with the local complaint wording can still follow the fix. In `## Validation`, name the reproducible configure/build/test entrypoint you will try from the workspace root before any focused leaf compile or smoke check, and include one bounded independent reproduction attempt for the collected failure signal when it is safe and cheap. Do not plan to claim `reproduced` unless that reproduction command or test can actually show the failure.
## Patch Pass
You are working on a bounded fixer proposal.
Read the evidence bundle at `./evidence.json`. The prepared workspace is `./workspace` and it was acquired via `debian-source`. Produce the smallest reasonable patch for the target repository, keep the change upstreamable, prefer the clearest control flow available, and do not keep avoidable `goto` when a simpler structure would read better. Before introducing new file, process, allocation, locking, networking, or platform APIs, inspect nearby code and project contribution docs for existing helpers or compatibility wrappers and use those local patterns unless you can explain why they do not fit. Validate from a reproducible workspace-root entrypoint before falling back to focused leaf commands; if a build or test cannot run, report the exact command, the exact blocker, and any narrower check you ran instead. During validation, also try one bounded independent reproduction of the collected failure signal when it is safe and cheap, such as a failing test, smoke command, perf/strace comparison, or before/after runtime check. Only use `reproduced` if that command or test actually reproduced the failure; otherwise keep `observed` and report the reproduction blocker. The final explanation must connect the observed issue evidence to the actual code change, not just paraphrase the diff. Write like a maintainer is going to read the patch mail cold: explain the bug in plain language, define subsystem-specific jargon the first time you need it, and make the causal story obvious. Explicitly classify evidence confidence as `reproduced`, `observed`, or `inferred`: `reproduced` means you reproduced the failure locally; `observed` means Fixer has direct crash/log/trace evidence but you did not independently reproduce it; `inferred` means the source patch is not pull-request-ready, so do not leave a source diff unless you first gather stronger observed/reproduced evidence; otherwise return a no-patch diagnosis/report. For any source-changing `observed` patch, say explicitly in `## Issue Connection` that the failure was observed by Fixer and not independently reproduced. If you introduce non-obvious state translation, index remapping, or backend split logic, add a short source comment that explains the invariant being preserved.
Start by explaining the likely root cause from the collected perf, strace, and /proc evidence. If you cannot land a safe patch, leave a diagnosis that is strong enough for an upstream bug report.
Interpreter process expectation: when the hot process is Python, Perl, shell, Node, Ruby, PHP, Lua, or another interpreter, inspect both the runtime and the script/application entrypoint from `details.interpreter_process`. Prefer the script or application package when evidence points there; do not patch the interpreter runtime merely because the hot binary is `python`, `perl`, `bash`, or similar. Runtime fixes are welcome only after checking the entrypoint and explaining why the runtime mishandles the workload or why running bad code faster/safer is itself the right upstream fix.
Upstream-style expectation: before planning or editing, check for contribution/style docs (`CONTRIBUTING`, `HACKING`, `README-hacking`, `README.md`, `docs/`, `dev-docs/`) and scan the touched subsystem for local helpers. If the project has wrappers for file IO, path-relative IO, process spawning, memory allocation, logging, locking, or platform compatibility, prefer those wrappers over generic libc/std APIs. Do not invent a reproducer or user-visible failure that is not in the evidence bundle; if the evidence is profiler-only or indirect, describe it as a targeted mitigation or stop with a diagnosis instead of presenting a speculative patch as a confirmed bug fix. In the plan and final validation, name any such helper, convention, or evidence limit you found, or say that no relevant local helper was found. Treat this as a `supervisor` upstream patch, not just a Debian-local workaround.
Keep the change narrowly scoped and summarize validation clearly.
In every authoring pass, your final response must start with `Subject: <single-line git commit subject>` and then include these markdown sections exactly:
## Commit Message
A short upstream-friendly explanation of what changed and why. Write it in plain language that a maintainer can follow without local complaint context. If you use subsystem jargon, define it immediately.
## Evidence Confidence
Exactly one word: `reproduced`, `observed`, or `inferred`. Use `reproduced` only when you reproduced the failure locally with a command or test, and include that command/test in `## Validation`. Use `observed` when Fixer has direct crash/log/trace evidence but you did not independently reproduce it. If `## Git Add Paths` lists source files for an `observed` patch, `## Issue Connection` must explicitly say the failure was observed by Fixer and not independently reproduced. Use `inferred` for profiler/strace/indirect evidence; inferred responses may be no-patch diagnoses or reports, but inferred source patches are not pull-request-ready until stronger evidence is gathered.
## Issue Connection
Write this as maintainer-facing patch mail, not as local Fixer notes. Cover four things explicitly in readable sentences: the user-visible symptom or the exact collected signal, the code-level cause or the cautious inference from evidence, the specific change you made, and the expected effect. Do not invent a reproducer, command line, crash, or user-visible failure that is not present in the evidence bundle. If the evidence is direct-but-not-reproduced, say it was observed by Fixer and not independently reproduced. If the evidence is indirect and you did not gather stronger evidence, do not leave a source diff; write a no-patch diagnosis/report instead. Include an explicit effect sentence such as `The expected effect is ...`, `This should reduce ...`, or `This prevents ...` for source patches. If the logic is non-obvious in code, mention that you added a short explanatory comment.
## Git Add Paths
List the repo-relative paths that belong in the final patch, one per line. Use `None` only when you intentionally made no source changes. Include intentionally new files, and do not list generated build artifacts.
## Validation
List the checks you ran, or say clearly that you could not run them. Include the independent reproduction command/test and result when `## Evidence Confidence` is `reproduced`; if reproduction was attempted but blocked, name the exact blocker and keep confidence at `observed` or `inferred`.
Before editing, read the plan at `./plan-output.txt` and follow it unless the code proves part of it wrong. If you change course, say so explicitly in the final write-up instead of silently drifting from the plan.
## Review Pass 1
You are reviewing a freshly generated fixer patch.
Read the evidence bundle at `./evidence.json`. The prepared workspace is `./workspace` and it was acquired via `debian-source`. Review the first patch pass. The original pre-edit snapshot is available at `./source` for diffing. The workspace currently changes these repo-relative paths: supervisor/supervisord.py, supervisor/tests/test_supervisord.py. Verify that `## Git Add Paths` matches this exact set and that `## Issue Connection` explains every shipped functional file.
Upstream-style expectation: before planning or editing, check for contribution/style docs (`CONTRIBUTING`, `HACKING`, `README-hacking`, `README.md`, `docs/`, `dev-docs/`) and scan the touched subsystem for local helpers. If the project has wrappers for file IO, path-relative IO, process spawning, memory allocation, logging, locking, or platform compatibility, prefer those wrappers over generic libc/std APIs. Do not invent a reproducer or user-visible failure that is not in the evidence bundle; if the evidence is profiler-only or indirect, describe it as a targeted mitigation or stop with a diagnosis instead of presenting a speculative patch as a confirmed bug fix. In the plan and final validation, name any such helper, convention, or evidence limit you found, or say that no relevant local helper was found. Treat this as a `supervisor` upstream patch, not just a Debian-local workaround. The latest author response is at `./patch-output.txt`. Inspect the current code and changed paths like a strict code reviewer. Focus on correctness, regressions, maintainability, awkward control flow such as avoidable `goto`, missing validation, weak or non-gittable commit message text, and explanations that fail to connect the observed issue evidence to the code change. Also review the maintainer experience: the patch mail should be easy to accept upstream, the user-visible bug should be explained in plain language, subsystem-specific jargon should be defined when first used, and any non-obvious state translation or index remapping in code should have a short explanatory comment. Reject patches that omit `## Evidence Confidence`, use anything other than `reproduced`, `observed`, or `inferred`, or sound more certain than that evidence level allows. Reject patches that claim `reproduced` unless `## Validation` names the concrete reproduction command or test and its result. Reject patches that invent a reproducer, command line, crash, or user-visible failure not present in the evidence bundle; indirect profiler or strace evidence may justify a cautious mitigation, but the author must say it is indirect instead of presenting it as a confirmed upstream bug. Reject patches that introduce generic libc/std APIs when nearby code or project docs provide a local compat/helper API for the same job, unless the author explicitly justifies the exception. Reject validation that only reports a leaf object/syntax build when a project-level configure/build/test entrypoint exists and was not attempted; if a full build cannot run, the author must show the exact failed command and blocker.
Do not apply code changes in this pass.
Return a short markdown review report. The first non-empty line must be exactly one of:
RESULT: ok
RESULT: fix-needed
If you choose `RESULT: fix-needed`, add a `## Findings` section with concrete, actionable items.
## Refinement Pass 1
You are refining a fixer patch after an explicit code review.
Read the evidence bundle at `./evidence.json`. The prepared workspace is `./workspace` and it was acquired via `debian-source`. Read the latest author response at `./patch-output.txt`. Read the review report at `./review-1-output.txt`. This is refinement round 1. The original pre-edit snapshot is available at `./source` if you need to compare the current patch against it. Re-read the planning pass at `./plan-output.txt` before editing. The workspace currently changes these repo-relative paths: supervisor/supervisord.py, supervisor/tests/test_supervisord.py. Either keep that exact set synchronized with `## Git Add Paths` and `## Issue Connection`, or revert any unintended file before you answer.
Upstream-style expectation: before planning or editing, check for contribution/style docs (`CONTRIBUTING`, `HACKING`, `README-hacking`, `README.md`, `docs/`, `dev-docs/`) and scan the touched subsystem for local helpers. If the project has wrappers for file IO, path-relative IO, process spawning, memory allocation, logging, locking, or platform compatibility, prefer those wrappers over generic libc/std APIs. Do not invent a reproducer or user-visible failure that is not in the evidence bundle; if the evidence is profiler-only or indirect, describe it as a targeted mitigation or stop with a diagnosis instead of presenting a speculative patch as a confirmed bug fix. In the plan and final validation, name any such helper, convention, or evidence limit you found, or say that no relevant local helper was found. Treat this as a `supervisor` upstream patch, not just a Debian-local workaround. Address the review findings with the smallest reasonable follow-up changes. If the review identifies a runtime or correctness bug in the changed code, you must update the code itself before answering; a metadata-only response is not sufficient. Keep the patch upstream-friendly, use local project helpers and compat APIs when available, avoid awkward control flow when a simpler structure will do, keep the final response gittable, make the maintainer-facing explanation plain and direct, keep `## Evidence Confidence` honest, add short comments for any non-obvious translation or remapping logic, run relevant project-level build/tests from the workspace root before narrower checks when possible, try a bounded independent reproduction command/test if the review or plan shows one is safe, and summarize which review findings you addressed.
In every authoring pass, your final response must start with `Subject: <single-line git commit subject>` and then include these markdown sections exactly:
## Commit Message
A short upstream-friendly explanation of what changed and why. Write it in plain language that a maintainer can follow without local complaint context. If you use subsystem jargon, define it immediately.
## Evidence Confidence
Exactly one word: `reproduced`, `observed`, or `inferred`. Use `reproduced` only when you reproduced the failure locally with a command or test, and include that command/test in `## Validation`. Use `observed` when Fixer has direct crash/lo
[truncated 13828 bytes]
Response
## Plan Pass
## Problem
Fixer observed `supervisord` running as `/usr/bin/python3 /usr/bin/supervisord -n -c /etc/supervisor/supervisord.conf` with high CPU attribution in Python, plus a syscall pattern of `poll(..., 1000)` timeouts followed by `wait4(-1, ..., WNOHANG) = -1 ECHILD`. The evidence does not prove a Python runtime bug or a zero-time poll spin; it does show Supervisor repeatedly attempting to reap children when the kernel reports there are none.
## Evidence Confidence
observed
## Proposed Subject
Avoid idle `waitpid` calls when Supervisor has no tracked child processes
## Patch Plan
- Treat this as an upstream Supervisor patch in `supervisor/supervisord.py`, not a Debian-local workaround.
- Use the existing local wrappers and conventions: keep child reaping through `ServerOptions.waitpid()` rather than calling `os.waitpid()` directly, and leave polling through `supervisor.poller.Poller`.
- Add a small linear guard in the main loop before the unconditional `self.reap()` so idle Supervisor instances do not call `waitpid` when `options.pidhistory` is empty. Avoid awkward control flow; a bounded `if self.options.pidhistory: self.reap()` is enough.
- Add focused `unittest` coverage in `supervisor/tests/test_supervisord.py` showing `runforever()` skips reaping with an empty `pidhistory`, and still reaps when `pidhistory` contains a tracked child.
- No prior Fixer patch is present to replace or improve: the worktree has no source diff, `.pc/applied-patches` is empty, and no `debian/patches` files were found.
- Maintainer-facing explanation should be careful: this is a targeted idle-loop mitigation for the observed `ECHILD` reaping cadence, not a claimed reproduction of the reported 99% CPU profile or the `PyDict_SetItem` sample.
## Risks
- Skipping `reap()` when `pidhistory` is empty could hide unexpected unmanaged children, but Supervisor’s normal process lifecycle records spawned children in `pidhistory` immediately after fork.
- The patch may reduce unnecessary syscalls and Python loop work without fully explaining the profiler’s `PyDict_SetItem` hotspot.
- Need to preserve shutdown and RPC paths that intentionally call `reap()`; keep the change scoped to the periodic main-loop call.
## Validation
- First run the reproducible project entrypoint from the workspace root: `tox -e py313 -- supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py` if `tox` is available; otherwise `python3 -m pytest --capture=no supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`.
- Run the full available test suite with `python3 -m pytest --capture=no` if dependencies are present.
- Bounded independent signal check: run an idle foreground Supervisor with `supervisor/tests/fixtures/donothing.conf` under `timeout`/`strace -e poll,wait4` before and after the patch, and verify the post-patch idle case no longer emits repeated `wait4(...)=ECHILD` while `poll(..., 1000)` still sleeps normally.
- In the final `## Issue Connection`, state that the failure signal was observed by Fixer and not independently reproduced as a CPU runaway.
## Patch Pass
Subject: Skip idle child reaping when no child PIDs are tracked
## Commit Message
Supervisor’s main loop reaped children on every idle tick, even when it had no tracked child processes. Since reaping uses `waitpid`, this caused unnecessary nonblocking child-status checks in idle configurations. Check `pidhistory`, Supervisor’s map of known child PIDs, before doing the periodic reap.
## Evidence Confidence
observed
## Issue Connection
Fixer observed a `supervisord` Python entrypoint with high CPU attribution and a trace alternating between `poll(..., 1000)` and `wait4(-1, ..., WNOHANG) = -1 ECHILD`, where `ECHILD` means the kernel had no child process status to return. The evidence was observed by Fixer and not independently reproduced as a CPU runaway here.
The cautious code-level cause is that `Supervisor.runforever()` called `self.reap()` unconditionally after every poll cycle. `reap()` correctly uses the local `ServerOptions.waitpid()` wrapper around `os.waitpid`, but when `options.pidhistory` is empty Supervisor has no managed child PID it expects to reap, so the periodic call can only produce the idle no-child path seen in the trace.
The change keeps the existing wrapper and main-loop structure, but calls `self.reap()` only when `pidhistory` is non-empty. The expected effect is to remove unnecessary idle `waitpid`/`wait4` calls while preserving normal child reaping when Supervisor has recorded child PIDs. I followed the existing upstream style from `docs/development.rst` and nearby `unittest` patterns; no new platform API or runtime patch was introduced.
## Git Add Paths
supervisor/supervisord.py
supervisor/tests/test_supervisord.py
## Validation
- `tox -e py313 -- supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`: could not run because `tox` is not installed (`/bin/bash: line 1: tox: command not found`).
- `PYTHONDONTWRITEBYTECODE=1 python3 -m pytest --capture=no supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`: passed, `63 passed`.
- `python3 -m pytest --capture=no`: ran from the workspace root; `1362 passed`, `10 failed`. The failures are unrelated socket-binding tests blocked by the sandbox with `PermissionError: [Errno 1] Operation not permitted`.
- Independent signal attempt with `strace -f -e trace=poll,wait4 ...`: blocked by sandbox ptrace restrictions: `PTRACE_TRACEME: Operation not permitted`.
- Bounded Python-level before/after check from `/tmp`: original snapshot called the instrumented idle `waitpid` path once; patched workspace called it zero times.
## Review Pass 1
RESULT: fix-needed
## Findings
- [supervisor/supervisord.py](./workspace/supervisor/supervisord.py:258): The new `if self.options.pidhistory:` guard changes more than the observed idle `ECHILD` case. `reap()` already has an explicit “unknown pid” path and test coverage for logging/reaping a waitable child that is not in `pidhistory`; with this guard, the main loop will not call `waitpid()` at all when `pidhistory` is empty, so an unexpected waitable child in that state would not be reaped by the normal loop. Either preserve the existing unknown-child reaping behavior, or add a maintainer-grade justification/test proving that an empty `pidhistory` is a valid invariant for “no waitable children” in `runforever()`.
- [./patch-output.txt](./patch-output.txt:24): The validation section does not report the exact independent reproduction command; it uses `strace -f -e trace=poll,wait4 ...`. The instructions require the exact blocked command and exact blocker. Replace the ellipsis with the actual attempted `timeout/strace/python -m supervisor.supervisord ...` command so the validation is auditable.
Git Add Paths matches the actual changed set exactly: `supervisor/supervisord.py` and `supervisor/tests/test_supervisord.py`. The evidence confidence is present and valid (`observed`), and the patch mail appropriately avoids claiming the CPU runaway was independently reproduced.
## Refinement Pass 1
Subject: Skip idle child reaping unless child work is pending
## Commit Message
Supervisor’s main loop reaped children on every idle tick. Reaping uses `waitpid`, the POSIX call that checks for exited child processes, so idle configurations could repeatedly ask the kernel for child status even when Supervisor had no child PID to track. Only run the periodic reap when Supervisor has tracked child PIDs or has received `SIGCHLD`, the child-exited signal.
## Evidence Confidence
observed
## Issue Connection
Fixer observed a `supervisord` Python entrypoint with high CPU attribution and a trace alternating between `poll(..., 1000)` and `wait4(-1, ..., WNOHANG) = -1 ECHILD`; `ECHILD` means the kernel had no child process status to return. The failure signal was observed by Fixer and not independently reproduced as a CPU runaway.
The cautious code-level inference is that `Supervisor.runforever()` called `self.reap()` after every poll cycle. `reap()` correctly goes through Supervisor’s local `ServerOptions.waitpid()` wrapper, but doing that with no tracked child PID causes the no-child path seen in the trace.
The change keeps the local wrapper and main-loop conventions, but fetches the pending signal once and reaps only when `pidhistory` has tracked child PIDs or the pending signal is `SIGCHLD`. This refinement addresses the review finding by preserving the existing unknown-pid cleanup path: a `SIGCHLD` can still trigger `reap()` even if that PID is missing from `pidhistory`. I added a short code comment for that invariant and tests for the idle skip, normal tracked-child reap, and SIGCHLD unknown-child case.
The expected effect is to reduce unnecessary idle `waitpid`/`wait4` calls without suppressing normal managed-child or unknown-child reaping. The touched test file documents the shipped behavior.
## Git Add Paths
supervisor/supervisord.py
supervisor/tests/test_supervisord.py
## Validation
- `tox -e py313 -- supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`: could not run because `tox` is not installed: `/bin/bash: line 1: tox: command not found`.
- `PYTHONDONTWRITEBYTECODE=1 python3 -m pytest --capture=no supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`: passed, `64 passed`.
- `timeout 4s strace -f -e trace=poll,wait4 -o /tmp/supervisor-fixer-after.log python3 -m supervisor.supervisord -c supervisor/tests/fixtures/donothing.conf -n >/tmp/supervisor-fixer-after.out 2>/tmp/supervisor-fixer-after.err`: blocked by sandbox ptrace restrictions: `PTRACE_TRACEME: Operation not permitted` and `ptrace(PTRACE_SEIZE, 6): Operation not permitted`.
- Python-level before/after check from `/tmp`: original snapshot called the instrumented idle `waitpid` path once; patched workspace called it zero times.
- `PYTHONDONTWRITEBYTECODE=1 python3 -m pytest --capture=no`: ran from the workspace root; `1363 passed`, `10 failed`. The failures are unrelated socket-binding tests blocked by the sandbox with `PermissionError: [Errno 1] Operation not permitted`.
- `git diff --check`: passed.
## Review Pass 2
RESULT: fix-needed
## Findings
- [supervisor/supervisord.py](./workspace/supervisor/supervisord.py:306): `handle_signal(sig=None)` still calls `options.get_signal()` when the caller explicitly passes `None`. In `runforever()`, the code first calls `get_signal()` for the reap decision, then calls `handle_signal(sig)`. If no signal is pending at line 258 but a `SIGCHLD` arrives before line 307, `handle_signal(None)` can pop and log that `SIGCHLD` after the reap decision has already been skipped; with empty `pidhistory`, the unknown-child reap path can then be missed on the next loop because the signal was consumed. Use a sentinel or separate helper so `runforever()` can pass the already-fetched value, including `None`, without a second signal read; keep the no-argument `handle_signal()` behavior for existing direct callers if needed.
Git Add Paths matches the actual changed set exactly: `supervisor/supervisord.py` and `supervisor/tests/test_supervisord.py`. The refined Issue Connection explains both the functional code and test coverage, and the validation now includes the exact blocked `strace` command.
## Refinement Pass 2
Subject: Skip idle child reaping unless child work is pending
## Commit Message
Supervisor’s main loop reaped children on every idle tick. Reaping uses `waitpid`, the POSIX call that checks for exited child processes, so idle configurations could repeatedly ask the kernel for child status even when Supervisor had no child PID to track. Only run the periodic reap when Supervisor has tracked child PIDs or has received `SIGCHLD`, the child-exited signal.
## Evidence Confidence
observed
## Issue Connection
Fixer observed a `supervisord` Python entrypoint with high CPU attribution and a trace alternating between `poll(..., 1000)` and `wait4(-1, ..., WNOHANG) = -1 ECHILD`; `ECHILD` means the kernel had no child process status to return. The failure signal was observed by Fixer and not independently reproduced as a CPU runaway.
The cautious code-level inference is that `Supervisor.runforever()` called `self.reap()` after every poll cycle. `reap()` correctly goes through Supervisor’s local `ServerOptions.waitpid()` wrapper, but doing that with no tracked child PID causes the no-child path seen in the trace.
The change keeps the local wrapper and main-loop conventions from the touched subsystem, but fetches the pending signal once and reaps only when `pidhistory` has tracked child PIDs or the pending signal is `SIGCHLD`. A private sentinel keeps `handle_signal(None)` from reading the signal queue a second time, which addresses the round-2 review finding. The existing no-argument `handle_signal()` behavior is preserved for direct callers.
This also preserves the existing unknown-pid cleanup path: a `SIGCHLD` can still trigger `reap()` even if that PID is missing from `pidhistory`. I kept the short code comment for that invariant and added tests for the idle skip, single signal read, SIGCHLD unknown-child reap, and normal tracked-child reap.
The expected effect is to reduce unnecessary idle `waitpid`/`wait4` calls without suppressing normal managed-child or unknown-child reaping. I checked `docs/development.rst` and nearby tests; no new platform API or runtime patch was introduced.
## Git Add Paths
supervisor/supervisord.py
supervisor/tests/test_supervisord.py
## Validation
- `tox -e py313 -- supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`: could not run because `tox` is not installed: `/bin/bash: line 1: tox: command not found`.
- `PYTHONDONTWRITEBYTECODE=1 python3 -m pytest --capture=no supervisor/tests/test_supervisord.py supervisor/tests/test_poller.py`: passed, `65 passed`.
- `timeout 4s strace -f -e trace=poll,wait4 -o /tmp/supervisor-fixer-after.log python3 -m supervisor.supervisord -c supervisor/tests/fixtures/donothing.conf -n >/tmp/supervisor-fixer-after.out 2>/tmp/supervisor-fixer-after.err`: blocked by sandbox ptrace restrictions: `PTRACE_TRACEME: Operation not permitted` and `ptrace(PTRACE_SEIZE, 6): Operation not permitted`.
- Python-level before/after check from `/tmp`: original snapshot called the instrumented idle `waitpid` path once; patched workspace called it zero times.
- `PYTHONDONTWRITEBYTECODE=1 python3 -m pytest --capture=no`: ran from the workspace root; `1364 passed`, `10 failed`. The failures are unrelated socket-binding tests blocked by the sandbox with `PermissionError: [Errno 1] Operation not permitted`.
- `git diff --check`: passed.
## Review Pass 3
RESULT: ok
The refined patch addresses the prior signal-consumption regression with a private sentinel, so `runforever()` can pass an already-read `None` signal without causing `handle_signal()` to read the signal queue again. The added tests cover idle skip, single signal read, SIGCHLD unknown-child reaping, and tracked-child reaping.
`## Git Add Paths` matches the actual changed set exactly:
`supervisor/supervisord.py`
`supervisor/tests/test_supervisord.py`
The `## Issue Connection` is evidence-appropriate for `observed`: it states Fixer observed the signal and that the CPU runaway was not independently reproduced, uses the existing `ServerOptions.waitpid()` wrapper, explains the functional code change, and mentions the test file behavior. Validation includes the project-level `tox` attempt and exact blocker, focused pytest, full pytest with sandbox socket failures, the exact blocked `strace` command, and `git diff --check`.
Diff
diff --git a/supervisor/supervisord.py b/supervisor/supervisord.py
index 8d9ebe3..aef48d3 100755
--- a/supervisor/supervisord.py
+++ b/supervisor/supervisord.py
@@ -45,6 +45,8 @@ from supervisor import events
from supervisor.states import SupervisorStates
from supervisor.states import getProcessStateDescription
+_NO_SIGNAL = object()
+
class Supervisor:
stopping = False # set after we detect that we are handling a stop request
lastshutdownreport = 0 # throttle for delayed process error reports at stop
@@ -255,8 +257,12 @@ class Supervisor:
for group in pgroups:
group.transition()
- self.reap()
- self.handle_signal()
+ sig = self.options.get_signal()
+ # A SIGCHLD can refer to a child missing from pidhistory; still
+ # reap so the existing unknown-pid cleanup path can run.
+ if self.options.pidhistory or sig == signal.SIGCHLD:
+ self.reap()
+ self.handle_signal(sig)
self.tick()
if self.options.mood < SupervisorStates.RUNNING:
@@ -299,8 +305,9 @@ class Supervisor:
# infinitely
self.reap(once=False, recursionguard=recursionguard+1)
- def handle_signal(self):
- sig = self.options.get_signal()
+ def handle_signal(self, sig=_NO_SIGNAL):
+ if sig is _NO_SIGNAL:
+ sig = self.options.get_signal()
if sig:
if sig in (signal.SIGTERM, signal.SIGINT, signal.SIGQUIT):
self.options.logger.warn(
diff --git a/supervisor/tests/test_supervisord.py b/supervisor/tests/test_supervisord.py
index 4099bba..83cc1a1 100644
--- a/supervisor/tests/test_supervisord.py
+++ b/supervisor/tests/test_supervisord.py
@@ -665,6 +665,58 @@ class SupervisordTests(unittest.TestCase):
supervisord.runforever()
self.assertEqual(len(supervisord.ticks), 3)
+ def test_runforever_skips_reap_without_child_pids(self):
+ options = DummyOptions()
+ options.test = True
+ waitpid_calls = []
+
+ def waitpid():
+ waitpid_calls.append(None)
+ return None, None
+
+ options.waitpid = waitpid
+ supervisord = self._makeOne(options)
+ supervisord.runforever()
+ self.assertEqual(waitpid_calls, [])
+
+ def test_runforever_reads_signal_once_without_child_pids(self):
+ options = DummyOptions()
+ options.test = True
+ signals = [None, signal.SIGCHLD]
+
+ def get_signal():
+ return signals.pop(0)
+
+ options.get_signal = get_signal
+ supervisord = self._makeOne(options)
+ supervisord.runforever()
+ self.assertEqual(signals, [signal.SIGCHLD])
+
+ def test_runforever_reaps_sigchld_without_child_pids(self):
+ options = DummyOptions()
+ options.test = True
+ options._signal = signal.SIGCHLD
+ options.waitpid_return = 2, 0
+ supervisord = self._makeOne(options)
+ supervisord.runforever()
+ self.assertEqual(options.logger.data[0],
+ 'reaped unknown pid 2 (exit status 0)')
+
+ def test_runforever_reaps_with_child_pids(self):
+ options = DummyOptions()
+ options.test = True
+ options.pidhistory[123] = None
+ waitpid_calls = []
+
+ def waitpid():
+ waitpid_calls.append(None)
+ return None, None
+
+ options.waitpid = waitpid
+ supervisord = self._makeOne(options)
+ supervisord.runforever()
+ self.assertEqual(waitpid_calls, [None])
+
def test_runforever_poll_dispatchers(self):
options = DummyOptions()
options.poller.result = [6], [7, 8]