Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions python/simpler/worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -1014,6 +1014,13 @@ def _read_args_from_mailbox(buf) -> TaskArgs:
return read_args_from_blob(mailbox_addr + _OFF_TASK_ARGS_BLOB)


# Idle mailbox polls between `getppid()` samples in a forked child. One poll
# costs ~0.1 us, so this samples roughly every 100 us — fast enough that an
# orphan is reaped before it is noticeable, cheap enough to be lost in the
# noise of the poll itself.
_PARENT_LIVENESS_POLL_INTERVAL = 1000


def _run_mailbox_loop(
buf,
state_addr: int,
Expand All @@ -1038,7 +1045,14 @@ def _run_mailbox_loop(
`on_shutdown()` runs on SHUTDOWN before the loop exits, for children that
own a nested Worker; per-child resource teardown that must survive an
exception belongs in the caller's own ``finally``.

A parent that dies without sending SHUTDOWN (SIGKILL from a timeout, an OOM
kill, a cancelled CI job) would otherwise leave this loop polling a mailbox
nobody writes to, for the lifetime of the machine. The loop therefore
samples its own parent and leaves by the SHUTDOWN path once it changes.
"""
parent_pid = os.getppid()
liveness_countdown = _PARENT_LIVENESS_POLL_INTERVAL
Comment on lines +1054 to +1055

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Parent PID sampled after fork leaves a small orphan window.

If the parent dies between fork() and this first os.getppid(), the sampled parent_pid is already the reparenting init/subreaper, so it never changes and the child spins forever — precisely the case #1493 targets. Capturing the parent's PID before the fork and passing it in closes the window.

🛡️ Sketch
 def _run_mailbox_loop(
     buf,
     state_addr: int,
     *,
     handle_task,
     handle_control,
     on_shutdown=None,
+    parent_pid: int | None = None,
 ) -> None:
@@
-    parent_pid = os.getppid()
+    # Prefer the pid captured by the forking parent; falling back to
+    # getppid() here can already observe the reparented ancestor.
+    parent_pid = parent_pid if parent_pid is not None else os.getppid()
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/simpler/worker.py` around lines 1054 - 1055, Capture the original
parent PID before the fork and pass it into the child worker initialization
instead of sampling it with os.getppid() after fork. Update the worker setup
around parent_pid and liveness_countdown to use this inherited PID, while
preserving the existing liveness polling behavior.

while True:
state = _mailbox_load_i32(state_addr)
if state == _TASK_READY:
Expand All @@ -1054,6 +1068,19 @@ def _run_mailbox_loop(
if on_shutdown is not None:
on_shutdown()
break
else:
liveness_countdown -= 1
if liveness_countdown <= 0:
liveness_countdown = _PARENT_LIVENESS_POLL_INTERVAL
# Comparing against the pid captured at entry rather than
# testing for pid 1: a subreaper (container init, systemd
# user session) adopts orphans instead of init, so the pid
# changes but never becomes 1. A live parent's pid cannot
# change, so this cannot fire spuriously.
if os.getppid() != parent_pid:
if on_shutdown is not None:
on_shutdown()
break


def _sub_worker_loop(
Expand Down
Loading
Loading