From a2d30d27fe855f52c5356b254c1960cc52a4c715 Mon Sep 17 00:00:00 2001 From: Liangsheng Yin Date: Sun, 19 Apr 2026 23:36:33 -0700 Subject: [PATCH] wait for reap in kill_process_tree (#23213) --- python/sglang/srt/entrypoints/engine.py | 6 ++- .../srt/entrypoints/http_server_engine.py | 2 +- python/sglang/srt/utils/common.py | 50 ++++++++++++++++++- .../test/server_fixtures/default_fixture.py | 2 +- .../server_fixtures/disaggregation_fixture.py | 2 +- .../test/server_fixtures/eagle_fixture.py | 2 +- .../test/server_fixtures/mmmu_fixture.py | 2 +- 7 files changed, 57 insertions(+), 9 deletions(-) diff --git a/python/sglang/srt/entrypoints/engine.py b/python/sglang/srt/entrypoints/engine.py index 04c16a33c..56ceda081 100644 --- a/python/sglang/srt/entrypoints/engine.py +++ b/python/sglang/srt/entrypoints/engine.py @@ -772,13 +772,15 @@ class Engine(EngineScoreMixin, EngineBase): ) def shutdown(self): - """Shutdown the engine""" + """Shutdown the engine; block until the scheduler subprocess releases + its GPU context so the caller can immediately reallocate on the same + device.""" if ( self.tokenizer_manager is not None and self.tokenizer_manager._subprocess_watchdog is not None ): self.tokenizer_manager._subprocess_watchdog.stop() - kill_process_tree(os.getpid(), include_parent=False) + kill_process_tree(os.getpid(), include_parent=False, wait_timeout=60) def __enter__(self): return self diff --git a/python/sglang/srt/entrypoints/http_server_engine.py b/python/sglang/srt/entrypoints/http_server_engine.py index cb6159836..8b8cbd97f 100644 --- a/python/sglang/srt/entrypoints/http_server_engine.py +++ b/python/sglang/srt/entrypoints/http_server_engine.py @@ -100,7 +100,7 @@ class HttpServerEngineAdapter(EngineBase): ) def shutdown(self): - kill_process_tree(self.process.pid) + kill_process_tree(self.process.pid, wait_timeout=60) def generate( self, diff --git a/python/sglang/srt/utils/common.py b/python/sglang/srt/utils/common.py index a91d42ae5..a56916f09 100644 --- a/python/sglang/srt/utils/common.py +++ b/python/sglang/srt/utils/common.py @@ -1047,8 +1047,48 @@ def check_pkg_version_at_least(pkg: str, min_version: str) -> bool: return False -def kill_process_tree(parent_pid, include_parent: bool = True, skip_pid: int = None): - """Kill the process and all its child processes.""" +def _wait_for_reap_or_raise(procs, wait_timeout: float) -> None: + """Wait for `procs` to exit; warn at ~10s, raise on `wait_timeout`. + + SIGKILL is asynchronous -- children hold GPU context, pinned memory and + fds until the kernel reaps them. Raise on timeout so a stuck process + surfaces instead of leaving a latent race. + """ + warn_at = min(10.0, wait_timeout / 2) + gone, alive = psutil.wait_procs(procs, timeout=warn_at) + if not alive: + return + logger.warning( + "kill_process_tree: %d process(es) still alive after %.1fs SIGKILL; " + "continuing to wait up to %.1fs total. pids=%s", + len(alive), + warn_at, + wait_timeout, + [p.pid for p in alive], + ) + remaining = wait_timeout - warn_at + if remaining > 0: + _, alive = psutil.wait_procs(alive, timeout=remaining) + if alive: + raise RuntimeError( + f"kill_process_tree: {len(alive)} process(es) not reaped within " + f"{wait_timeout}s after SIGKILL; pids={[p.pid for p in alive]}" + ) + + +def kill_process_tree( + parent_pid, + include_parent: bool = True, + skip_pid: int = None, + wait_timeout: Optional[float] = None, +): + """Kill the process and all its child processes. + + `wait_timeout` (seconds) blocks until every killed process is reaped and + raises `RuntimeError` on timeout; `None` is fire-and-forget. The + `parent_pid == os.getpid()` branch calls `sys.exit(0)` and cannot wait + for itself -- use `include_parent=False` if child reap must finish first. + """ if parent_pid is None: parent_pid = os.getpid() include_parent = False @@ -1059,11 +1099,13 @@ def kill_process_tree(parent_pid, include_parent: bool = True, skip_pid: int = N return children = itself.children(recursive=True) + killed = [] for child in children: if child.pid == skip_pid: continue try: child.kill() + killed.append(child) except psutil.NoSuchProcess: pass @@ -1078,9 +1120,13 @@ def kill_process_tree(parent_pid, include_parent: bool = True, skip_pid: int = N # Sometime processes cannot be killed with SIGKILL (e.g, PID=1 launched by kubernetes), # so we send an additional signal to kill them. itself.send_signal(signal.SIGQUIT) + killed.append(itself) except psutil.NoSuchProcess: pass + if wait_timeout is not None and killed: + _wait_for_reap_or_raise(killed, wait_timeout) + def monkey_patch_p2p_access_check(): """ diff --git a/python/sglang/test/server_fixtures/default_fixture.py b/python/sglang/test/server_fixtures/default_fixture.py index 89cf1c8fc..d10b72658 100644 --- a/python/sglang/test/server_fixtures/default_fixture.py +++ b/python/sglang/test/server_fixtures/default_fixture.py @@ -65,7 +65,7 @@ class DefaultServerBase(CustomTestCase): @classmethod def tearDownClass(cls): - kill_process_tree(cls.process.pid) + kill_process_tree(cls.process.pid, wait_timeout=60) time.sleep(2) @classmethod diff --git a/python/sglang/test/server_fixtures/disaggregation_fixture.py b/python/sglang/test/server_fixtures/disaggregation_fixture.py index dce859fae..83d5d5157 100644 --- a/python/sglang/test/server_fixtures/disaggregation_fixture.py +++ b/python/sglang/test/server_fixtures/disaggregation_fixture.py @@ -143,7 +143,7 @@ class PDDisaggregationServerBase(CustomTestCase): for process in [cls.process_lb, cls.process_decode, cls.process_prefill]: if process: try: - kill_process_tree(process.pid) + kill_process_tree(process.pid, wait_timeout=60) except Exception as e: print(f"Error killing process {process.pid}: {e}") diff --git a/python/sglang/test/server_fixtures/eagle_fixture.py b/python/sglang/test/server_fixtures/eagle_fixture.py index 3330d2868..d3201c087 100644 --- a/python/sglang/test/server_fixtures/eagle_fixture.py +++ b/python/sglang/test/server_fixtures/eagle_fixture.py @@ -57,7 +57,7 @@ class EagleServerBase(CustomTestCase): @classmethod def tearDownClass(cls): - kill_process_tree(cls.process.pid) + kill_process_tree(cls.process.pid, wait_timeout=60) def send_request(self): time.sleep(random.uniform(0, 2)) diff --git a/python/sglang/test/server_fixtures/mmmu_fixture.py b/python/sglang/test/server_fixtures/mmmu_fixture.py index 097dbff76..26cf5b26d 100644 --- a/python/sglang/test/server_fixtures/mmmu_fixture.py +++ b/python/sglang/test/server_fixtures/mmmu_fixture.py @@ -62,7 +62,7 @@ class MMMUServerBase(CustomTestCase): def tearDownClass(cls): if cls.process is not None and cls.process.poll() is None: try: - kill_process_tree(cls.process.pid) + kill_process_tree(cls.process.pid, wait_timeout=60) except Exception as e: logger.error(f"Error killing process: {e}") time.sleep(2)