Skip to content

Commit f3beffe

Browse files
committed
fix(mxc): preserve ProcessContainer loopback readiness
Observe the target listener from the host MXC driver because an AppContainer cannot safely probe a not-yet-listening loopback port or query the Windows TCP table. Confirm readiness over the versioned relay control channel while preserving target-exit diagnostics, shutdown cancellation, and the full five-minute cold-start budget. Signed-off-by: Prekshi Vyas <prekshiv@nvidia.com>
1 parent 348d4ed commit f3beffe

5 files changed

Lines changed: 341 additions & 188 deletions

File tree

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ miette = { version = "7", features = ["fancy"] }
5757
thiserror = "2"
5858

5959
# Windows platform APIs (ETW/TDH audit consumer in openshell-driver-mxc; Windows-only)
60-
windows = { version = "0.62", features = ["Wdk_System_Threading", "Win32_Foundation", "Win32_System_Diagnostics_Etw", "Win32_System_Time"] }
60+
windows = { version = "0.62", features = ["Wdk_System_Threading", "Win32_Foundation", "Win32_NetworkManagement_IpHelper", "Win32_Networking_WinSock", "Win32_System_Diagnostics_Etw", "Win32_System_Time"] }
6161
anyhow = "1"
6262

6363
# Logging/Tracing

crates/openshell-driver-mxc/src/control_channel.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ pub type ReadySlot = Mutex<Option<oneshot::Sender<Result<(), String>>>>;
5050
/// "forward", or the `"target_ready"` event itself) -- an independently
5151
/// staged, stale relay binary then fails fast with a clear error instead of
5252
/// hanging or misbehaving against fields/events it doesn't understand.
53-
const REQUIRED_SUPERVISOR_RELAY_PROTOCOL_VERSION: u64 = 3;
53+
const REQUIRED_SUPERVISOR_RELAY_PROTOCOL_VERSION: u64 = 4;
5454

