Skip to content

Commit 1d39a4a

Browse files
committed
0.8.4: a stopped step stops everything it started
When a step was stopped (its timeout ran out, the run was canceled, or the runner was shutting down) the runner killed only the shell that wraps the step's command. Whatever ran under that shell carried on with nobody watching: on 2026-11-11 the org-root dataset-scout seed step hit its 240 minute timeout and its claude process kept working against the server for another 40 minutes. Each step now runs in its own session and process group. On a stop the runner notes every process under the step (one that moved to a session of its own is still found, by parentage), sends them SIGTERM so a program that manages children of its own can stop them, and kills whatever is still running 5 seconds later. Windows is unchanged. The old cancel test only looked at the shell's own return code, which is why it never saw this. Four tests now cover a grandchild, a grandchild in its own session, a process that ignores SIGTERM, and that a step never shares the runner's process group.
1 parent 7ed99c0 commit 1d39a4a

4 files changed

Lines changed: 220 additions & 5 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,30 @@
33
Notable changes to the `datatorch` python package (CLI, SDK and the runner
44
daemon). Versions are the tags on this repository and the releases on PyPI.
55

6+
## 0.8.4 (2026-11-12)
7+
8+
### Fixed
9+
10+
- Runner: a step that is stopped (its timeout ran out, the run was
11+
canceled, or the runner is shutting down) now stops everything it
12+
started, not only the shell that wraps its command. Before, the runner
13+
killed that one shell and whatever ran under it carried on with nobody
14+
watching: on 2026-11-11 the org-root `dataset-scout` seed step hit its
15+
240 minute timeout and its `claude` process kept working against the
16+
server for another 40 minutes. Each step now runs in its own session
17+
and process group. On a stop the runner notes every process under the
18+
step (one that moved to a session of its own is still found, by
19+
parentage), sends them SIGTERM so a program that manages children of
20+
its own can stop them, and kills whatever is still running 5 seconds
21+
later (`TERMINATE_GRACE_SECONDS`). Windows is unchanged: the direct
22+
child is terminated as before.
23+
24+
### Changed
25+
26+
- Runner: because a step has its own session, Ctrl-C in the terminal the
27+
runner was started from reaches the runner alone. The runner then stops
28+
its running steps itself, through the path above.
29+
630
## 0.8.3 (2026-11-09)
731

832
### Fixed

‎datatorch/runner/pipelines/runner/runner.py‎

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,12 @@
11
import os
22
import json
3+
import time
4+
import signal
35
import typing
46
import asyncio
57

8+
import psutil
9+
610
from typing import Any, Awaitable, Dict
711
from ..template import Variables
812

@@ -19,6 +23,51 @@ class ProcessCodeError(Exception):
1923
# output value (e.g. an http-request body), not ordinary log lines.
2024
STDOUT_LINE_LIMIT = 16 * 1024 * 1024
2125

26+
# How long a stopped step's processes get to exit by themselves (SIGTERM)
27+
# before they are killed. The server has already closed the step by then,
28+
# so nothing waits on this except the processes' own cleanup.
29+
TERMINATE_GRACE_SECONDS = 5.0
30+
31+
# Windows has no process groups in this sense and no SIGKILL.
32+
_POSIX = os.name == "posix"
33+
_SIGKILL = getattr(signal, "SIGKILL", signal.SIGTERM)
34+
35+
36+
def _descendants(pid: int) -> list:
37+
"""Every process under `pid` right now, as psutil handles (which
38+
refuse to signal a pid that has since been reused)."""
39+
try:
40+
return psutil.Process(pid).children(recursive=True)
41+
except psutil.Error:
42+
return []
43+
44+
45+
def _alive(proc) -> bool:
46+
try:
47+
return proc.is_running() and proc.status() != psutil.STATUS_ZOMBIE
48+
except psutil.Error:
49+
return False
50+
51+
52+
def _signal_tree(process, tree: list, sig: int) -> None:
53+
"""Send `sig` to the step's process group and to every noted
54+
descendant, wherever it now lives. A process already gone is fine."""
55+
if process.returncode is None:
56+
try:
57+
if _POSIX:
58+
# run_cmd gives each step its own session, so its group id
59+
# is the shell's pid and never the runner's own group.
60+
os.killpg(process.pid, sig)
61+
else:
62+
process.send_signal(sig)
63+
except (ProcessLookupError, PermissionError, OSError):
64+
pass
65+
for proc in tree:
66+
try:
67+
proc.send_signal(sig)
68+
except (psutil.Error, OSError):
69+
pass
70+
2271

