Skip to content

Commit 65e2ed0

Browse files
committed
feat: add container cleanup and OpenHands session cache
- Track and label FeatureBench Docker containers for infer, eval, and data flows so interrupts and process exit can clean them up. - Return exit code 130 on KeyboardInterrupt and prevent duplicate cleanup races while shutdown is already in progress. - Add OpenHands --session-cache support with per-attempt X-Session-Id headers and backend session release via DELETE /session_release. - Mirror OpenHands SDK events into live inference logs and send reasoning aliases through both reasoning_content and reasoning fields. - Mark completed eval reports, rerun legacy interrupted reports, and avoid saving partial reports during shutdown. - Thread Docker labels and task-specific env through infer/eval/review containers and document the new CLI option. - Add tests for infer, eval, data container cleanup and OpenHands session release behavior.
1 parent 9513156 commit 65e2ed0

14 files changed

Lines changed: 1766 additions & 101 deletions

docs/infer_cli_arg.md

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -135,15 +135,19 @@ flags can override metadata (see the argument list below).
135135
Force native tool calling off (`LLM_NATIVE_TOOL_CALLING=false`).
136136
Resume mode: ignored (uses metadata).
137137

138-
- `--send-reasoning-content`
139-
Send prior assistant `reasoning_content` back to the model in subsequent OpenHands requests.
140-
Useful for thinking models whose chat template supports reasoning history.
138+
- `--send-reasoning-content`
139+
Send prior assistant reasoning back to the model in subsequent OpenHands requests using both `reasoning_content` and `reasoning` fields.
140+
Useful for thinking models whose chat template supports reasoning history.
141141
Resume mode: ignored (uses metadata).
142142

143143
- `--litellm-extra-body`
144144
Pass a JSON object to OpenHands `LLM.litellm_extra_body`, for example `--litellm-extra-body '{"enable_thinking": true}'`.
145145
Resume mode: ignored (uses metadata).
146146

147+
- `--session-cache`
148+
Enable backend KV-cache session reuse for OpenHands by sending a unique `X-Session-Id` header for each task attempt and releasing it after the run with `DELETE /session_release?session_id=...`.
149+
Resume mode: ignored (uses metadata).
150+
147151
- `--max-iters`
148152
Maximum iterations for OpenHands (`OPENHANDS_MAX_ITERATIONS`).
149153
Default: no override (OpenHands default applies).

