From 9ceb5a9c7d95018423d76fd7659e526f42daf23b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=83=91=E6=98=8E=E5=86=8D?= Date: Tue, 9 Jun 2026 00:05:21 +0800 Subject: [PATCH 1/3] fix(model): use os._exit for OOM to trigger pool recovery sys.exit(1) only raises SystemExit, which xoscar's _ErrorProcessor swallows with an unconditional `return True`. The process then stays alive, its returncode stays None, monitor_sub_pools keeps treating it as alive, and recover_sub_pool never fires. Replace sys.exit(1) with os._exit(1) in _handle_oom_error so the subprocess actually exits and xoscar can detect the returncode change and rebuild the sub pool. Add a reentrancy gate (_oom_handling) to handle concurrent OOM on the sync batch path, and reduce the stop() timeout from 30s to 5s to bound recovery delay. Add regression tests in both sync and async client test suites. Co-Authored-By: Claude Opus 4.7 --- xinference/client/tests/test_async_client.py | 39 ++++++++++++++ xinference/client/tests/test_client.py | 38 ++++++++++++++ xinference/core/model.py | 54 ++++++++++++-------- 3 files changed, 111 insertions(+), 20 deletions(-) diff --git a/xinference/client/tests/test_async_client.py b/xinference/client/tests/test_async_client.py index 36d9e61bea..72be459ffd 100644 --- a/xinference/client/tests/test_async_client.py +++ b/xinference/client/tests/test_async_client.py @@ -616,6 +616,45 @@ async def test_model_error(set_test_oom_error, setup_cluster): await client.close() +@pytest.mark.asyncio +async def test_model_oom_triggers_restart(set_test_oom_error, setup_cluster): + # OOM must hard-exit the model subprocess via os._exit(1) so xoscar + # can detect the exit and recover it. With the old sys.exit(1) the + # SystemExit was swallowed by xoscar's _ErrorProcessor, the process + # never died, and this test would time out at the "subprocess did not + # exit" assert below. + endpoint, _ = setup_cluster + current_proc = psutil.Process() + before = set(current_proc.children(recursive=True)) + client = AsyncRESTfulClient(endpoint) + + model_uid = await client.launch_model( + model_name="qwen1.5-chat", + model_engine="llama.cpp", + model_size_in_billions="0_5", + quantization="q4_0", + ) + assert len(await client.list_models()) == 1 + model_proc = next(iter(set(current_proc.children(recursive=True)) - before)) + + model = await client.get_model(model_uid=model_uid) + assert isinstance(model, AsyncRESTfulChatModelHandle) + + # Trigger OOM -> os._exit(1) on the model subprocess. + with pytest.raises(Exception): + await model.generate("Once upon a time, there was a very old computer") + + # The old subprocess must actually exit (regression guard). + for _ in range(30): + if not model_proc.is_running() or model_proc.status() == psutil.STATUS_ZOMBIE: + break + time.sleep(1) + else: + assert False, "OOM subprocess did not exit (sys.exit swallowed by xoscar)" + await model.close() + await client.close() + + @pytest.mark.asyncio async def test_async_restful_chat_enable_thinking_injected(): handle = AsyncRESTfulChatModelHandle.__new__(AsyncRESTfulChatModelHandle) diff --git a/xinference/client/tests/test_client.py b/xinference/client/tests/test_client.py index 6c9238d555..2385d3bd58 100644 --- a/xinference/client/tests/test_client.py +++ b/xinference/client/tests/test_client.py @@ -545,6 +545,44 @@ def test_model_error(set_test_oom_error, setup_cluster): model.generate("Once upon a time, there was a very old computer") +def test_model_oom_triggers_restart(set_test_oom_error, setup_cluster): + # OOM must hard-exit the model subprocess via os._exit(1) so xoscar + # can detect the exit and recover it. With the old sys.exit(1) the + # SystemExit was swallowed by xoscar's _ErrorProcessor, the process + # never died, and this test would time out at the "subprocess did not + # exit" assert below. + endpoint, _ = setup_cluster + current_proc = psutil.Process() + before = set(current_proc.children(recursive=True)) + client = RESTfulClient(endpoint) + + model_uid = client.launch_model( + model_name="qwen2.5-instruct", + model_engine="llama.cpp", + model_size_in_billions="0_5", + model_format="ggufv2", + quantization="q4_0", + ) + assert len(client.list_models()) == 1 + model_proc = next(iter(set(current_proc.children(recursive=True)) - before)) + + model = client.get_model(model_uid=model_uid) + assert isinstance(model, RESTfulChatModelHandle) + + # Trigger OOM. The handler runs os._exit(1) on the model subprocess, so the + # client sees an error (OOM report or connection drop) -- either is fine. + with pytest.raises(Exception): + model.generate("Once upon a time, there was a very old computer") + + # The old subprocess must actually exit (this is the regression guard). + for _ in range(30): + if not model_proc.is_running() or model_proc.status() == psutil.STATUS_ZOMBIE: + break + time.sleep(1) + else: + assert False, "OOM subprocess did not exit (sys.exit swallowed by xoscar)" + + def test_restful_chat_enable_thinking_injected(): handle = RESTfulChatModelHandle("test-model", "http://localhost", {}) dummy_session = _DummySession() diff --git a/xinference/core/model.py b/xinference/core/model.py index a4937ea495..12df5d4f62 100644 --- a/xinference/core/model.py +++ b/xinference/core/model.py @@ -18,7 +18,6 @@ import json import os import queue -import sys import time import types import uuid @@ -519,35 +518,40 @@ async def stop(self): await asyncio.to_thread(self._model.close) async def _handle_oom_error(self, ex): + # Reentrancy gate: on the sync batch path the OOM-triggering worker + # thread does not await this coroutine, so before os._exit(1) actually + # lands it can raise OOM again and concurrently schedule this handler. + # Only let the first invocation through to avoid re-entering stop()/del. + if getattr(self, "_oom_handling", False): + return + self._oom_handling = True + error_message = ( f"Model actor is out of memory, model id: {self.model_uid()}, error: {ex}" ) logger.exception(error_message) - worker_ref = await self._get_worker_ref() - await worker_ref.update_model_status( - self._replica_model_uid, last_error=error_message - ) - # ยง4.1: Graceful cleanup instead of os._exit(1). - # 1) Stop model to release underlying resources (with timeout guard) try: - await asyncio.wait_for(self.stop(), timeout=30) - except asyncio.TimeoutError: - logger.warning( - "Model stop() timed out after 30s for %s, proceeding with cleanup", - self.model_uid(), + worker_ref = await self._get_worker_ref() + await worker_ref.update_model_status( + self._replica_model_uid, last_error=error_message ) + except Exception: + logger.warning("Failed to report OOM status before exit", exc_info=True) + + # Best-effort cleanup. At an OOM site this mostly cannot reclaim already + # reserved GPU fragments; failures are fine because os._exit(1) below is + # the real backstop. stop() timeout is 5s (not 30s): keep the exit delay + # bounded so xoscar's detection and recovery are not slowed down. + try: + await asyncio.wait_for(self.stop(), timeout=5) except Exception: logger.warning( - "Model stop() failed for %s, proceeding with cleanup", - self.model_uid(), - exc_info=True, + "Model stop() failed/timeout during OOM cleanup", exc_info=True ) - # 2) Delete model reference to allow GC to reclaim GPU memory try: del self._model except AttributeError: pass - # 3) Release PyTorch GPU cache try: import torch @@ -555,9 +559,19 @@ async def _handle_oom_error(self, ex): torch.cuda.empty_cache() except Exception: pass - # 4) Use sys.exit(1) instead of os._exit(1) so that Python cleanup hooks - # run and xoscar can detect the sub-process exit, triggering recover_sub_pool. - sys.exit(1) + + # Critical fix: must use os._exit(1), NOT sys.exit(1). + # sys.exit(1) only raises SystemExit, which xoscar's + # _ErrorProcessor.__exit__ (backends/pool.py) swallows with an + # unconditional `return True`. The process then stays alive, its + # returncode stays None, monitor_sub_pools keeps treating it as alive, + # and recover_sub_pool never fires. os._exit goes straight to the + # syscall so xoscar can detect the returncode change and rebuild the + # sub pool. + logger.error( + "Exiting model subprocess via os._exit(1) to trigger pool recovery" + ) + os._exit(1) def _to_generator(self, output_type: str, gen: types.GeneratorType): start_time = time.time() From bff91c2d4318f2abca6a6836d4ef20d222e5a58d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=83=91=E6=98=8E=E5=86=8D?= Date: Tue, 9 Jun 2026 00:42:27 +0800 Subject: [PATCH 2/3] fix(model): add timeout to OOM status update and use asyncio.sleep in test Address code review feedback: 1. Wrap worker_ref.update_model_status in asyncio.wait_for(timeout=5) in _handle_oom_error. During OOM the worker may be unresponsive or under high load, and the RPC could block indefinitely, preventing os._exit(1) from being reached and delaying pool recovery. The status update is best-effort and should not block the exit path. 2. Replace time.sleep(1) with await asyncio.sleep(1) in async test test_model_oom_triggers_restart. Blocking the event loop with time.sleep in an async test can delay other async tasks and cause test flakiness or timeouts. Co-Authored-By: Claude Opus 4.7 --- xinference/client/tests/test_async_client.py | 2 +- xinference/core/model.py | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/xinference/client/tests/test_async_client.py b/xinference/client/tests/test_async_client.py index 72be459ffd..bae4b6934f 100644 --- a/xinference/client/tests/test_async_client.py +++ b/xinference/client/tests/test_async_client.py @@ -648,7 +648,7 @@ async def test_model_oom_triggers_restart(set_test_oom_error, setup_cluster): for _ in range(30): if not model_proc.is_running() or model_proc.status() == psutil.STATUS_ZOMBIE: break - time.sleep(1) + await asyncio.sleep(1) else: assert False, "OOM subprocess did not exit (sys.exit swallowed by xoscar)" await model.close() diff --git a/xinference/core/model.py b/xinference/core/model.py index 12df5d4f62..be8b31aae7 100644 --- a/xinference/core/model.py +++ b/xinference/core/model.py @@ -532,8 +532,11 @@ async def _handle_oom_error(self, ex): logger.exception(error_message) try: worker_ref = await self._get_worker_ref() - await worker_ref.update_model_status( - self._replica_model_uid, last_error=error_message + await asyncio.wait_for( + worker_ref.update_model_status( + self._replica_model_uid, last_error=error_message + ), + timeout=5, ) except Exception: logger.warning("Failed to report OOM status before exit", exc_info=True) From 5646f5726a924592019d4a8e5d35a8ce0d5d9148 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=83=91=E6=98=8E=E5=86=8D?= Date: Tue, 9 Jun 2026 14:13:46 +0800 Subject: [PATCH 3/3] fix(model): bound _get_worker_ref inside the same xoscar timeout Address review feedback: _get_worker_ref() calls xo.actor_ref() which is also an xoscar RPC and can hang during OOM / worker failure. Wrap the entire status report (_get_worker_ref + update_model_status) in a single xo.wait_for(timeout=5) so the process always reaches os._exit(1) promptly, even if the worker is completely unreachable. Co-Authored-By: Claude Opus 4.7 --- xinference/core/model.py | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/xinference/core/model.py b/xinference/core/model.py index be8b31aae7..79cba41349 100644 --- a/xinference/core/model.py +++ b/xinference/core/model.py @@ -530,14 +530,15 @@ async def _handle_oom_error(self, ex): f"Model actor is out of memory, model id: {self.model_uid()}, error: {ex}" ) logger.exception(error_message) - try: + + async def _report_oom_status(): worker_ref = await self._get_worker_ref() - await asyncio.wait_for( - worker_ref.update_model_status( - self._replica_model_uid, last_error=error_message - ), - timeout=5, + await worker_ref.update_model_status( + self._replica_model_uid, last_error=error_message ) + + try: + await xo.wait_for(_report_oom_status(), timeout=5) except Exception: logger.warning("Failed to report OOM status before exit", exc_info=True)