Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion lelab/rollout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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`,
Expand Down Expand Up @@ -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}"}
Expand Down
32 changes: 32 additions & 0 deletions tests/test_rollout.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down