Skip to content

fix(server): prevent unrelated sandbox deletes from blocking deletion events #2326

Description

@pimlock

Agent Diagnostic

  • Loaded the repo-local create-spike workflow and used a principal-engineer review of the CLI, gateway compute runtime, Podman driver/watcher, supervisor process path, E2E harness, and architecture docs.
  • Inspected the linked failure and searched GitHub Actions logs for the exact sandbox_create_with_no_keep_cleans_up_after_tty_command assertion. Found at least seven exact failures between July 6 and July 16, 2026, across unrelated PRs and merge-queue candidates.
  • Confirmed the linked PR's AWS STS changes do not intersect the sandbox delete, Podman event, supervisor shutdown, or E2E lifecycle paths.
  • Confirmed the same SHA passed on rerun, including the linked PR's attempts 2 and 3.
  • Searched open and closed repository issues for the test name, assertion, --no-keep, Podman deletion races, and DeleteSandbox; no duplicate was found.
  • Latest release testing is not applicable because this is a current Branch E2E failure against unreleased PR and merge-queue SHAs.

Problem Statement

The rootless Podman E2E test sandbox_create_with_no_keep_cleans_up_after_tty_command flakes because sandbox deletion can take more than 20 seconds to become absent from ListSandboxes. A slow delete holds a process-wide gateway mutex across the external Podman call, so an unrelated sandbox delete blocks watch-driven cleanup for the first sandbox. Host-native rootless Podman amplifies the race because the process-enabled supervisor runs as container PID 1 without SIGTERM handling, causing each stop to consume the full 10-second timeout before SIGKILL.

The test's 10-second observation window is equal to one Podman stop timeout. It also sleeps after its final poll without checking once more, so it can fail even when deletion completes during that final sleep.

Reproduction Steps

  1. Run the Rust E2E suite against rootless Podman on Ubuntu 24.04.
  2. Let the two tests in e2e/rust/tests/sandbox_lifecycle.rs execute concurrently.
  3. Both tests create a sandbox and eventually delete it. The default-keep test performs explicit cleanup while the no-keep CLI path performs automatic cleanup.
  4. Observe Podman wait 10 seconds for SIGTERM before using SIGKILL, while each gateway delete holds the shared ComputeRuntime::sync_lock.
  5. Observe the no-keep sandbox remain visible as Deleting while its watcher events wait behind the sibling sandbox deletion. The fixed 20 x 500 ms poll can then expire.

CI Occurrences

The following jobs contain the exact should have been deleted automatically assertion:

Date (UTC) Context Job
2026-07-16 PR #1782, AWS STS 87501178906
2026-07-15 Merge queue for PR #2288 87491941822
2026-07-15 PR #1782, earlier SHA 87251810998
2026-07-15 Merge queue for PR #2302 87486355113
2026-07-15 PR #2027, supervisor middleware 87245713093
2026-07-10 PR #2216, Helm supervisor image 86439898310
2026-07-06 PR #2153, compute readiness 85442993990

PR #2216's run passed on attempt 2, which shows that searching only final workflow conclusions misses occurrences hidden by reruns.

Technical Context

Observed timeline from the linked job

For no-keep sandbox meek-rabbit:

  1. Podman deletion starts at 00:07:35.549.
  2. Podman cannot stop PID 1 with SIGTERM, waits 10 seconds, then uses SIGKILL.
  3. DeleteSandbox returns success at 00:07:45.769 after 10.225 seconds.
  4. The sibling test starts deleting resilient-manakin and holds the same gateway synchronization mutex for another Podman stop timeout.
  5. meek-rabbit remains in the store while its watch events wait for the mutex.
  6. At 00:07:56.437, after the sibling delete releases the lock, a queued event changes meek-rabbit from Deleting to Error with ContainerExited and exit code 137.
  7. The test fails at 00:07:56.930.

Other occurrences show the same pattern: a 10-second Podman stop timeout, successful DeleteSandbox, and either a late Deleting to Error / Provisioning transition or delayed deleted-event cleanup.

