Skip to content

[nanvix-http] B: serve_kill waits for VM exit#20

Open
esaurez wants to merge 1 commit into
enhancement-windows-http-modefrom
bugfix-serve-kill-wait-order
Open

[nanvix-http] B: serve_kill waits for VM exit#20
esaurez wants to merge 1 commit into
enhancement-windows-http-modefrom
bugfix-serve-kill-wait-order

Conversation

@esaurez

@esaurez esaurez commented May 15, 2026

Copy link
Copy Markdown
Owner

Summary

Re-order the cleanup in nanvix-http's serve_kill so the VMM task is awaited before the gateway bridge is aborted. A small, surgical correctness fix that was previously silent on Unix and becomes user-visible on Windows once the cross-platform gateway endpoint lands (#18).

Base

This PR is stacked on #18 (enhancement-windows-http-mode). The new bridge field name (gateway_sockaddr) and cfg(unix)-gated remove_file are introduced there.

What changed

// before
running._gateway_bridge.abort();
#[cfg(unix)]
let _ = std::fs::remove_file(&running.gateway_sockaddr);
match running.handle.wait().await { ... }

// after
let wait_result = running.handle.wait().await;
running._gateway_bridge.abort();
#[cfg(unix)]
let _ = std::fs::remove_file(&running.gateway_sockaddr);
match wait_result { ... }

The match arm now operates on the saved wait_result.

A multi-paragraph comment captures the rationale and documents the invariant the new ordering relies on: the bridge's consumer (the gateway UDS peer on Unix or the named-pipe peer on Windows) must keep draining the bytes the bridge forwards. If a future consumer stops reading mid-stream, the connection write back-pressures the bridge, the bridge stops draining output_rx, the io_handler eventually blocks on output_tx.send().await (once the bounded channel buffer fills), and the guest stalls without reaching VM exit — which would hang the KILL response.

The same RunningVm.handle/bridge are also torn down in StandaloneState::cleanup(), but cleanup() uses abort_and_wait() (forced shutdown) where the drain invariant does not apply. A short comment in cleanup() records this asymmetry so a future contributor does not "harmonise" the two paths.

Why this matters

The previous order aborted the bridge as soon as the shim's KILL arrived, which closed output_rx while the guest was still writing to stdio. Every subsequent guest write then failed at the io_handler with output channel closed. The consequences:

  • Unix: gateway-socket consumers (e.g., the in-tree test harness) see an abrupt EOF instead of a graceful close.
  • Windows (after [nanvixd] E: Enable HTTP mode on Windows #18): the same code path is what the containerd shim's gateway-pipe consumer reads. Without this fix, the consumer loses every byte the guest writes after the shim's KILL arrives — ~60 s of output for typical CPython workloads — and the guest exits 120 from the resulting BrokenPipeError.

After the fix, the bridge ends naturally when the io_handler closes output_tx at VM teardown: output_rx.recv() returns None, the bridge's read loop terminates, and the spawned task resolves on its own. The abort() after wait() is retained as defensive cleanup in case the bridge has not yet released its connection handles by the time we return the KILL response.

Why it's safe

  • The order swap moves a single await earlier; the function's semantics (return KILL exit code after the VM exits) are unchanged.
  • _gateway_bridge.abort() no longer races with output_tx: the io_handler has already closed it from the VMM side by the time wait() returns.
  • The new ordering does not introduce a deadlock for well-behaved consumers (those satisfying the drain invariant documented in the code comment).

Tests

No straightforward unit test for this path (requires a live VM + bridge). Existing standalone integration coverage exercises the new ordering implicitly. z.ps1 build, z.ps1 build -- format-check, z.ps1 build -- lint-check, and z.ps1 build -- spellcheck all pass on Windows.

Notes for reviewers

@esaurez esaurez force-pushed the bugfix-serve-kill-wait-order branch from 16e6452 to 02a1348 Compare May 19, 2026 01:20
@esaurez esaurez changed the base branch from dev to enhancement-windows-http-mode May 19, 2026 01:21
@esaurez esaurez force-pushed the enhancement-windows-http-mode branch from f9252b4 to 03203a8 Compare May 19, 2026 01:35
@esaurez esaurez force-pushed the bugfix-serve-kill-wait-order branch from 02a1348 to 721dc4a Compare May 19, 2026 01:38
@esaurez esaurez force-pushed the enhancement-windows-http-mode branch from 03203a8 to 3052d99 Compare May 19, 2026 02:12
Re-order the cleanup in `serve_kill` so the VMM task is awaited
BEFORE the gateway bridge task is aborted.

The previous order aborted the bridge as soon as the shim's KILL
arrived, which closed `output_rx` while the guest was still
writing to stdio. Every subsequent guest write then failed at the
io_handler with `output channel closed`. On Unix this surfaces to
consumers of the gateway socket as an abrupt EOF instead of a
graceful close; on Windows (after the cross-platform gateway
consumer landed) it manifests as CPython BrokenPipeError -> exit
120, ~60 seconds after KILL was issued.

After the fix, the bridge ends naturally when the io_handler
closes `output_tx` at VM teardown: `output_rx.recv()` returns
`None`, the bridge's read loop terminates, and the spawned task
resolves on its own. The `abort()` after `wait()` is retained as
defensive cleanup in case the bridge has not yet released its
connection handles by the time we return the KILL response.

The comment block documents the invariant the new ordering relies
on: the bridge's consumer (UDS peer on Unix, named-pipe peer on
Windows) must keep draining bytes the bridge forwards. If a
future consumer stops reading mid-stream, the connection write
back-pressures the bridge, the bridge stops draining `output_rx`,
and the io_handler eventually blocks on `output_tx.send().await`
-- which would hang the KILL response.

The fix is a 3-line re-order. It is logically correct on both
Unix and Windows; on Unix it merely cleans up an EOF-instead-of-
graceful-close artifact in the gateway socket consumers, but on
Windows it is what keeps CPython workloads alive past their first
stdio write.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant