fix(code-runtime-python): clear stale SIGKILL timer and clamp inherited soft rlimit

Two further review findings on the CPython backend:
- The grace-window SIGKILL timer was left armed after settlement, so on a
  normal completion a kill(-pid) could fire up to graceMs later and strike a
  recycled pgid once the kernel reused the leader's pid. settle() now clears
  the timer the moment the process group is confirmed empty (the normal path
  and when the poll sees the survivor gone), bounding the reuse window to the
  genuine-survivor case where the group cannot be empty to reuse.
- _clamped bounded rlimits by the inherited hard limit only, silently raising
  an inherited soft limit stricter than the request (loosening RLIMIT_AS or
  deferring RLIMIT_CPU SIGXCPU). It now clamps each side against its own
  inherited counterpart and pins soft under hard, keeping the strictest of
  configured and inherited. Adds an inherited-soft-limit regression test.

Agent Note expanded to seven fixes with the two new rejected alternatives;
zh pair re-recorded.
This commit is contained in:
Chinesezjc
2026-08-31 14:21:19 +08:00
committed by Tianyi Cui
parent 6cb70e6e69
commit ff604dc876
6 changed files with 84 additions and 26 deletions
@@ -534,7 +534,7 @@ def _make_error_class(name: str, member_name_property: str) -> type:
def _clamped(which: int, soft: int, hard: int) -> tuple[int, int]:
"""Bound a requested (soft, hard) rlimit pair by the inherited hard limit.
"""Bound a requested (soft, hard) rlimit pair by BOTH inherited limits.
An unprivileged process may lower a hard limit but never raise it, so a
harness already started under a tighter ceiling (``ulimit -v`` below
@@ -542,13 +542,24 @@ def _clamped(which: int, soft: int, hard: int) -> tuple[int, int]:
``setrlimit`` raise ``ValueError`` and fail every run — despite the
inherited limit being STRONGER than the one requested. Clamping keeps the
stricter of the two, which still satisfies the containment contract.
``RLIM_INFINITY`` compares as -1, so it is special-cased rather than
treated as the smallest bound.
Both inherited bounds matter, not just the hard one. A deployment that
inherited a soft limit BELOW what is requested (e.g. inherited ``(100, 200)``,
requested ``(150, 160)``) must keep the stricter soft — returning the
requested ``150`` would RAISE the effective soft limit, loosening RLIMIT_AS
memory or deferring the RLIMIT_CPU SIGXCPU, the opposite of "strictest of
configured and inherited". So each side is clamped against its inherited
counterpart. ``RLIM_INFINITY`` compares as -1, so an infinite inherited bound
imposes no ceiling and the requested value stands.
"""
inherited = resource.getrlimit(which)[1]
if inherited == resource.RLIM_INFINITY:
return (soft, hard)
return (min(soft, inherited), min(hard, inherited))
inherited_soft, inherited_hard = resource.getrlimit(which)
clamped_soft = soft if inherited_soft == resource.RLIM_INFINITY else min(soft, inherited_soft)
clamped_hard = hard if inherited_hard == resource.RLIM_INFINITY else min(hard, inherited_hard)
# setrlimit requires soft <= hard. Clamping the two sides independently can
# invert them (a finite inherited soft below the clamped hard is fine, but a
# requested hard below the inherited soft would leave soft > hard), so pin
# soft under hard as the final step; the stricter hard ceiling wins.
return (min(clamped_soft, clamped_hard), clamped_hard)
# ---------------------------------------------------------------------------
@@ -1064,24 +1064,27 @@ export class PythonCodeRuntime extends CodeRuntime {
}
resolve({ ...result, logs })
// `finished` is what teardown awaits to honor "no subprocess outlives the
// fiber". When no escalation ran (normal completion, no kill) or the group
// is already empty, resolve it now. Otherwise a same-group descendant that
// ignored SIGTERM but released the pipes is still alive here (its `close`
// is what got us to settle); withhold `finished` until the grace-window
// SIGKILL has emptied the group. The poll timers are REF'd on purpose: a
// short-lived host (a one-shot headless run, a config subprocess) would
// otherwise exit before the unref'd SIGKILL timer fired, reparenting the
// survivor to init — the leak this await exists to prevent. The wait is
// bounded by the same graceMs + margin the SIGKILL escalation uses, so a
// truly unreapable process (it cannot be, since it is in the group
// `kill(-pid)` reaches) could not hang disposal.
// fiber". When no escalation ran (normal completion, no kill) or the
// group is already empty, cancel the pending SIGKILL and resolve now.
// Clearing it is what bounds the PID-reuse hazard: an armed `kill(-pid)`
// left to fire up to graceMs later could hit a RECYCLED pgid once the
// kernel reused the leader's pid, SIGKILLing an unrelated group. So the
// timer stays armed only while a real survivor exists — a same-group
// descendant that ignored SIGTERM but released the pipes, still alive
// here because its `close` is what got us to settle. In that case
// withhold `finished` and poll the group on REF'd timers (a short-lived
// host would otherwise exit before the unref'd SIGKILL fired, reparenting
// the survivor to init), clearing the timer the moment the group empties;
// the wait is bounded by the same graceMs + margin the escalation uses.
if (!killing || groupEmpty()) {
if (graceTimer !== undefined) clearTimeout(graceTimer)
finishResolve()
return
}
const deadline = Date.now() + this.config.graceMs + CLOSE_REAP_MARGIN_MS
const pollGroup = (): void => {
if (groupEmpty() || Date.now() >= deadline) {
if (graceTimer !== undefined) clearTimeout(graceTimer)
finishResolve()
return
}
@@ -411,6 +411,29 @@ describe('PythonCodeRuntime — inherited resource limits', () => {
// the configured megabytes — exactly what the unclamped path applied.
expect(result.value).toEqual([42, 43, 400 * 1024 * 1024])
}, 15_000)
it('preserves an inherited soft limit stricter than the configured cap', async () => {
// Clamping reads BOTH inherited bounds, not just the hard one. A deployment
// that inherited a soft rlimit below the configured cap must keep that
// stricter soft: returning the configured value would RAISE the effective
// soft limit, loosening containment. The wrapper lowers only the SOFT CPU
// limit (`ulimit -S -t`) and leaves the hard limit unlimited, so the
// requested soft (`cpuSeconds`) sits above the inherited soft — the case that
// exposed the bug. RLIMIT_CPU is used because macOS ignores `ulimit -v`
// (RLIMIT_AS), which is exactly why the backend skips address space there.
const dir = await mkdtemp(join(tmpdir(), 'dsh-rlimit-soft-'))
const wrapper = join(dir, 'python3-soft-capped')
// Soft CPU 5 s, well below the configured 30 s, hard left unlimited.
await writeFile(wrapper, '#!/bin/sh\nulimit -S -t 5\nexec python3 "$@"\n', { mode: 0o755 })
const { runtime } = await setup({ pythonBin: wrapper, cpuSeconds: 30 })
const result = await runtime.run({
program: 'import resource\nreturn resource.getrlimit(resource.RLIMIT_CPU)[0]',
bindings: [],
})
expect(result.error).toBeUndefined()
// The applied SOFT limit is the inherited 5 s, not the configured 30 s.
expect(result.value).toBe(5)
}, 15_000)
})
describe('PythonCodeRuntime — programs and bindings', () => {