Architecture and data flow

  1. sandbox create --no-keep runs the TTY command and finalize_sandbox_create_session synchronously calls sandbox_delete.
  2. ComputeRuntime::delete_sandbox takes one process-wide sync_lock, writes phase Deleting, and keeps the mutex while awaiting the compute driver's DeleteSandbox RPC.
  3. The Podman driver sends SIGTERM with stop_timeout_secs = 10, force-removes the container, and returns whether the container existed.
  4. When the driver returns true, the gateway does not synchronously remove the sandbox record. It relies on the Podman watcher remove event and apply_deleted to remove the record.
  5. Podman stop / die events produce status snapshots. remove produces the deleted event. Applying either event requires the same process-wide sync_lock.
  6. ListSandboxes returns all persisted sandbox rows, including Deleting, so delayed event application remains user-visible after the delete RPC reports success.
  7. Podman starts /opt/openshell/bin/openshell-sandbox directly as container PID 1. The process-enabled supervisor awaits the entrypoint child but does not select on SIGINT / SIGTERM. The existing shutdown-signal helper is used only for network-only mode, so Linux PID 1 does not take the normal default SIGTERM action.

Affected Components

Component Key files Role
CLI lifecycle crates/openshell-cli/src/run.rs Runs the session and synchronously requests no-keep deletion.
Gateway compute runtime crates/openshell-server/src/compute/mod.rs Serializes lifecycle/store mutations, invokes drivers, and applies watch events.
Podman driver and watcher crates/openshell-driver-podman/src/driver.rs, crates/openshell-driver-podman/src/watcher.rs, crates/openshell-driver-podman/src/container.rs Stops/removes containers and translates Podman events into snapshots/deletions.
E2E coverage e2e/rust/tests/sandbox_lifecycle.rs, .github/workflows/e2e-test.yml Executes concurrent lifecycle tests on host-native rootless Podman.

Code References

Location Description
crates/openshell-cli/src/run.rs:1744 finalize_sandbox_create_session awaits automatic deletion when the sandbox should not persist.
crates/openshell-cli/src/run.rs:2345 TTY command completion enters the no-keep finalizer.
crates/openshell-server/src/compute/mod.rs:566 delete_sandbox holds the process-wide sync_lock across the driver RPC.
crates/openshell-server/src/compute/mod.rs:598 Store deletion happens synchronously only when the driver reports deleted = false.
crates/openshell-server/src/compute/mod.rs:1083 Watch snapshots and deleted events feed the same runtime state machine.
crates/openshell-server/src/compute/mod.rs:1112 Applying a watch snapshot waits for the same sync_lock.
crates/openshell-server/src/compute/mod.rs:1347 Applying a deleted event also waits for the same sync_lock.
crates/openshell-driver-podman/src/driver.rs:655 Podman delete waits for the configured stop timeout before removal.
crates/openshell-driver-podman/src/watcher.rs:184 remove maps to deletion while stop / die map to status snapshots.
crates/openshell-driver-podman/src/container.rs:895 The supervisor binary is installed directly as container entrypoint / PID 1.
crates/openshell-sandbox/src/lib.rs:622 Process-enabled mode only awaits run_process.
crates/openshell-sandbox/src/lib.rs:738 SIGINT / SIGTERM handling is explicitly limited to network-only mode.
e2e/rust/tests/sandbox_lifecycle.rs:104 The no-keep E2E test uses a fixed 20 x 500 ms loop and sleeps after the final poll.
.github/workflows/e2e-test.yml:95 Rootless Podman runs directly on Ubuntu hosts, where this timing is visible.

Current Behavior

  • Deleting sandbox A can block lifecycle events for unrelated sandbox B because external driver latency occurs inside one process-wide critical section.
  • DeleteSandbox can report success while the sandbox remains visible in ListSandboxes until a watcher event acquires that mutex.
  • Queued stop / die snapshots can regress a record from Deleting to Error or Provisioning before the later remove event cleans it up.
  • Process-enabled Podman sandboxes routinely require SIGKILL after 10 seconds because the PID 1 supervisor does not handle SIGTERM.
  • The E2E test's observation budget is exactly one stop timeout and can miss deletion during its final sleep.

Proposed Approach