5555
/// One control channel per sandboxed process. `request()` is safe to call
5656
/// concurrently — each call gets its own correlation id and awaits only its
@@ -284,7 +284,7 @@ mod tests {
284284
let (slot, rx) = armed_ready_slot();
285285

286286
let consumed =
287-
ControlChannel::try_route_ready(&slot, r#"{"event":"ready","protocol_version":3}"#)
287+
ControlChannel::try_route_ready(&slot, r#"{"event":"ready","protocol_version":4}"#)
288288
.await;
289289

290290
assert!(consumed);
@@ -309,7 +309,7 @@ mod tests {
309309
"error should name the offending version: {err}"
310310
);
311311
assert!(
312-
err.contains('3'),
312+
err.contains('4'),
313313
"error should name the required version: {err}"
314314
);
315315
}
@@ -363,7 +363,7 @@ mod tests {
363363
// other's slot.
364364
let consumed = ControlChannel::try_route_target_status(
365365
&slot,
366-
r#"{"event":"ready","protocol_version":3}"#,
366+
r#"{"event":"ready","protocol_version":4}"#,
367367
)
368368
.await;
369369

crates/openshell-driver-mxc/src/driver.rs

Lines changed: 219 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ use openshell_core::proto_struct::struct_to_json_value;
2121
use openshell_core::provider_credentials::ProviderCredentialState;
2222
use serde::{Deserialize, Serialize};
2323
use std::collections::HashMap;
24+
use std::mem::size_of;
2425
use std::net::SocketAddr;
2526
use std::path::{Path, PathBuf};
2627
use std::pin::Pin;
@@ -30,12 +31,119 @@ use tokio::process::Child;
3031
use tokio::sync::{Mutex, broadcast, mpsc, oneshot, watch};
3132
use tokio_stream::wrappers::ReceiverStream;
3233
use tracing::{info, warn};
34+
use windows::Win32::Foundation::ERROR_INSUFFICIENT_BUFFER;
35+
use windows::Win32::NetworkManagement::IpHelper::{
36+
GetExtendedTcpTable, MIB_TCP_STATE_LISTEN, MIB_TCPROW_LH, MIB_TCPTABLE,
37+
TCP_TABLE_BASIC_LISTENER,
38+
};
39+
use windows::Win32::Networking::WinSock::AF_INET;
3340

3441
const DRIVER_NAME: &str = "mxc";
3542
const DRIVER_VERSION: &str = env!("CARGO_PKG_VERSION");
3643
/// Sentinel image name — MXC has no OCI image; this string must be non-empty
3744
/// so the gateway's `default_image` cache is satisfied, but it is not pullable.
3845
const DEFAULT_IMAGE_SENTINEL: &str = "mxc:process-container";
46+
const TARGET_READY_TIMEOUT: std::time::Duration = std::time::Duration::from_mins(5);
47+
const TARGET_READY_POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(300);
48+
49+
#[allow(unsafe_code)]
50+
fn tcp_listener_is_present(port: u16) -> std::io::Result<bool> {
51+
let mut byte_count = 0_u32;
52+
// SAFETY: The null-buffer call only asks Windows for the required size;
53+
// `byte_count` points to initialized writable storage.
54+
let status = unsafe {
55+
GetExtendedTcpTable(
56+
None,
57+
&raw mut byte_count,
58+
false,
59+
u32::from(AF_INET.0),
60+
TCP_TABLE_BASIC_LISTENER,
61+
0,
62+
)
63+
};
64+
if status != ERROR_INSUFFICIENT_BUFFER.0 && status != 0 {
65+
return Err(std::io::Error::from_raw_os_error(status.cast_signed()));
66+
}
67+
if (byte_count as usize) < size_of::<u32>() {
68+
return Err(std::io::Error::new(
69+
std::io::ErrorKind::InvalidData,
70+
"Windows returned an invalid TCP listener table size",
71+
));
72+
}
73+
74+
let mut buffer;
75+
loop {
76+
let word_count = (byte_count as usize).div_ceil(size_of::<u32>());
77+
buffer = vec![0_u32; word_count];
78+
// SAFETY: `buffer` has the returned table's alignment and at least
79+
// the requested byte count. Windows updates `byte_count` if the table
80+
// grows concurrently.
81+
let status = unsafe {
82+
GetExtendedTcpTable(
83+
Some(buffer.as_mut_ptr().cast()),
84+
&raw mut byte_count,
85+
false,
86+
u32::from(AF_INET.0),
87+
TCP_TABLE_BASIC_LISTENER,
88+
0,
89+
)
90+
};
91+
if status == ERROR_INSUFFICIENT_BUFFER.0 {
92+
continue;
93+
}
94+
if status != 0 {
95+
return Err(std::io::Error::from_raw_os_error(status.cast_signed()));
96+
}
97+
break;
98+
}
99+
100+
let table = buffer.as_ptr().cast::<MIB_TCPTABLE>();
101+
// SAFETY: A successful call writes a `MIB_TCPTABLE` header followed by
102+
// `dwNumEntries` rows into the caller-provided buffer.
103+
let entry_count = unsafe { (*table).dwNumEntries as usize };
104+
let row_offset = std::mem::offset_of!(MIB_TCPTABLE, table);
105+
let available_rows = (byte_count as usize)
106+
.saturating_sub(row_offset)
107+
.checked_div(size_of::<MIB_TCPROW_LH>())
108+
.unwrap_or_default();
109+
if entry_count > available_rows {
110+
return Err(std::io::Error::new(
111+
std::io::ErrorKind::InvalidData,
112+
"Windows returned a truncated TCP listener table",
113+
));
114+
}
115+
// SAFETY: The bounds check proves every row lies within `buffer`.
116+
let rows = unsafe { std::slice::from_raw_parts((*table).table.as_ptr(), entry_count) };
117+
Ok(rows.iter().any(|row| {
118+
let port_bytes = row.dwLocalPort.to_ne_bytes();
119+
let listener_port = u16::from_be_bytes([port_bytes[0], port_bytes[1]]);
120+
// SAFETY: `dwState` and `State` are views of the same SDK union field,
121+
// and Windows initialized every returned row.
122+
let state = unsafe { row.Anonymous.dwState };
123+
state == MIB_TCP_STATE_LISTEN.0.cast_unsigned() && listener_port == port
124+
}))
125+
}
126+
127+
async fn wait_for_target_listener(port: u16) -> std::io::Result<()> {
128+
let start = tokio::time::Instant::now();
129+
let deadline = start + TARGET_READY_TIMEOUT;
130+
info!(port, timeout = ?TARGET_READY_TIMEOUT, "waiting for target listener in host TCP table");
131+
loop {
132+
if tcp_listener_is_present(port)? {
133+
info!(port, elapsed = ?start.elapsed(), "target listener observed in host TCP table");
134+
return Ok(());
135+
}
136+
let now = tokio::time::Instant::now();
137+
if now >= deadline {
138+
break;
139+
}
140+
tokio::time::sleep_until(std::cmp::min(now + TARGET_READY_POLL_INTERVAL, deadline)).await;
141+
}
142+
Err(std::io::Error::new(
143+
std::io::ErrorKind::TimedOut,
144+
format!("timed out after {TARGET_READY_TIMEOUT:?} waiting for port {port}"),
145+
))
146+
}
39147

40148
// ── Config ────────────────────────────────────────────────────────────────────
41149

@@ -1802,9 +1910,10 @@ async fn run_lifecycle(
18021910
(None, None)
18031911
};
18041912
// Target-status signal from the spawner (see control_channel.rs's
1805-
// try_route_target_status): Ok once the target port accepts connections,
1806-
// or Err with the target's real exit/stderr diagnostic. Distinct from the
1807-
// "launch" response below, which only confirms the command/env arrived.
1913+
// try_route_target_status): Ok once the host observes the target listener
1914+
// and the relay confirms the target has not exited, or Err with the
1915+
// target's real exit/stderr diagnostic. Distinct from the "launch"
1916+
// response below, which only confirms the command/env arrived.
18081917
let (target_ready_slot, target_ready_rx) = if spawner_wrapping_active {
18091918
let (tx, rx) = oneshot::channel::<Result<(), String>>();
18101919
(Some(Arc::new(Mutex::new(Some(tx)))), Some(rx))
@@ -1886,14 +1995,14 @@ async fn run_lifecycle(
18861995
// Publish a cancellable handle (exec_child, and for ProcessContainer
18871996
// shutdown_tx/terminated_rx too) and release the startup gate now,
18881997
// rather than holding it until the target-readiness wait below (up to
1889-
// ~430s worst case: 120s ready + 310s target_ready) completes or times
1998+
// ~430s worst case: 120s relay-ready + 300s listener + handshakes) completes or times
18901999
// out. stop_sandbox/delete_sandbox block on lifecycle_gate before doing
18912000
// anything else, so holding it this long meant a stop/delete arriving
18922001
// while a target is slow to (or never does) come up had no way to
18932002
// interrupt that wait -- it just queued up behind it. See also imp.rs's
1894-
// matching fix: openshell-supervisor-relay now races its own
1895-
// port-readiness wait against a "shutdown" request instead of only
1896-
// observing shutdown once that wait finishes.
2003+
// matching fix: openshell-supervisor-relay now races the host-readiness
2004+
// confirmation against a "shutdown" request instead of only observing
2005+
// shutdown once startup finishes.
18972006
let shutdown_rx = {
18982007
let mut reg = registry.lock().await;
18992008
let Some(entry) = reg.get_mut(&sandbox_id) else {
@@ -2019,51 +2128,100 @@ async fn run_lifecycle(
20192128
let launch_err = if let Some(e) = ready_err {
20202129
Some(e)
20212130
} else {
2022-
let launch_data = serde_json::json!({
2023-
"command": sandbox_config.command,
2024-
"env": env,
2025-
});
2026-
match channel
2027-
.request("launch", launch_data, std::time::Duration::from_mins(2))
2028-
.await
2029-
{
2030-
Ok(resp) if resp.get("ok").and_then(serde_json::Value::as_bool) == Some(true) => {
2031-
info!(sandbox = %sandbox_name, "control-channel launch acknowledged");
2032-
// The "launch" response above only confirms the
2033-
// command/env reached the spawner -- it still needs
2034-
// to spawn the target and confirm its configured
2035-
// port is accepting connections
2036-
// (openshell-supervisor-relay's own
2037-
// wait_for_port_ready, up to ~300s worst case across
2038-
// its own retries). Await that distinct
2039-
// "target_ready" event before treating launch as
2040-
// successful, so a caller acting on Ready=True below
2041-
// can never race a target that hasn't bound its port
2042-
// yet.
2043-
let target_ready_timeout = std::time::Duration::from_secs(310);
2044-
match tokio::time::timeout(target_ready_timeout, target_ready_rx).await {
2045-
Ok(Ok(Ok(()))) => {
2046-
info!(sandbox = %sandbox_name, "control-channel target ready");
2047-
None
2048-
}
2049-
// `target_failed` carries the bounded target stderr
2050-
// diagnostic supplied by openshell-supervisor-relay.
2051-
Ok(Ok(Err(target_err))) => Some(target_err),
2052-
Ok(Err(_)) => {
2053-
Some("spawner exited before its target became ready".to_string())
2131+
let target_port = config.pc_relay_target_port;
2132+
match tcp_listener_is_present(target_port) {
2133+
Ok(true) => Some(format!(
2134+
"target port {target_port} is already listening before launch"
2135+
)),
2136+
Err(error) => Some(format!(
2137+
"failed to inspect target port {target_port} before launch: {error}"
2138+
)),
2139+
Ok(false) => {
2140+
let launch_data = serde_json::json!({
2141+
"command": sandbox_config.command,
2142+
"env": env,
2143+
});
2144+
match channel
2145+
.request("launch", launch_data, std::time::Duration::from_mins(2))
2146+
.await
2147+
{
2148+
Ok(resp)
2149+
if resp.get("ok").and_then(serde_json::Value::as_bool)
2150+
== Some(true) =>
2151+
{
2152+
info!(sandbox = %sandbox_name, "control-channel launch acknowledged");
2153+
// The AppContainer cannot safely probe its own
2154+
// pre-listener loopback port or inspect the TCP
2155+
// table. Observe the listener from the host while
2156+
// racing the relay's early-exit diagnostic.
2157+
let mut target_ready_rx = target_ready_rx;
2158+
let listener_error = tokio::select! {
2159+
result = wait_for_target_listener(target_port) => {
2160+
result.err().map(|error| error.to_string())
2161+
}
2162+
status = &mut target_ready_rx => {
2163+
Some(match status {
2164+
Ok(Err(target_err)) => target_err,
2165+
Ok(Ok(())) => "spawner reported target ready before host confirmation".to_string(),
2166+
Err(_) => "spawner exited before its target became ready".to_string(),
2167+
})
2168+
}
2169+
};
2170+
if let Some(error) = listener_error {
2171+
Some(error)
2172+
} else {
2173+
let confirm_timeout = std::time::Duration::from_secs(10);
2174+
match channel
2175+
.request(
2176+
"target_ready",
2177+
serde_json::Value::Null,
2178+
confirm_timeout,
2179+
)
2180+
.await
2181+
{
2182+
Ok(resp)
2183+
if resp.get("ok").and_then(serde_json::Value::as_bool)
2184+
== Some(true) =>
2185+
{
2186+
match tokio::time::timeout(
2187+
confirm_timeout,
2188+
&mut target_ready_rx,
2189+
)
2190+
.await
2191+
{
2192+
Ok(Ok(Ok(()))) => {
2193+
info!(sandbox = %sandbox_name, "control-channel target ready");
2194+
None
2195+
}
2196+
Ok(Ok(Err(target_err))) => Some(target_err),
2197+
Ok(Err(_)) => Some(
2198+
"spawner exited before confirming target readiness"
2199+
.to_string(),
2200+
),
2201+
Err(_) => Some(format!(
2202+
"timed out after {confirm_timeout:?} waiting for target readiness confirmation"
2203+
)),
2204+
}
2205+
}
2206+
Ok(resp) => Some(
2207+
resp.get("error")
2208+
.and_then(|value| value.as_str())
2209+
.unwrap_or("target readiness confirmation rejected")
2210+
.to_string(),
2211+
),
2212+
Err(error) => Some(error.to_string()),
2213+
}
2214+
}
20542215
}
2055-
Err(_) => Some(format!(
2056-
"timed out after {target_ready_timeout:?} waiting for target to become ready"
2057-
)),
2216+
Ok(resp) => Some(
2217+
resp.get("error")
2218+
.and_then(|v| v.as_str())
2219+
.unwrap_or("launch rejected")
2220+
.to_string(),
2221+
),
2222+
Err(e) => Some(e.to_string()),
20582223
}
20592224
}
2060-
Ok(resp) => Some(
2061-
resp.get("error")
2062-
.and_then(|v| v.as_str())
2063-
.unwrap_or("launch rejected")
2064-
.to_string(),
2065-
),
2066-
Err(e) => Some(e.to_string()),
20672225
}
20682226
};
20692227
if let Some(err) = launch_err {
@@ -2394,6 +2552,18 @@ mod lifecycle_tests {
23942552
};
23952553
use std::time::Duration;
23962554

2555+
#[test]
2556+
fn target_ready_budget_remains_five_minutes() {
2557+
assert_eq!(TARGET_READY_TIMEOUT, Duration::from_mins(5));
2558+
}
2559+
2560+
#[tokio::test]
2561+
async fn host_tcp_table_observes_loopback_listener() {
2562+
let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap();
2563+
let port = listener.local_addr().unwrap().port();
2564+
assert!(tcp_listener_is_present(port).unwrap());
2565+
}
2566+
23972567
fn driver_sandbox(id: &str) -> DriverSandbox {
23982568
let shell =
23992569
std::env::var("COMSPEC").unwrap_or_else(|_| r"C:\Windows\System32\cmd.exe".to_string());

0 commit comments

Comments
 (0)