Skip to content

Commit 91b6ccf

Browse files
committed
fix(supervisor): bound SIGTERM shutdown and give Docker supervisors 10s
Stopping a dedicated sandbox now finishes within 8 seconds, including the exit report and trace flush, while the workload keeps its 5 second TERM grace. A boundary connection lost while stopping is reported as a terminated workload instead of failing the shutdown. Only a dedicated supervisor's own shutdown skips boundary recovery; a shared supervisor removing one sandbox keeps recovering as before. The Docker driver gave the supervisor 5 seconds, the same as its wait for the workload, so stops ended in SIGKILL. It now allows 10 seconds. Signed-off-by: divesh <dgude@nvidia.com>
1 parent 235e738 commit 91b6ccf

5 files changed

Lines changed: 76 additions & 26 deletions

File tree

‎crates/openshell-driver-docker/README.md‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ New generations require matching supervisor and sandbox runtime binaries.
128128

129129
## Stop, Start, and Delete
130130

131-
Stop terminates the supervisor companion and stops the workload container
132-
without removing it. Docker retains the workload writable layer and attached
131+
Stop terminates the supervisor companion, allowing it 10 seconds to stop the
132+
workload, and stops the workload container without removing it. Docker retains the workload writable layer and attached
133133
volumes. Start stages a fresh sandbox bootstrap bundle, restarts that workload,
134134
and creates a new supervisor companion. A durably stopped sandbox stays stopped
135135
across gateway restarts.