featurebench/cli.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,8 @@ def _run_with_patched_argv(
5656
if result is None:
5757
return 0
5858
return int(result)
59+
except KeyboardInterrupt:
60+
return 130
5961
except SystemExit as exc:
6062
code = exc.code
6163
if code is None:
@@ -126,4 +128,4 @@ def main(argv: Sequence[str] | None = None) -> int:
126128

127129

128130
if __name__ == "__main__":
129-
raise SystemExit(main())
131+
raise SystemExit(main())

featurebench/docker/image_manager.py

Lines changed: 176 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,14 @@
11
import logging
2+
import atexit
23
import subprocess
34
import tempfile
45
import os
56
import hashlib
67
import platform
78
import importlib.util
89
import shutil
10+
import signal
11+
import threading
912
from concurrent.futures import ThreadPoolExecutor, as_completed
1013
from typing import Dict, Optional, List, Tuple
1114
from pathlib import Path
@@ -24,6 +27,11 @@
2427
from featurebench.utils.utils import select_candidate_pool_and_allocate_gpu, release_gpu
2528

2629

30+
FEATUREBENCH_RUN_LABEL = "featurebench.run"
31+
FEATUREBENCH_KIND_LABEL = "featurebench.kind"
32+
FEATUREBENCH_TASK_LABEL = "featurebench.task"
33+
34+
2735
class ImageManager:
2836
"""Docker image manager."""
2937

@@ -52,6 +60,159 @@ def __init__(
5260

5361
# Track GPUs per container: {container_id: [gpu_id, ...]}
5462
self._container_gpu_map: Dict[str, List[int]] = {}
63+
64+
# Track containers created by fb data so Ctrl+C/process-exit cleanup can
65+
# remove them even if worker threads do not reach their own finally block.
66+
self._cleanup_run_id = f"data-{int(time.time() * 1000)}-{os.getpid()}"
67+
self._active_container_ids_lock = threading.RLock()
68+
self._active_container_ids: set[str] = set()
69+
self._cleanup_lock = threading.RLock()
70+
self._cleanup_in_progress = False
71+
self._cleanup_interrupt_notice_printed = False
72+
self._previous_sigint = None
73+
self._signal_cleanup_installed = False
74+
if threading.current_thread() is threading.main_thread():
75+
self._previous_sigint = signal.getsignal(signal.SIGINT)
76+
signal.signal(signal.SIGINT, self._handle_interrupt)
77+
self._signal_cleanup_installed = True
78+
self._atexit_cleanup = self._cleanup_active_containers_at_exit
79+
atexit.register(self._atexit_cleanup)
80+
81+
def _container_labels(self, specs_name: str, purpose: str = "data") -> Dict[str, str]:
82+
return {
83+
FEATUREBENCH_RUN_LABEL: self._cleanup_run_id,
84+
FEATUREBENCH_KIND_LABEL: "data",
85+
FEATUREBENCH_TASK_LABEL: str(specs_name),
86+
"featurebench.purpose": purpose,
87+
}
88+
89+
def _add_container_label_args(self, docker_command: List[str], specs_name: str) -> None:
90+
for key, value in self._container_labels(specs_name).items():
91+
docker_command.extend(["--label", f"{key}={value}"])
92+
93+
def _register_container_id(self, container_id: str) -> None:
94+
if not container_id:
95+
return
96+
with self._active_container_ids_lock:
97+
self._active_container_ids.add(container_id)
98+
99+
def _unregister_container_id(self, container_id: str) -> None:
100+
if not container_id:
101+
return
102+
with self._active_container_ids_lock:
103+
self._active_container_ids.discard(container_id)
104+
105+
def _ignore_interrupt_during_cleanup(self, signum, frame) -> None:
106+
if not self._cleanup_interrupt_notice_printed:
107+
self._cleanup_interrupt_notice_printed = True
108+
try:
109+
self.logger.warning("Cleanup already in progress; ignoring additional Ctrl+C.")
110+
except Exception:
111+
pass
112+
113+
def _handle_interrupt(self, signum, frame) -> None:
114+
try:
115+
self.logger.warning("Interrupted; cleaning FeatureBench data containers...")
116+
except Exception:
117+
pass
118+
self.cleanup_active_containers("keyboard interrupt")
119+
120+
previous = self._previous_sigint
121+
if callable(previous):
122+
previous(signum, frame)
123+
elif previous == signal.SIG_DFL:
124+
raise KeyboardInterrupt
125+
126+
def _release_container_gpu(self, container_id: str) -> None:
127+
gpu_ids = self._container_gpu_map.pop(container_id, None)
128+
if gpu_ids:
129+
release_gpu(gpu_ids, self.logger)
130+
self.logger.debug(f"Container {container_id[:12]} released GPUs {gpu_ids}")
131+
132+
def _remove_container_id_best_effort(self, container_id: str) -> bool:
133+
if not container_id:
134+
return False
135+
try:
136+
self._release_container_gpu(container_id)
137+
subprocess.run(["docker", "kill", container_id], capture_output=True)
138+
subprocess.run(["docker", "rm", "-f", container_id], capture_output=True)
139+
return True
140+
finally:
141+
self._unregister_container_id(container_id)
142+
143+
def _cleanup_labeled_containers(self) -> int:
144+
try:
145+
result = subprocess.run(
146+
[
147+
"docker",
148+
"ps",
149+
"-aq",
150+
"--filter",
151+
f"label={FEATUREBENCH_RUN_LABEL}={self._cleanup_run_id}",
152+
"--filter",
153+
f"label={FEATUREBENCH_KIND_LABEL}=data",
154+
],
155+
capture_output=True,
156+
text=True,
157+
check=False,
158+
)
159+
except Exception as exc:
160+
self.logger.warning(f"Failed to scan FeatureBench data containers by label: {exc}")
161+
return 0
162+
163+
if result.returncode != 0:
164+
stderr = (result.stderr or "").strip()
165+
if stderr:
166+
self.logger.warning(f"Failed to scan FeatureBench data containers by label: {stderr}")
167+
return 0
168+
169+
removed = 0
170+
for container_id in result.stdout.splitlines():
171+
if self._remove_container_id_best_effort(container_id.strip()):
172+
removed += 1
173+
return removed
174+
175+
def cleanup_active_containers(self, reason: str) -> int:
176+
with self._cleanup_lock:
177+
if self._cleanup_in_progress:
178+
return 0
179+
self._cleanup_in_progress = True
180+
181+
previous_sigint = None
182+
signal_replaced = False
183+
try:
184+
if threading.current_thread() is threading.main_thread():
185+
previous_sigint = signal.getsignal(signal.SIGINT)
186+
signal.signal(signal.SIGINT, self._ignore_interrupt_during_cleanup)
187+
signal_replaced = True
188+
189+
with self._active_container_ids_lock:
190+
container_ids = list(self._active_container_ids)
191+
192+
removed = 0
193+
if container_ids:
194+
self.logger.warning(
195+
f"Cleaning {len(container_ids)} active FeatureBench data container(s) after {reason}..."
196+
)
197+
for container_id in container_ids:
198+
if self._remove_container_id_best_effort(container_id):
199+
removed += 1
200+
201+
removed += self._cleanup_labeled_containers()
202+
if removed:
203+
self.logger.warning(f"Removed {removed} FeatureBench data container(s).")
204+
return removed
205+
finally:
206+
if signal_replaced:
207+
try:
208+
signal.signal(signal.SIGINT, previous_sigint)
209+
except Exception:
210+
pass
211+
with self._cleanup_lock:
212+
self._cleanup_in_progress = False
213+
214+
def _cleanup_active_containers_at_exit(self) -> None:
215+
self.cleanup_active_containers("process exit")
55216

56217
def prepare_images(self, repo_manager: RepoManager) -> None:
57218
"""
@@ -664,6 +825,7 @@ def run_container(
664825

665826
# Add runtime config to docker_command and compute env_setup + GPU IDs
666827
env_setup_cmd, selected_gpu_ids = self._add_docker_runtime_config(docker_command, specs)
828+
self._add_container_label_args(docker_command, specs_name)
667829

668830
# Add image
669831
docker_command.append(instance_image)
@@ -695,6 +857,7 @@ def run_container(
695857
)
696858

697859
container_id = result.stdout.strip()
860+
self._register_container_id(container_id)
698861

699862
# Record GPUs used by container (if any)
700863
if selected_gpu_ids:
@@ -1097,17 +1260,16 @@ def stop_container(self, container_id: str, force: bool = False) -> None:
10971260
container_id: Container ID
10981261
force: Force kill without graceful stop (use on timeout)
10991262
"""
1100-
# Release GPUs used by container before stop
1101-
gpu_ids = self._container_gpu_map.get(container_id)
1102-
if gpu_ids:
1103-
release_gpu(gpu_ids, self.logger)
1104-
del self._container_gpu_map[container_id]
1105-
self.logger.debug(f"Container {container_id[:12]} released GPUs {gpu_ids}")
1106-
1107-
if force:
1108-
# Force kill container (no wait)
1109-
subprocess.run(['docker', 'kill', container_id], capture_output=True)
1110-
else:
1111-
# Graceful stop
1112-
subprocess.run(['docker', 'stop', container_id], capture_output=True)
1113-
subprocess.run(['docker', 'rm', container_id], capture_output=True)
1263+
try:
1264+
# Release GPUs used by container before stop
1265+
self._release_container_gpu(container_id)
1266+
1267+
if force:
1268+
# Force kill container (no wait)
1269+
subprocess.run(['docker', 'kill', container_id], capture_output=True)
1270+
else:
1271+
# Graceful stop
1272+
subprocess.run(['docker', 'stop', container_id], capture_output=True)
1273+
subprocess.run(['docker', 'rm', container_id], capture_output=True)
1274+
finally:
1275+
self._unregister_container_id(container_id)

featurebench/harness/container.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,7 @@ def create_container(
190190
gpu_ids: str | None = None,
191191
proxy_port: int | None = None,
192192
docker_runtime_config: dict | None = None,
193+
labels: dict[str, str] | None = None,
193194
) -> Container:
194195
"""
195196
Create and start a Docker container for evaluation.
@@ -201,6 +202,7 @@ def create_container(
201202
gpu_ids: Comma-separated GPU IDs to use
202203
proxy_port: Proxy port for network access (uses bridge network with Docker host gateway)
203204
docker_runtime_config: Runtime config from repo_settings (need_gpu, shm_size, env_vars, env_exports, number_once)
205+
labels: Docker labels to attach to the container
204206
205207
Returns:
206208
Docker container object
@@ -297,6 +299,7 @@ def create_container(
297299
environment=environment,
298300
network_mode=network_mode,
299301
shm_size=shm_size,
302+
labels=labels,
300303
)
301304

302305
# Check if GPU is available
@@ -365,5 +368,4 @@ def cleanup_container(self, container: Container) -> None:
365368
container.remove()
366369
self.logger.info("Container removed successfully")
367370
except Exception as e:
368-
self.logger.warning(f"Failed to cleanup container: {e}")
369-
371+
self.logger.warning(f"Failed to cleanup container: {e}")

featurebench/harness/review_codes.py

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ def save_review_codes_level1(
135135
log_dir: Path,
136136
docker_image: str,
137137
logger: logging.Logger,
138+
labels: dict[str, str] | None = None,
138139
) -> None:
139140
"""
140141
Save review codes for Level 1 instance.
@@ -176,6 +177,7 @@ def save_review_codes_level1(
176177
name=container_name,
177178
detach=True,
178179
remove=False,
180+
labels=labels,
179181
)
180182

181183
# Step 1: Restore project
@@ -290,6 +292,7 @@ def save_review_codes_level2(
290292
log_dir: Path,
291293
docker_image: str,
292294
logger: logging.Logger,
295+
labels: dict[str, str] | None = None,
293296
) -> None:
294297
"""
295298
Save review codes for Level 2 instance.
@@ -332,6 +335,7 @@ def save_review_codes_level2(
332335
name=container_name,
333336
detach=True,
334337
remove=False,
338+
labels=labels,
335339
)
336340

337341
# Step 1: Clean /testbed/ and create README.md
@@ -402,4 +406,4 @@ def save_review_codes_level2(
402406
container.stop(timeout=10)
403407
container.remove()
404408
except Exception as e:
405-
logger.warning(f"Failed to cleanup review container: {e}")
409+
logger.warning(f"Failed to cleanup review container: {e}")

0 commit comments

Comments
 (0)