From 235e738a5dc725d5d421ad32e5d77657ea73df6d Mon Sep 17 00:00:00 2001 From: divesh Date: Fri, 6 Nov 2026 11:14:23 -0800 Subject: [PATCH] fix(supervisor): stop recovering a lost boundary after SIGTERM After SIGTERM the supervisor signals, waits for, and terminates the boundary. When the boundary connection drops during that sequence, each call retried reattach for up to 30s, so the supervisor outlived the runtime's stop timeout and was killed. Once shutdown starts, report a lost connection instead of recovering it. Un-ignore the conformance test that covers slow-TERM deletes. Signed-off-by: divesh --- .../openshell-sandbox-backend/src/runtime.rs | 81 +++++++++++++++++++ crates/openshell-supervisor/src/lib.rs | 1 + tests/suites/conformance/src/lifecycle.rs | 4 - 3 files changed, 82 insertions(+), 4 deletions(-) diff --git a/crates/openshell-sandbox-backend/src/runtime.rs b/crates/openshell-sandbox-backend/src/runtime.rs index 9ea72d396c..319a98ce8c 100644 --- a/crates/openshell-sandbox-backend/src/runtime.rs +++ b/crates/openshell-sandbox-backend/src/runtime.rs @@ -958,6 +958,12 @@ impl RunningBoundary { expect_response(response, "boundary_terminated") } + /// Stop waiting for a lost boundary connection to recover. The owner is + /// stopping, and the boundary may be stopping with it. + pub fn begin_stop(&self) { + self.process.client.stopping.store(true, Ordering::Release); + } + /// Whether the negotiated backend advertised suspend/resume support. pub fn supports_suspend(&self) -> bool { self.process @@ -1601,6 +1607,9 @@ struct BoundaryClient { recovery_timeout_secs: Option, /// True once a connection of this client was confirmed. confirmed: tokio::sync::watch::Sender, + /// Set when the owner stops the boundary; a lost connection is then + /// reported instead of recovered. + stopping: AtomicBool, } #[derive(Clone)] @@ -1800,6 +1809,7 @@ impl BoundaryClient { requires_takeover: false, recovery_timeout_secs: None, confirmed: tokio::sync::watch::Sender::new(false), + stopping: AtomicBool::new(false), } } @@ -2208,6 +2218,11 @@ impl BoundaryClient { } cached.transport.abort(); } + if self.stopping.load(Ordering::Acquire) { + return Err(BackendError::Unavailable( + "boundary connection lost while stopping".to_string(), + )); + } *self.mediation.lock().await = None; let attach = self @@ -3970,6 +3985,72 @@ mod tests { assert_eq!(requests.load(Ordering::Acquire), 5); } + #[tokio::test] + async fn stopping_client_reports_a_lost_boundary_without_recovery() { + let certificate = test_certificate(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let address = listener.local_addr().unwrap(); + let server_config = certificate.server_config.clone(); + let server = tokio::spawn(async move { + let (stream, _) = listener.accept().await.unwrap(); + let stream = tokio_rustls::TlsAcceptor::from(server_config) + .accept(stream) + .await + .unwrap(); + let service = TestGrpcBoundary { + wait_for_half_close: false, + expected_token: "a".repeat(32), + requests: Arc::default(), + mediation_failures: Arc::new(std::sync::atomic::AtomicUsize::new(0)), + mediation_ready: false, + provider_environment_generation: 0, + confirmation: test_confirmation(), + attach_snapshots: Arc::default(), + received: Arc::default(), + }; + tonic::transport::Server::builder() + .add_service(IsolationBackendServer::new(TestIsolationBackend::default())) + .add_service(SandboxProtocolServer::new(service)) + .serve_with_incoming(tokio_stream::iter([Ok::<_, std::io::Error>(TestTlsIo( + Box::new(stream), + ))])) + .await + .unwrap(); + }); + let client = BoundaryClient::new( + tls_runtime_descriptor(address, certificate.client_tls), + test_bearer(&"a".repeat(32)), + ); + let attach = Request::Attach { + supervisor_instance_id: client.supervisor_instance_id, + policy: Box::new(SandboxPolicyWire::from(sandbox().policy)), + supervisor_takeover: None, + supervisor_recovery_timeout_secs: None, + }; + client.call_idempotent(attach).await.unwrap(); + client.call_idempotent(Request::Confirm).await.unwrap(); + + // The boundary stops with its owner: no listener remains to reattach. + server.abort(); + let _ = server.await; + client.stopping.store(true, Ordering::Release); + client + .grpc_channel + .lock() + .await + .as_ref() + .expect("cached channel") + .transport + .abort(); + let result = tokio::time::timeout( + Duration::from_secs(5), + client.call_idempotent(Request::TerminateBoundary), + ) + .await + .expect("a stopping client must not wait for the boundary to recover"); + assert!(matches!(result, Err(BackendError::Unavailable(_)))); + } + #[test] fn wait_recovery_window_begins_at_transport_failure() { let wait_started = tokio::time::Instant::now(); diff --git a/crates/openshell-supervisor/src/lib.rs b/crates/openshell-supervisor/src/lib.rs index b9a5546bf2..68d9126c35 100644 --- a/crates/openshell-supervisor/src/lib.rs +++ b/crates/openshell-supervisor/src/lib.rs @@ -1800,6 +1800,7 @@ pub async fn run_sandbox_instance( )); } () = &mut shutdown_requested => { + running.begin_stop(); let _ = agent .signal(openshell_isolation_interface::contract::BoundarySignal::Term) .await; diff --git a/tests/suites/conformance/src/lifecycle.rs b/tests/suites/conformance/src/lifecycle.rs index dd47c0b4d6..6af1d145b3 100644 --- a/tests/suites/conformance/src/lifecycle.rs +++ b/tests/suites/conformance/src/lifecycle.rs @@ -52,11 +52,7 @@ const DELETE_DEADLINE: Duration = Duration::from_secs(10); /// Many workloads do not exit immediately on TERM, for example while they run /// cleanup handlers. Deleting such a running sandbox should still complete /// promptly, rather than waiting out the runtime's termination grace period. -// -// Unignore this when the supervisor half of #5025 lands: on Podman the -// supervisor container still waits out its stop timeout. #[testkit::test] -#[ignore = "https://github.com/NVIDIA/OpenShell/issues/5025"] async fn delete_is_prompt_when_workload_is_slow_to_handle_term( cx: &mut Context, ) -> testkit::Result {