2372
class Runner(object):
2473
def __init__(self, config: dict, action: "Action"):
@@ -44,8 +93,8 @@ async def monitor_cmd(self, command: str, env: dict = None):
4493
4594
Streams stdout live (rather than waiting then draining) so that if
4695
the step is canceled or times out server-side — the asyncio task
47-
running it receives ``CancelledError`` — the child process is killed
48-
promptly instead of being orphaned.
96+
running it receives ``CancelledError`` — the child process and
97+
everything it started are stopped instead of being orphaned.
4998
"""
5099
process = await self.run_cmd(command, wait=False, env=env)
51100

@@ -69,11 +118,36 @@ async def monitor_cmd(self, command: str, env: dict = None):
69118
)
70119

71120
async def _terminate(self, process):
72-
"""Kill a still-running child process (best effort)."""
121+
"""Stop a still-running step: the whole process tree, not only the
122+
shell that wraps the command (best effort).
123+
124+
`run_cmd` starts the command through a shell, so `process` is that
125+
shell. Killing it alone left everything under it running: on
126+
2026-11-11 a timed-out seed step's `claude` process worked on for
127+
40 minutes after the server had closed the step. So: note every
128+
descendant first (a process that moved to its own group is found by
129+
parentage, and once its parent dies that link is gone), ask the
130+
tree to stop with SIGTERM so a process that manages children of its
131+
own can stop them, then kill whatever is left after
132+
`TERMINATE_GRACE_SECONDS`.
133+
"""
73134
if process.returncode is not None:
74135
return
136+
137+
tree = _descendants(process.pid)
138+
_signal_tree(process, tree, signal.SIGTERM)
139+
140+
deadline = time.monotonic() + TERMINATE_GRACE_SECONDS
141+
while time.monotonic() < deadline:
142+
if process.returncode is not None and not any(
143+
_alive(proc) for proc in tree
144+
):
145+
break
146+
await asyncio.sleep(0.1)
147+
else:
148+
_signal_tree(process, tree, _SIGKILL)
149+
75150
try:
76-
process.kill()
77151
await process.wait()
78152
except ProcessLookupError:
79153
pass
@@ -157,6 +231,10 @@ async def run_cmd(self, command: str, wait: bool = True, env: dict = None):
157231
# page body — asyncio's 64KB default readline limit kills the
158232
# stream with LimitOverrunError on anything larger.
159233
limit=STDOUT_LINE_LIMIT,
234+
# Its own session and process group, so a stop can signal the
235+
# step's whole tree at once without touching the runner (see
236+
# _terminate). Ignored on Windows.
237+
start_new_session=True,
160238
)
161239
if wait:
162240
await process.wait()

‎setup.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333

3434
setup(
3535
name="datatorch",
36-
version="0.8.3",
36+
version="0.8.4",
3737
description="A CLI and library for interacting with DataTorch.",
3838
author="DataTorch",
3939
author_email="support@datatorch.io",

‎test/runner/pipelines/test_runner_cancel.py‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,15 @@
66
"""
77

88
import asyncio
9+
import os
10+
import shlex
11+
import sys
12+
import time
913

14+
import psutil
1015
import pytest
1116

17+
from datatorch.runner.pipelines.runner import runner as runner_module
1218
from datatorch.runner.pipelines.runner.runner import Runner
1319

1420

@@ -47,3 +53,110 @@ async def capturing(command, wait=True, env=None):
4753
# A short wall-clock timeout guards against a regression where cancel
4854
# hangs waiting for the full `sleep 30`.
4955
asyncio.run(asyncio.wait_for(inner(), timeout=10))
56+
57+
58+
# --- The whole tree stops, not only the shell (0.8.4) -----------------------
59+
#
60+
# On 2026-11-11 a seed step hit its timeout, the runner killed the shell that
61+
# wrapped the action, and the `claude` process under it kept working against
62+
# the server for another 40 minutes. The test above could not see that: it
63+
# only looked at the shell's own return code.
64+
65+
66+
def _gone(pid: int) -> bool:
67+
try:
68+
proc = psutil.Process(pid)
69+
return proc.status() == psutil.STATUS_ZOMBIE
70+
except psutil.NoSuchProcess:
71+
return True
72+
73+
74+
def _cancel_and_collect(command: str, pid_file: str, expected: int = 1):
75+
"""Run `command` as a step, wait until it has written `expected` pids
76+
to `pid_file`, cancel the step, and return those pids."""
77+
78+
async def inner():
79+
runner = Runner({}, _FakeAction())
80+
task = asyncio.ensure_future(runner.monitor_cmd(command))
81+
82+
pids = []
83+
deadline = time.monotonic() + 5
84+
while time.monotonic() < deadline and len(pids) < expected:
85+
await asyncio.sleep(0.05)
86+
if os.path.exists(pid_file):
87+
with open(pid_file) as handle:
88+
pids = [int(part) for part in handle.read().split()]
89+
assert len(pids) == expected, "the step never started its children"
90+
assert all(psutil.pid_exists(pid) for pid in pids)
91+
92+
task.cancel()
93+
with pytest.raises(asyncio.CancelledError):
94+
await task
95+
return pids
96+
97+
return asyncio.run(asyncio.wait_for(inner(), timeout=20))
98+
99+
100+
@pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX")
101+
def test_cancel_stops_a_grandchild(tmp_path):
102+
"""The shape of a python action: the shell starts a program, and that
103+
program starts another. Both must be gone after a cancel."""
104+
pid_file = str(tmp_path / "pids")
105+
command = f"(sleep 60 & echo $! > {shlex.quote(pid_file)}; wait)"
106+
107+
(grandchild,) = _cancel_and_collect(command, pid_file)
108+
109+
assert _gone(grandchild)
110+
111+
112+
@pytest.mark.skipif(os.name != "posix", reason="sessions are POSIX")
113+
def test_cancel_stops_a_grandchild_in_its_own_session(tmp_path):
114+
"""A program that puts its children in their own session (an agent's
115+
shell tool does) takes them out of the step's process group. They are
116+
still found, by parentage, and stopped."""
117+
pid_file = str(tmp_path / "pids")
118+
script = (
119+
"import subprocess, sys;"
120+
"p = subprocess.Popen(['sleep', '60'], start_new_session=True);"
121+
"open(sys.argv[1], 'w').write(str(p.pid));"
122+
"p.wait()"
123+
)
124+
command = " ".join(
125+
shlex.quote(part) for part in (sys.executable, "-c", script, pid_file)
126+
)
127+
128+
(grandchild,) = _cancel_and_collect(command, pid_file)
129+
130+
assert _gone(grandchild)
131+
132+
133+
@pytest.mark.skipif(os.name != "posix", reason="signals are POSIX")
134+
def test_cancel_kills_what_ignores_the_polite_stop(tmp_path, monkeypatch):
135+
"""A process that ignores SIGTERM is killed once the grace runs out."""
136+
monkeypatch.setattr(runner_module, "TERMINATE_GRACE_SECONDS", 0.5)
137+
pid_file = str(tmp_path / "pids")
138+
command = f"trap '' TERM; (sleep 60 & echo $! > {shlex.quote(pid_file)}; wait)"
139+
140+
started = time.monotonic()
141+
(grandchild,) = _cancel_and_collect(command, pid_file)
142+
143+
assert _gone(grandchild)
144+
assert time.monotonic() - started < 10
145+
146+
147+
@pytest.mark.skipif(os.name != "posix", reason="process groups are POSIX")
148+
def test_a_step_runs_outside_the_runners_process_group():
149+
"""The stop signals a whole group, so that group must never be the
150+
runner's own."""
151+
152+
async def inner():
153+
runner = Runner({}, _FakeAction())
154+
process = await runner.run_cmd("sleep 60", wait=False)
155+
try:
156+
assert os.getpgid(process.pid) == process.pid
157+
assert os.getpgid(process.pid) != os.getpgrp()
158+
finally:
159+
await runner._terminate(process)
160+
assert process.returncode is not None
161+
162+
asyncio.run(asyncio.wait_for(inner(), timeout=10))

0 commit comments

Comments
 (0)