Treat this as a gateway lifecycle synchronization fix, not a test-only retry:

  1. Do not hold the process-wide sandbox synchronization mutex across the external driver delete await. Preserve same-sandbox serialization or idempotency with an ID-scoped in-flight gate or equivalent mechanism while allowing watcher events for unrelated sandboxes to proceed.
  2. Keep the durable record in Deleting while the driver request is in flight, then explicitly reconcile or recover it if the driver returns an error. Preserve event-driven deletion semantics for Kubernetes, where successful delete may mean accepted rather than physically complete.
  3. Make delete reconciliation monotonic. Stale stop / die snapshots must not incorrectly revive a completed deletion, while genuine driver delete failures must still be allowed to recover from Deleting.
  4. Keep the E2E tests parallel as the regression scenario, but replace the fixed loop with a named deadline helper that performs a final check and has a budget greater than the configured stop timeout. Do not serialize the tests as the only fix, because that hides cross-sandbox head-of-line blocking.

Process-enabled PID 1 signal forwarding is deliberately out of scope. It explains the consistent 10-second Podman delay and deserves a separate product issue, but slow driver operations can still occur and must not block unrelated sandbox lifecycle events.

Alternative Approaches Considered

  • Only increase retries: reduces CI noise but preserves slow PID 1 shutdown, global lock contention, and non-monotonic lifecycle state.
  • Only serialize the two tests: avoids the observed overlap but hides the production behavior when users delete different sandboxes concurrently.
  • Fix PID 1 signal forwarding in this issue: would reduce the common Podman delay but bundles a separate process-lifecycle change and would not protect unrelated sandboxes from other slow driver operations.
  • Release the global lock without same-sandbox coordination: addresses head-of-line blocking but permits duplicate deletes and unsafe watcher interleavings for one sandbox.

Patterns to Follow

  • Continue using CAS/resource versions for persisted phase transitions. The gateway architecture documents update_message_cas as the concurrency boundary for internal compute phase updates.
  • Preserve the existing Deleting / Error terminal-state protections used by supervisor-session updates.
  • Keep Podman watcher ordering explicit: stop / die are status observations, while remove is resource deletion.
  • Use deterministic fake-driver synchronization in unit tests rather than wall-clock sleeps to prove that one blocked delete cannot delay another sandbox's event.

Scope Assessment

  • Complexity: Medium
  • Confidence: High on the failure mechanism; medium on the final synchronization design
  • Estimated files to change: 3 to 5
  • Issue type: fix
  • Documentation impact: Update architecture/compute-runtimes.md if delete completion or driver/watch ownership changes. No gateway TOML field or published config reference change is expected.
  • LSM impact: No direct SELinux/AppArmor policy change is required.

Risks and Open Questions

  • What is the public contract of successful DeleteSandbox: driver resource accepted for deletion, driver resource removed, or gateway record absent? Kubernetes deletion and finalizers make synchronous physical removal an unsafe assumption, while the CLI currently prints Deleted sandbox, so delayed list visibility is surprising.
  • Releasing the process-wide mutex around the driver RPC introduces same-sandbox races unless deletion ownership is explicit and stale watch snapshots are rejected.
  • The current test apply_sandbox_update_allows_delete_failures_to_recover permits Deleting to become Ready. The fix must distinguish a failed delete that should recover from an in-flight or completed delete that must remain monotonic.
  • HA gateways cannot rely only on an in-memory tombstone. Any deletion generation/ownership used for correctness must fit the persisted CAS model or be clearly limited to eliminating local queued events after the backend has been removed.

Test Considerations

  • Add a gateway unit test with two sandbox IDs and a fake driver that blocks deletion A. Prove deletion/watch cleanup for B is not blocked by A.
  • Add state-machine tests for stale stop / die snapshots arriving before and after the remove event. A completed delete must not regress or resurrect the row.
  • Keep the rootless Podman E2E test concurrent and make its eventual assertion deadline-based with a final check.
  • Run the focused server unit/integration suites, the rootless Podman E2E path on Ubuntu 24.04 and 26.04, and the broader sandbox E2E path because this changes compute lifecycle behavior.

Created by spike investigation. Use build-from-issue to plan and implement.

Metadata

Metadata

Assignees

Labels

Type

No type

Fields

No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions