Skip to content

Commit cc78886

Browse files
committed
[bugfix]: avoid duplicate worker shutdown cleanup path
1 parent 3c4fe9a commit cc78886

2 files changed

Lines changed: 54 additions & 4 deletions

File tree

fastvideo/tests/worker/test_multiproc_executor.py

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,16 @@ def shutdown(self):
5555
return {"status": "shutdown"}
5656

5757

58+
class _FailingWorker:
59+
60+
def __init__(self):
61+
self.shutdown_calls = 0
62+
63+
def shutdown(self):
64+
self.shutdown_calls += 1
65+
raise RuntimeError("interrupted shutdown")
66+
67+
5868
class _GracefulProcess:
5969

6070
def __init__(self, required_timeout: float):
@@ -149,6 +159,29 @@ def test_worker_shutdown_closes_worker_and_compile_pool_once(monkeypatch) -> Non
149159
assert compile_shutdowns == [True]
150160

151161

162+
def test_worker_shutdown_still_idempotent_after_failure(monkeypatch) -> None:
163+
worker = _FailingWorker()
164+
compile_shutdowns = []
165+
proc = WorkerMultiprocProc.__new__(WorkerMultiprocProc)
166+
proc.rank = 0
167+
proc.worker = worker
168+
proc._shutdown_started = False
169+
proc._shutdown_complete = False
170+
proc._shutdown_response = None
171+
monkeypatch.setattr(
172+
"fastvideo.worker.multiproc_executor._shutdown_torch_compile_workers",
173+
lambda: compile_shutdowns.append(True),
174+
)
175+
176+
first = proc.shutdown()
177+
second = proc.shutdown()
178+
179+
assert first == {"status": "shutdown"}
180+
assert second == {"status": "shutdown"}
181+
assert worker.shutdown_calls == 1
182+
assert compile_shutdowns == [True]
183+
184+
152185
def test_compile_worker_cleanup_uses_only_an_already_loaded_inductor(monkeypatch) -> None:
153186
compile_shutdowns = []
154187
fake_async_compile = SimpleNamespace(shutdown_compile_workers=lambda: compile_shutdowns.append(True))

fastvideo/worker/multiproc_executor.py

Lines changed: 21 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -522,6 +522,7 @@ def __init__(
522522
self.streaming_input_queue = streaming_input_queue
523523
self.streaming_output_queue = streaming_output_queue
524524
self._initial_log_handler = _initial_log_handler
525+
self._shutdown_started = False
525526
self._shutdown_complete = False
526527
self._shutdown_response: dict[str, Any] | None = None
527528
wrapper = WorkerWrapperBase(fastvideo_args=fastvideo_args, rpc_rank=rank)
@@ -719,10 +720,26 @@ def shutdown(self) -> dict[str, Any]:
719720
assert self._shutdown_response is not None
720721
return self._shutdown_response
721722

722-
response = self.worker.shutdown()
723-
_shutdown_torch_compile_workers()
724-
self._shutdown_response = response
725-
self._shutdown_complete = True
723+
if getattr(self, "_shutdown_started", False):
724+
if self._shutdown_response is None:
725+
self._shutdown_response = {"status": "shutdown"}
726+
return self._shutdown_response
727+
728+
self._shutdown_started = True
729+
response = {"status": "shutdown"}
730+
shutdown_succeeded = False
731+
732+
try:
733+
response = self.worker.shutdown()
734+
shutdown_succeeded = True
735+
except Exception:
736+
logger.exception("Worker %d failed to shut down", self.rank)
737+
finally:
738+
_shutdown_torch_compile_workers()
739+
if shutdown_succeeded:
740+
self._shutdown_complete = True
741+
self._shutdown_response = response
742+
726743
return response
727744

728745
def worker_busy_loop(self) -> None:

0 commit comments

Comments
 (0)