Skip to content

Commit 0e4ed61

Browse files
ZhiXiao-LinRoy Lin
andauthored
fix(pool): close warm-pool shutdown-vs-replenish push race (orphaned-VM leak) (#145)
From the concurrency audit. A freshly-booted warm VM could be pushed into the idle pool AFTER drain_idle already cleared it, leaking the VM (mount/IP/volume) since there is no Drop reaper. The replenish task checked `*shutdown_rx.borrow()` and only THEN acquired the idle lock to push — with an await in between. drain_all does signal_shutdown() then drain_idle() (which locks idle, drains, and runs exactly once). If the unlocked check observed shutdown=false and drain_idle completed before the task acquired the lock, the VM was pushed into the already-drained pool and never reclaimed. Fix: acquire the idle lock FIRST, then re-check shutdown UNDER it before pushing — making the check-and-push atomic against drain_idle (which holds the same lock while draining, always after signal_shutdown). On shutdown, destroy the VM instead of pushing. The same latent leak existed in `release()` (returns a VM to the pool with no shutdown check); guard it the same way. Validated: full a3s-box-runtime pool/lib suite (55) green, fmt + clippy clean. A minimal check-under-lock change; the interleaving itself needs a real-VM harness to exercise (like the kill-identity / durable-write fixes). Co-authored-by: Roy Lin <roylin@a3s.box>
1 parent 8afe462 commit 0e4ed61

1 file changed

Lines changed: 19 additions & 2 deletions

File tree

src/runtime/src/pool/warm_pool.rs

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -259,6 +259,17 @@ impl WarmPool {
259259
pub async fn release(&self, vm: VmManager) -> Result<()> {
260260
let mut idle = self.idle.lock().await;
261261

262+
// Don't return a VM to a pool that is shutting down: drain_idle has (or
263+
// soon will have) cleared `idle` and won't run again, so a push here leaks
264+
// the VM (no Drop reaper). Checked under the idle lock so it is atomic with
265+
// a concurrent drain_idle. Destroy the VM instead.
266+
if *self.shutdown_rx.borrow() {
267+
drop(idle);
268+
let mut vm = vm;
269+
vm.destroy().await?;
270+
return Ok(());
271+
}
272+
262273
if idle.len() >= self.config.max_size {
263274
// Pool is full — destroy the VM
264275
drop(idle); // Release lock before async destroy
@@ -809,16 +820,22 @@ impl WarmPool {
809820
// booting, drain_idle has already cleared
810821
// `idle` and will not run again, so a VM
811822
// pushed now leaks (no Drop reaper). Destroy
812-
// it instead.
823+
// it instead. Acquire the idle lock FIRST and
824+
// re-check shutdown UNDER it: drain_idle drains
825+
// while holding this same lock (always after
826+
// signal_shutdown), so the check-and-push is
827+
// atomic against it — closing the TOCTOU window
828+
// that an unlocked `borrow()` check left open.
829+
let mut pool = idle.lock().await;
813830
if *shutdown_rx.borrow() {
831+
drop(pool);
814832
tracing::debug!(
815833
box_id = %box_id,
816834
"Pool shutting down mid-replenish; destroying freshly-booted VM"
817835
);
818836
let _ = vm.destroy_with_timeout(2000).await;
819837
continue;
820838
}
821-
let mut pool = idle.lock().await;
822839
pool.push(WarmVm {
823840
vm,
824841
created_at: Instant::now(),

0 commit comments

Comments
 (0)