diff --git a/lelab/rollout.py b/lelab/rollout.py index cfff79a8..0b34f31e 100644 --- a/lelab/rollout.py +++ b/lelab/rollout.py @@ -266,6 +266,11 @@ def handle_start_inference(request: InferenceRequest) -> dict[str, Any]: # Claim the slot now so a concurrent caller losing the race sees us. inference_active = True + # Opened partway through setup; the stdout pump thread takes ownership once + # it starts. Tracked here so a failure before that hand-off can close it + # instead of leaking the file handle (which on Windows also keeps the log + # locked). + log_handle = None try: # `setup_follower_calibration_file` returns the basename without the # .json extension. We need that stripped form for `--robot.id`, @@ -323,9 +328,14 @@ def handle_start_inference(request: InferenceRequest) -> dict[str, Any]: name="inference-stdout-pump", daemon=True, ).start() + log_handle = None # pump thread owns and closes it from here on except Exception as exc: logger.exception("Failed to start inference") - # Subprocess never started — release the slot. + # Startup failed before the pump thread took over (most often Popen + # itself). Release the slot and close the log file if we opened it before + # failing, so the handle isn't leaked. + if log_handle is not None: + log_handle.close() with _state_lock: inference_active = False return {"success": False, "status_code": 500, "message": f"Failed to start inference: {exc}"} diff --git a/tests/test_rollout.py b/tests/test_rollout.py index c0bb1f3d..5e445b70 100644 --- a/tests/test_rollout.py +++ b/tests/test_rollout.py @@ -263,6 +263,38 @@ def test_handle_start_inference_blocked_when_already_active(monkeypatch) -> None assert "already active" in result["message"] +def test_handle_start_inference_closes_log_when_popen_fails(monkeypatch, tmp_path) -> None: + """If Popen fails after the log file is opened, the handle must be closed, + not leaked — on Windows a leaked handle also keeps the log file locked.""" + from lelab import rollout + + monkeypatch.setattr(rollout, "setup_follower_calibration_file", lambda cfg: "robot_a") + monkeypatch.setattr(rollout, "_resolve_policy_path", lambda ref: str(tmp_path / "model")) + + opened: list = [] + real_open = rollout.Path.open + + def tracking_open(self, *args, **kwargs): + handle = real_open(self, *args, **kwargs) + opened.append(handle) + return handle + + monkeypatch.setattr(rollout.Path, "open", tracking_open) + + def boom(*args, **kwargs): + raise OSError("simulated Popen failure") + + monkeypatch.setattr(rollout.subprocess, "Popen", boom) + + result = rollout.handle_start_inference(_stub_request()) + + assert result["success"] is False + assert result["status_code"] == 500 + assert opened, "expected the inference log file to have been opened" + assert all(handle.closed for handle in opened), "log handle leaked on Popen failure" + assert rollout.inference_active is False + + def test_classify_outcome_ok_warns_and_fails() -> None: from lelab.rollout import _classify_outcome