From 30fdef139b84e0fb79c901f4abc06a3b20fb6f68 Mon Sep 17 00:00:00 2001
From: rx-oo <gimezjz10@gmail.com>
Date: Mon, 6 Jul 2026 22:49:11 -0400
Subject: [PATCH] fix(sglang): never skip kill_process_tree when router removal
 fails in shutdown
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The miles-router / old-router branch of SGLangEngine.shutdown posted
/remove_worker without any exception guard. When the router is already
dead — which is exactly the failure-cleanup scenario where
RolloutManager.shutdown_hard drives this method — requests raises
ConnectionError, the method aborts, and kill_process_tree never runs.

ray.kill on the engine actor does not terminate its child processes,
and the CUDA context lives in the SGLang server tree, so the orphaned
servers keep their VRAM until a manual pkill; subsequent runs OOM.
The >=0.3.0 router branch already had a try/except guard — this brings
the other branches to parity and makes router removal best-effort:
warn on failure, always kill the process tree.
---
 miles/backends/sglang_utils/sglang_engine.py | 65 ++++++++++++--------
 1 file changed, 40 insertions(+), 25 deletions(-)

diff --git a/miles/backends/sglang_utils/sglang_engine.py b/miles/backends/sglang_utils/sglang_engine.py
index 6dd140ebf47..58786cbdf31 100644
--- a/miles/backends/sglang_utils/sglang_engine.py
+++ b/miles/backends/sglang_utils/sglang_engine.py
@@ -569,33 +569,48 @@ def shutdown(self):
             return
 
         logger.info(f"Shutdown engine {self.server_host}:{self.server_port}...")
+        # Router removal is best-effort: the router may already be dead in
+        # failure-cleanup scenarios (which is exactly when
+        # RolloutManager.shutdown_hard drives this method). A raise here must
+        # not skip kill_process_tree below — the CUDA context lives in the
+        # SGLang server child processes, and ray.kill on the actor does NOT
+        # terminate them, so skipping the kill orphans their VRAM until a
+        # manual pkill.
         if self.node_rank == 0:
-            worker_url = f"http://{self.server_host}:{self.server_port}"
-            response = None
-            if parse(sglang_router.__version__) <= parse("0.2.1") or self.args.use_miles_router:
-                response = requests.post(
-                    f"http://{self.router_ip}:{self.router_port}/remove_worker?url=http://{self.server_host}:{self.server_port}"
+            try:
+                worker_url = f"http://{self.server_host}:{self.server_port}"
+                response = None
+                if parse(sglang_router.__version__) <= parse("0.2.1") or self.args.use_miles_router:
+                    response = requests.post(
+                        f"http://{self.router_ip}:{self.router_port}/remove_worker?url=http://{self.server_host}:{self.server_port}"
+                    )
+                elif parse(sglang_router.__version__) < parse("0.3.0"):
+                    worker_url = quote(worker_url, safe="")
+                    response = requests.delete(f"http://{self.router_ip}:{self.router_port}/workers/{worker_url}")
+                else:
+                    try:
+                        all_workers = requests.get(f"http://{self.router_ip}:{self.router_port}/workers").json()[
+                            "workers"
+                        ]
+                        for worker in all_workers:
+                            if worker["url"] == worker_url:
+                                worker_id = worker["id"]
+                                response = requests.delete(
+                                    f"http://{self.router_ip}:{self.router_port}/workers/{worker_id}"
+                                )
+                                break
+                        else:
+                            logger.warning(f"Worker {worker_url} not found in router during shutdown.")
+                    except Exception as e:
+                        logger.warning(f"Failed to fetch workers list or remove worker: {e}")
+
+                if response is not None:
+                    response.raise_for_status()
+            except Exception as e:
+                logger.warning(
+                    f"shutdown: router worker removal failed (router may already be down); "
+                    f"proceeding to kill the server process tree: {e}"
                 )
-            elif parse(sglang_router.__version__) < parse("0.3.0"):
-                worker_url = quote(worker_url, safe="")
-                response = requests.delete(f"http://{self.router_ip}:{self.router_port}/workers/{worker_url}")
-            else:
-                try:
-                    all_workers = requests.get(f"http://{self.router_ip}:{self.router_port}/workers").json()["workers"]
-                    for worker in all_workers:
-                        if worker["url"] == worker_url:
-                            worker_id = worker["id"]
-                            response = requests.delete(
-                                f"http://{self.router_ip}:{self.router_port}/workers/{worker_id}"
-                            )
-                            break
-                    else:
-                        logger.warning(f"Worker {worker_url} not found in router during shutdown.")
-                except Exception as e:
-                    logger.warning(f"Failed to fetch workers list or remove worker: {e}")
-
-            if response is not None:
-                response.raise_for_status()
         kill_process_tree(self.process.pid)
 
     def get_weight_version(self):