Skip to content

Commit faa7969

Browse files
shiju-nvdrew
andauthored
fix(sandbox-backend): recover TCP mediation after boundary disconnects (#3403)
Use authenticated boundary recovery for pending TCP accepts. Close the response direction on gRPC failure and retain the generation observed before a request so delayed errors cannot retire its replacement. Add TLS transport-loss, concurrent recovery, terminal-error, and idle-wait regressions. Interrupted opens remain denied and established connections are not replayed. Related to #3396. Signed-off-by: Shiju <shiju@nvidia.com> Signed-off-by: Drew Newberry <anewberry@nvidia.com> Co-authored-by: Drew Newberry <anewberry@nvidia.com>
1 parent 1fe79f5 commit faa7969

3 files changed

Lines changed: 511 additions & 14 deletions

File tree

‎architecture/sandbox.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ credentials to claim the existing runtime generation. Confirmation resumes the
3939
workload; expiration terminates it. A credential replacement does not displace
4040
the active connection until the new connection is confirmed. Idle healthy
4141
connections remain usable.
42+
TCP mediation accepts use the same authenticated transport recovery as process waits. A healthy idle accept has no timeout. An interrupted pending open fails closed, while a replacement accept waits for new workload traffic; decisions and established byte streams are not replayed. Boundary rejections and failed recovery remain terminal to the proxy.
4243

4344
A renewed Sandbox Protocol bearer is authenticated even when its credential epoch is unchanged. The supervisor confirms that bearer on the active physical connection and records its fingerprint only after confirmation succeeds, preserving pending streams and the mediation session. Changing the credential epoch still requires an authenticated replacement connection.
4445

‎crates/openshell-sandbox-backend/src/runtime.rs‎

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -792,7 +792,10 @@ struct RemoteNetworkMediation {
792792
#[async_trait]
793793
impl NetworkMediationSource for RemoteNetworkMediation {
794794
async fn accept_tcp(&self) -> Result<PendingTcpOpen, BackendError> {
795-
let (stream, response) = self.client.open_exchange(Request::AcceptNetwork).await?;
795+
// An idle accept has no deadline. Recover transport loss before exposing
796+
// a source error, which permanently stops the proxy. The boundary drops
797+
// the interrupted pending open; a retry never replays its decision.
798+
let (stream, response) = self.client.call_wait_stream(Request::AcceptNetwork).await?;
796799
let Response::NetworkConnected {
797800
identity,
798801
destination,
@@ -1079,6 +1082,7 @@ impl BoundaryClient {
10791082
let envelope = Self::prepare_request(request)?;
10801083
tokio::time::timeout(timeout, async {
10811084
loop {
1085+
let generation = self.connection_generation().await;
10821086
match self.exchange_envelope(&envelope).await {
10831087
Ok(response) => {
10841088
if remember_attach {
@@ -1100,7 +1104,7 @@ impl BoundaryClient {
11001104
Err(BackendError::Unavailable(message))
11011105
if is_transport_unavailable(&message) =>
11021106
{
1103-
self.recover_after_unavailable().await?;
1107+
self.recover_after_unavailable(generation).await?;
11041108
tokio::time::sleep(Duration::from_millis(25)).await;
11051109
}
11061110
Err(error) => return Err(error),
@@ -1116,18 +1120,29 @@ impl BoundaryClient {
11161120
}
11171121

11181122
async fn call_wait(&self, request: Request) -> Result<Response, BackendError> {
1123+
let (_, response) = self.call_wait_stream(request).await?;
1124+
Ok(response)
1125+
}
1126+
1127+
/// Wait without an idle timeout, retaining the stream for TCP mediation.
1128+
/// Only transport failures enter recovery; boundary rejections stay terminal.
1129+
async fn call_wait_stream(
1130+
&self,
1131+
request: Request,
1132+
) -> Result<(BoundaryDuplexStream, Response), BackendError> {
11191133
let envelope = Self::prepare_request(request)?;
11201134
let mut recovery_deadline = None;
11211135
loop {
1122-
match self.exchange_envelope(&envelope).await {
1136+
let generation = self.connection_generation().await;
1137+
match self.open_exchange_envelope(&envelope).await {
11231138
Ok(response) => return Ok(response),
11241139
Err(BackendError::Unavailable(message)) if is_transport_unavailable(&message) => {
11251140
let deadline =
11261141
begin_recovery_window(&mut recovery_deadline, tokio::time::Instant::now());
11271142
if tokio::time::Instant::now() >= deadline {
11281143
return Err(BackendError::Unavailable(message));
11291144
}
1130-
self.recover_after_unavailable().await?;
1145+
self.recover_after_unavailable(generation).await?;
11311146
tokio::time::sleep(Duration::from_millis(25)).await;
11321147
}
11331148
Err(error) => return Err(error),
@@ -1142,12 +1157,13 @@ impl BoundaryClient {
11421157
let envelope = Self::prepare_request(request)?;
11431158
tokio::time::timeout(REQUEST_TIMEOUT, async {
11441159
loop {
1160+
let generation = self.connection_generation().await;
11451161
match self.open_exchange_envelope(&envelope).await {
11461162
Ok(response) => return Ok(response),
11471163
Err(BackendError::Unavailable(message))
11481164
if is_transport_unavailable(&message) =>
11491165
{
1150-
self.recover_after_unavailable().await?;
1166+
self.recover_after_unavailable(generation).await?;
11511167
tokio::time::sleep(Duration::from_millis(25)).await;
11521168
}
11531169
Err(error) => return Err(error),
@@ -1165,12 +1181,13 @@ impl BoundaryClient {
11651181
let envelope = Self::prepare_request(request)?;
11661182
tokio::time::timeout(REQUEST_TIMEOUT, async {
11671183
loop {
1184+
let generation = self.connection_generation().await;
11681185
match self.open_exchange_envelope(&envelope).await {
11691186
Ok(response) => return Ok(response),
11701187
Err(BackendError::Unavailable(message))
11711188
if is_transport_unavailable(&message) =>
11721189
{
1173-
self.recover_after_unavailable().await?;
1190+
self.recover_after_unavailable(generation).await?;
11741191
tokio::time::sleep(Duration::from_millis(25)).await;
11751192
}
11761193
Err(error) => return Err(error),
@@ -1194,6 +1211,7 @@ impl BoundaryClient {
11941211
.map_err(|error| BackendError::Process(format!("encode control request: {error}")))
11951212
}
11961213

1214+
#[cfg(test)]
11971215
async fn open_exchange(
11981216
&self,
11991217
request: Request,
@@ -1353,15 +1371,21 @@ impl BoundaryClient {
13531371
Ok(())
13541372
}
13551373

1356-
/// Replace a failed physical transport and replay the authenticated
1357-
/// lifecycle needed to make the new HTTP/2 connection authoritative.
1358-
async fn recover_after_unavailable(&self) -> Result<(), BackendError> {
1359-
let observed_generation = self
1360-
.grpc_channel
1374+
async fn connection_generation(&self) -> Option<u64> {
1375+
self.grpc_channel
13611376
.lock()
13621377
.await
13631378
.as_ref()
1364-
.map(|cached| cached.generation);
1379+
.map(|cached| cached.generation)
1380+
}
1381+
1382+
/// Replace a failed physical transport and replay its authenticated lifecycle.
1383+
/// Capture the generation before the attempt: a late failure from an old
1384+
/// connection must not retire one another caller has already recovered.
1385+
async fn recover_after_unavailable(
1386+
&self,
1387+
observed_generation: Option<u64>,
1388+
) -> Result<(), BackendError> {
13651389
let _reconnect = self.reconnect.lock().await;
13661390
if self
13671391
.grpc_channel
@@ -1481,12 +1505,13 @@ impl BoundaryClient {
14811505
// waiting. Never multiply that deadline with message-matching retries.
14821506
let session = tokio::time::timeout(REQUEST_TIMEOUT, async {
14831507
loop {
1508+
let generation = self.connection_generation().await;
14841509
match self.open_mediation_session().await {
14851510
Ok(session) => return Ok(session),
14861511
Err(BackendError::Unavailable(message))
14871512
if is_transport_unavailable(&message) =>
14881513
{
1489-
self.recover_after_unavailable().await?;
1514+
self.recover_after_unavailable(generation).await?;
14901515
tokio::time::sleep(Duration::from_millis(25)).await;
14911516
}
14921517
Err(error) => return Err(error),
@@ -1750,6 +1775,10 @@ where
17501775
}
17511776
Err(error) => {
17521777
tracing::debug!(%error, "boundary gRPC response stream ended");
1778+
// The request pump still owns the other duplex half. Explicitly
1779+
// close this direction so a waiting exchange observes EOF and
1780+
// can recover instead of waiting for both halves to drop.
1781+
let _ = writer.shutdown().await;
17531782
return;
17541783
}
17551784
}
@@ -1864,6 +1893,7 @@ fn is_transport_unavailable(message: &str) -> bool {
18641893
#[cfg(test)]
18651894
mod tests {
18661895
mod credential_renewal;
1896+
mod network_recovery;
18671897

18681898
use std::path::PathBuf;
18691899
use std::pin::Pin;
@@ -2175,13 +2205,31 @@ mod tests {
21752205
Response::Confirmed { .. }
21762206
));
21772207

2178-
client.recover_after_unavailable().await.unwrap();
2208+
let failed_generation = client.connection_generation().await;
2209+
client
2210+
.recover_after_unavailable(failed_generation)
2211+
.await
2212+
.unwrap();
21792213
tokio::time::timeout(Duration::from_secs(1), server)
21802214
.await
21812215
.expect("replacement connection accepted")
21822216
.unwrap();
21832217
assert_eq!(accepted.load(Ordering::Acquire), 2);
21842218
assert_eq!(requests.load(Ordering::Acquire), 4);
2219+
2220+
// Another accept can report the same transport loss after recovery.
2221+
// It must reuse the replacement without a third connection or replay.
2222+
let recovered_generation = client.connection_generation().await;
2223+
tokio::time::timeout(
2224+
Duration::from_secs(1),
2225+
client.recover_after_unavailable(failed_generation),
2226+
)
2227+
.await
2228+
.expect("stale failure must reuse the recovered connection")
2229+
.unwrap();
2230+
assert_eq!(client.connection_generation().await, recovered_generation);
2231+
assert_eq!(accepted.load(Ordering::Acquire), 2);
2232+
assert_eq!(requests.load(Ordering::Acquire), 4);
21852233
}
21862234

21872235
#[test]

0 commit comments

Comments
 (0)