‎crates/openshell-driver-docker/src/lib.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,9 @@ const WATCH_POLL_MAX_BACKOFF: Duration = Duration::from_secs(30);
9595
// request a bounded opportunity to mark the control exit intentional before
9696
// publishing a fail-closed runtime error.
9797
const SUPERVISOR_INTENTIONAL_SHUTDOWN_GRACE: Duration = Duration::from_secs(1);
98+
// Longer than the supervisor's own bound on stopping its workload, so it
99+
// finishes stopping before Docker kills it.
100+
const SUPERVISOR_STOP_TIMEOUT_SECS: i32 = 10;
98101
const SUPERVISOR_HEALTH_INTERVAL_NS: i64 = 30_000_000_000;
99102
// Docker probes at this interval only before the first healthy result and
100103
// within the start period.
@@ -5607,7 +5610,7 @@ async fn spawn_docker_control_process(
56075610
_ = &mut shutdown_requested => {
56085611
let _ = monitored_docker.stop_container(
56095612
&monitored_supervisor_id,
5610-
Some(StopContainerOptionsBuilder::default().t(5).build()),
5613+
Some(StopContainerOptionsBuilder::default().t(SUPERVISOR_STOP_TIMEOUT_SECS).build()),
56115614
).await;
56125615
let _ = remove_supervisor().await;
56135616
}

‎crates/openshell-supervisor/README.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,10 @@ loads the launch configuration from its arguments and environment and prepares
5757
push and the OCSF JSON settings with its logging layers, so renewal by either
5858
gateway client keeps both current. `run_sandbox_with_backend` sets the
5959
process-wide OCSF context, binds the readiness socket and port to instance
60-
readiness, and stops the instance on SIGTERM or SIGINT.
60+
readiness, and stops the instance on SIGTERM or SIGINT. Stopping gives the
61+
workload 5 seconds to exit after TERM and finishes, including the exit report
62+
and trace flush, within 8 seconds. A boundary connection lost while the
63+
process stops is not recovered; the workload is reported as terminated.
6164

6265
`sandbox_instance_host::SandboxInstanceHost` runs several sandbox instances in
6366
one process. Each instance gets its own Tokio runtime; every thread of that

‎crates/openshell-supervisor/src/lib.rs‎

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -504,6 +504,17 @@ use provider_readiness::{EnvironmentIdentity, Tracker as ProviderReadinessTracke
504504
use tokio::sync::mpsc::UnboundedSender;
505505
use tokio::time::timeout;
506506

507+
/// Time the workload gets to exit after TERM before it is killed.
508+
const WORKLOAD_TERM_GRACE: Duration = Duration::from_secs(5);
509+
510+
/// Bound on stopping an instance, including its exit report and trace flush.
511+
/// Below the 10 second grace the Docker driver gives the supervisor.
512+
const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(8);
513+
514+
/// Exit code reported when the boundary is lost before it reports the
515+
/// stopped workload's status: terminated by TERM.
516+
const STOPPED_EXIT_CODE: i32 = 128 + 15;
517+
507518
/// Whether a dedicated supervisor's environment grants the SSH socket to its
508519
/// group rather than its owner.
509520
#[must_use]
@@ -836,6 +847,7 @@ pub async fn run_sandbox_with_backend(
836847
bindings,
837848
SandboxInstanceControls {
838849
shutdown: Box::pin(wait_for_control_shutdown_signal()),
850+
shutdown_ends_process: true,
839851
suspend,
840852
readiness,
841853
},
@@ -999,6 +1011,9 @@ pub struct SandboxInstanceControls {
9991011
/// Completes when the owner asks the instance to stop its workload. It is
10001012
/// first polled once the workload has started.
10011013
pub shutdown: Pin<Box<dyn Future<Output = ()> + Send>>,
1014+
/// True when `shutdown` also ends this process, so its boundary may be
1015+
/// stopping too and a lost connection is not worth recovering.
1016+
pub shutdown_ends_process: bool,
10021017
/// Requests to suspend only this instance. A failed suspension can be
10031018
/// retried by sending another request. Closing the stream disables it.
10041019
pub suspend: Pin<Box<dyn futures::Stream<Item = ()> + Send>>,
@@ -1110,6 +1125,7 @@ pub async fn run_sandbox_instance(
11101125
} = bindings;
11111126
let SandboxInstanceControls {
11121127
shutdown: shutdown_requested,
1128+
shutdown_ends_process,
11131129
mut suspend,
11141130
readiness,
11151131
} = controls;
@@ -1781,6 +1797,7 @@ pub async fn run_sandbox_instance(
17811797
SandboxInstanceReadiness::start(readiness, boundary_access.session_readiness());
17821798
let instance_id = boundary_access.instance_id().to_string();
17831799
let mut shutdown_requested = shutdown_requested;
1800+
let mut shutdown_deadline = None;
17841801
// `None` means the boundary was suspended and its reference persisted.
17851802
let lifecycle = loop {
17861803
let wait_agent = agent.clone();
@@ -1800,24 +1817,42 @@ pub async fn run_sandbox_instance(
18001817
));
18011818
}
18021819
() = &mut shutdown_requested => {
1803-
running.begin_stop();
1804-
let _ = agent
1805-
.signal(openshell_isolation_interface::contract::BoundarySignal::Term)
1806-
.await;
1807-
let status = if let Ok(result) = timeout(Duration::from_secs(5), agent.wait()).await {
1808-
result
1809-
} else {
1810-
let _ = agent.terminate().await;
1811-
agent.wait().await
1820+
let deadline = tokio::time::Instant::now() + SHUTDOWN_TIMEOUT;
1821+
shutdown_deadline = Some(deadline);
1822+
if shutdown_ends_process {
1823+
running.begin_stop();
18121824
}
1813-
.map_err(|error| miette::miette!(error.to_string()))?;
1814-
running
1815-
.terminate()
1816-
.await
1817-
.map_err(|error| miette::miette!(
1818-
"sandbox did not acknowledge terminal state: {error}"
1819-
))?;
1820-
break Some((boundary_exit_code(status), false));
1825+
let stop = async {
1826+
let _ = agent
1827+
.signal(openshell_isolation_interface::contract::BoundarySignal::Term)
1828+
.await;
1829+
let status = if let Ok(result) = timeout(WORKLOAD_TERM_GRACE, agent.wait()).await {
1830+
result
1831+
} else {
1832+
let _ = agent.terminate().await;
1833+
agent.wait().await
1834+
}?;
1835+
running.terminate().await?;
1836+
Ok(status)
1837+
};
1838+
let exit_code = match tokio::time::timeout_at(deadline, stop).await {
1839+
Ok(Ok(status)) => boundary_exit_code(status),
1840+
Ok(Err(
1841+
openshell_isolation_interface::contract::BackendError::Unavailable(error)
1842+
| openshell_isolation_interface::contract::BackendError::Terminated(error),
1843+
)) => {
1844+
warn!(%error, "Isolation boundary lost while stopping; finishing shutdown");
1845+
STOPPED_EXIT_CODE
1846+
}
1847+
Ok(Err(error)) => {
1848+
return Err(miette::miette!("stop sandbox workload: {error}"));
1849+
}
1850+
Err(_) => {
1851+
warn!("Isolation boundary did not stop in time; finishing shutdown");
1852+
STOPPED_EXIT_CODE
1853+
}
1854+
};
1855+
break Some((exit_code, false));
18211856
}
18221857
// The agent wait is cancelled while suspending so a fenced
18231858
// generation's stream failure cannot race the suspension.
@@ -1854,19 +1889,27 @@ pub async fn run_sandbox_instance(
18541889
if !retain_access {
18551890
instance_readiness.release();
18561891
}
1857-
boundary_access
1858-
.publish_main_exit(exit_code, await_main_process_attachment)
1859-
.await;
1892+
let publish = boundary_access.publish_main_exit(exit_code, await_main_process_attachment);
1893+
if let Some(deadline) = shutdown_deadline {
1894+
let _ = tokio::time::timeout_at(deadline, publish).await;
1895+
} else {
1896+
publish.await;
1897+
}
1898+
let flush_timeout = shutdown_deadline.map_or(otlp_relay::FLUSH_TIMEOUT, |deadline| {
1899+
deadline
1900+
.saturating_duration_since(tokio::time::Instant::now())
1901+
.min(otlp_relay::FLUSH_TIMEOUT)
1902+
});
18601903
// Flush relayed agent traces before the exit is reported so the last
18611904
// spans of a short run are not lost. Bounded; never fails. When the
18621905
// access plane is retained for exec sessions, the relay keeps serving
18631906
// them and shuts down at final teardown instead.
18641907
if retain_access {
18651908
if let Some(relay) = otlp_relay.as_ref() {
1866-
relay.flush(otlp_relay::FLUSH_TIMEOUT).await;
1909+
relay.flush(flush_timeout).await;
18671910
}
18681911
} else if let Some(relay) = otlp_relay.take() {
1869-
relay.shutdown(otlp_relay::FLUSH_TIMEOUT).await;
1912+
relay.shutdown(flush_timeout).await;
18701913
}
18711914
// `shutdown_requested` has already completed when shutdown won the
18721915
// lifecycle select above and must not be polled again.

‎crates/openshell-supervisor/src/sandbox_instance_host.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -689,6 +689,7 @@ fn run_hosted_sandbox_instance(
689689
});
690690
let instance = body(SandboxInstanceControls {
691691
shutdown,
692+
shutdown_ends_process: false,
692693
suspend: Box::pin(tokio_stream::wrappers::ReceiverStream::new(suspend_requested)),
693694
readiness,
694695
});

0 commit comments

Comments
 (0)