diff --git a/PATCHES.md b/PATCHES.md index ba53678..39f1a5c 100644 --- a/PATCHES.md +++ b/PATCHES.md @@ -78,6 +78,28 @@ in-tree means the patch can't be lost to a dependency re-resolve. the exec exits / on force-close) — without that caller, upstream never deletes execs at all and the shared control container leaks a connection + `runConnections()` task per turn. +10. **`Sources/ContainerizationOS/Socket/Socket.swift` — `acceptStream` survives transient accept + errors.** Upstream cancelled the accept `DispatchSource` on ANY `accept(2)` failure, permanently + ending accepting while the socket stayed **bound and listening** — a silent black hole: every + later client `connect(2)` SUCCEEDED into the kernel backlog and hung forever unanswered. When + the socket is the relayed control plane of a shared container, that is the "agent produced no + output within 60s / stdio transport stalled" all-sessions wedge, fixable only by recreating the + VM (an app restart). Transient errors — `ECONNABORTED`/`ECONNRESET` (a queued connection dying + before accept, routine under connection churn), `EMFILE`/`ENFILE`/`ENOBUFS`/`ENOMEM` (resource + pressure), `EINTR`/`EAGAIN` — now skip that one accept and keep listening (new + `isTransientAcceptError`). This is SHARED code: the fix reaches the host by a normal build and + the guest via the initfs rebuild. Marked `[Nucleic vendored patch]`. + +11. **`Sources/Containerization/UnixSocketRelay.swift` — relay loops contain per-connection + failures.** Both accept loops (`setupHostVsockListener` — the host half of a container's relayed + control socket — and `setupHostVsockDial`) used to let ONE thrown per-connection dial/connect + (host server rebinding, backlog momentarily full → `ECONNREFUSED`, fd pressure) propagate out of + the loop, whose teardown then removed the vsock listener — permanently severing every session in + the container from the host control plane. Each connection is now handled on its own task with + its error logged and contained (mirroring the guest `VsockProxy`), and the pre-relay failure + paths close both ends so a failed connection fails FAST for the peer and leaks no fds. Marked + `[Nucleic vendored patch]`. + ### GUEST-side patches (require rebuilding the initfs — see below) Patches #1–#7 are host-side (the `Containerization` library), shipped by a normal `swift build`. @@ -132,6 +154,27 @@ rebuild whenever a guest patch changes. Built locally, not in CI: the host frame `linux.resources` at `linux.cgroupsPath`, which the patch repoints (init leaf) and clears accordingly. +12. **`vminitd/Sources/VminitdCore/VsockProxy.swift` — leak-proof, crash-proof relay connections; + no black-hole listener.** Four fixes to the guest half of the relayed control socket (the path + every session's MCP/approval traffic crosses in a shared container): + - **fd leak (the root of the recurring all-sessions stall):** `cleanup` ran its two epoll + unregisters and two `close(2)`s in one `do/catch`, so a thrown unregister SKIPPED the closes — + leaking both connection fds. Control-plane traffic is connection-churny by design (an SSE + `tools/call` closes its connection every gated call; every intercepted git/gh/command event is + a short-lived connection), so the leaks accumulated until vminitd hit `EMFILE`, its accept + path began failing, and — before patch #10 — the accept stream died with the guest socket + still bound: every session in the container then stalled ("produced no output within 60s") + until the VM was recreated. Each cleanup step now runs independently. + - **double-resume crash:** both fds' epoll handlers can reach the cleanup condition; a second + entry would resume the `CheckedContinuation` twice — a fatal trap in the VM's PID-1 agent. + `cleanup` is now once-guarded. + - **`try!` registrations:** an `epoll_ctl` failure crashed vminitd outright; registration + failures now fail only that connection, releasing whatever was already set up. + - **no black-hole listener:** if the accept loop ever ends unexpectedly, the proxy now closes + its listener (new `listenerLoopEnded`), so peers get fail-fast refusals instead of connecting + into a never-accepted backlog. A failed pre-relay connection is also closed explicitly. + Marked `[Nucleic vendored patch]`. + ## Re-vendoring a newer upstream commit 1. `git clone` upstream (or copy `.build/checkouts/containerization` after bumping the URL pin @@ -146,7 +189,10 @@ rebuild whenever a guest patch changes. Built locally, not in CI: the host frame stdio-or-abort guard in `start()`), patch #7 (the bounded `deleteProcess` timeout in `Vminitd.swift`), and patch #8 (the `ManagedProcess.start` event-loop offload in `vminitd/`). Grep for `[Nucleic vendored patch]` to find every site, and patch #9 (per-exec cgroups) across - `Cgroup2Manager.swift` / `ManagedContainer.swift` / `ManagedProcess.swift`. After re-applying any + `Cgroup2Manager.swift` / `ManagedContainer.swift` / `ManagedProcess.swift`, patch #10 + (`Socket.acceptStream` transient-error tolerance + `isTransientAcceptError`), patch #11 (the + `UnixSocketRelay` per-connection containment + fail-fast closes), and patch #12 (the `VsockProxy` + cleanup/`try!`/listener hardening in `vminitd/`). After re-applying any `vminitd/` patch, rebuild + publish the custom init image with `make vminit-image` + `make vminit-image-push`, and bump `ContainerEngine.vminitReference`. 5. Update the commit hash above and in the root `Package.swift` comment. diff --git a/Sources/Containerization/UnixSocketRelay.swift b/Sources/Containerization/UnixSocketRelay.swift index 4069bf4..9008bb5 100644 --- a/Sources/Containerization/UnixSocketRelay.swift +++ b/Sources/Containerization/UnixSocketRelay.swift @@ -109,12 +109,24 @@ extension UnixSocketRelay { $0.t = Task { do { for try await connection in connectionStream { - try await self.handleHostUnixConn( - hostConn: connection, - port: self.port, - vm: self.vm, - log: self.log - ) + // [Nucleic vendored patch] Contain per-connection failures. A thrown dial + // (guest listener briefly absent, transient vsock error) used to propagate + // out of the loop and end the relay for the CONTAINER'S LIFETIME — every + // later client of this socket then failed, unrecoverably. One bad + // connection must only fail that connection. Handled on its own task so a + // slow dial can't head-of-line-block later connections either. + Task { + do { + try await self.handleHostUnixConn( + hostConn: connection, + port: self.port, + vm: self.vm, + log: self.log + ) + } catch { + self.log?.error("failed to relay host unix connection: \(error)") + } + } } } catch { log?.error("failed in unix socket relay loop: \(error)") @@ -138,18 +150,30 @@ extension UnixSocketRelay { state.withLock { $0.listener = listener $0.t = Task { - do { - defer { try? listener.finish() } - for await connection in listener { - try await self.handleGuestVsockConn( - vsockConn: connection, - hostConnectionPath: hostPath, - port: self.port, - log: self.log - ) + defer { try? listener.finish() } + for await connection in listener { + // [Nucleic vendored patch] Contain per-connection failures. This loop is the + // host half of a container's relayed control socket: a single thrown connect + // (host server rebinding, listen backlog momentarily full → ECONNREFUSED, fd + // pressure) used to propagate out of the loop, whose defer then tore down the + // vsock listener — permanently severing EVERY session in the container from the + // host control plane until the container was recreated (an app restart). One + // bad connection must only fail that connection; the client retries. Handled on + // its own task so a slow host connect can't head-of-line-block later guest + // connections. + Task { + do { + try await self.handleGuestVsockConn( + vsockConn: connection, + hostConnectionPath: hostPath, + port: self.port, + log: self.log + ) + } catch { + self.log?.error( + "failed to relay between vsock \(self.port) and \(hostPath.path): \(error)") + } } - } catch { - self.log?.error("failed to setup relay between vsock \(self.port) and \(hostPath.path): \(error)") } } } @@ -176,6 +200,10 @@ extension UnixSocketRelay { ) } catch { log?.error("failed to relay between vsock \(port) and \(hostConn)") + // [Nucleic vendored patch] Close the accepted client connection on failure so the peer + // sees a prompt EOF (fail fast, retryable) instead of a half-open socket, and its fd + // isn't leaked (acceptStream vends closeOnDeinit: false). + try? hostConn.close() throw error } } @@ -186,12 +214,22 @@ extension UnixSocketRelay { port: UInt32, log: Logger? ) async throws { + // [Nucleic vendored patch] Any failure before the relay owns the fds must close BOTH ends: + // the guest connection so the in-guest client sees a prompt EOF (fail fast, retryable — not + // a half-open socket it waits on forever), and the freshly-made host socket so its fd isn't + // leaked (closeOnDeinit is false). let hostPath = hostConnectionPath.path - let socketType = try UnixType(path: hostPath) - let hostSocket = try Socket( - type: socketType, - closeOnDeinit: false - ) + let hostSocket: Socket + do { + let socketType = try UnixType(path: hostPath) + hostSocket = try Socket( + type: socketType, + closeOnDeinit: false + ) + } catch { + try? vsockConn.close() + throw error + } log?.debug( "initiating connection from guest to host", metadata: [ @@ -199,7 +237,13 @@ extension UnixSocketRelay { "hostFd": "\(hostSocket.fileDescriptor)", "guestFd": "\(vsockConn.fileDescriptor)", ]) - try hostSocket.connect() + do { + try hostSocket.connect() + } catch { + try? hostSocket.close() + try? vsockConn.close() + throw error + } do { try await self.relay( @@ -208,6 +252,8 @@ extension UnixSocketRelay { ) } catch { log?.error("failed to relay between vsock \(port) and \(hostPath)") + try? hostSocket.close() + try? vsockConn.close() } } diff --git a/Sources/ContainerizationOS/Socket/Socket.swift b/Sources/ContainerizationOS/Socket/Socket.swift index 3c97d3b..580b557 100644 --- a/Sources/ContainerizationOS/Socket/Socket.swift +++ b/Sources/ContainerizationOS/Socket/Socket.swift @@ -130,6 +130,21 @@ extension Socket { SocketError.withErrno("\(msg) (\(_errnoString(errno)))", errno: errno) } + /// [Nucleic vendored patch] Whether an `accept(2)` failure is transient — the listener is still + /// healthy and later accepts can succeed — as opposed to a dead listener. ECONNABORTED/ECONNRESET + /// are a single queued connection dying before accept; EMFILE/ENFILE/ENOBUFS/ENOMEM are resource + /// pressure that clears; EINTR/EAGAIN are spurious wakeups. See `acceptStream`. + static func isTransientAcceptError(_ error: Swift.Error) -> Bool { + guard case SocketError.withErrno(_, let code) = error else { return false } + switch code { + case ECONNABORTED, ECONNRESET, EINTR, EAGAIN, EWOULDBLOCK, EMFILE, ENFILE, ENOBUFS, ENOMEM, + EPROTO: + return true + default: + return false + } + } + public func connect() throws { try state.withLock { currentState in guard currentState.socketState == .created else { @@ -277,6 +292,17 @@ extension Socket { } catch SocketError.closed { source.cancel() } catch { + // [Nucleic vendored patch] One-strike accepting is a control-plane brick: a + // single transient accept(2) failure — ECONNABORTED (peer aborted while queued, + // routine under connection churn), EMFILE/ENFILE (fd pressure), ENOBUFS/ENOMEM, + // EINTR — used to cancel the source FOREVER while the socket stayed bound and + // listening. Every later client then connect(2)ed into the kernel backlog and + // hung unanswered (a silent black hole — the "agent produced no output within + // 60s" stall when this socket is a relayed control plane). Skip the failed + // accept and keep listening; only a genuinely dead listener ends the stream. + if Self.isTransientAcceptError(error) { + return + } cont.yield(with: .failure(error)) source.cancel() } diff --git a/vminitd/Sources/VminitdCore/VsockProxy.swift b/vminitd/Sources/VminitdCore/VsockProxy.swift index 393fad1..e490a04 100644 --- a/vminitd/Sources/VminitdCore/VsockProxy.swift +++ b/vminitd/Sources/VminitdCore/VsockProxy.swift @@ -175,6 +175,11 @@ extension VsockProxy { ) } catch { self.log?.error("failed to handle connection: \(error)") + // [Nucleic vendored patch] A connection that failed before the relay + // owned it must be closed, or its fd leaks for the proxy's lifetime + // (accept vends closeOnDeinit: true, but the Socket is retained by the + // stream's yielded value until then — close deterministically). + try? conn.close() } } // Safe: actor serialization ensures this runs before connTask can execute its defer. @@ -183,10 +188,33 @@ extension VsockProxy { } catch { self.log?.error("failed to accept connection: \(error)") } + // [Nucleic vendored patch] If this loop ever ends while the proxy is still nominally + // running (fatal accept error), the listening socket MUST come down with it. Leaving it + // bound-but-unaccepted turned the relayed control socket into a silent black hole: every + // later client connect(2) SUCCEEDED into the kernel backlog and hung forever unanswered + // — for Nucleic, every session in the container stalling with "produced no output within + // 60s" until the VM was recreated. Closing the listener makes later connects fail fast + // (ECONNREFUSED/ENOENT), which callers surface and retry. + self.listenerLoopEnded() } self.task = task } + /// [Nucleic vendored patch] The accept loop ended. If `close()` already ran (normal teardown) + /// this is a no-op; otherwise the listener died unexpectedly — tear it down so peers get + /// fail-fast refusals instead of connecting into a never-accepted backlog. + private func listenerLoopEnded() { + guard listener != nil else { return } + log?.error( + "proxy accept loop ended unexpectedly; closing listener", + metadata: [ + "vport": "\(port)", + "uds": "\(path)", + "action": "\(action)", + ]) + try? close() + } + private func handleConn( conn: ContainerizationOS.Socket, connType: SocketType @@ -229,7 +257,18 @@ extension VsockProxy { // - both the client and server have half closed via: // - read hangup on epoll // - EOF on splice + // + // [Nucleic vendored patch] Hardened: (1) runs at most once — both fds' epoll + // handlers can reach the cleanup condition, and a second entry after a failed + // unregister would double-resume the continuation (a fatal trap in the guest's + // PID-1 agent); (2) every step is attempted independently — a thrown unregister + // used to SKIP the close(2)s, leaking both connection fds. Under control-plane + // connection churn those leaks accumulated until vminitd hit EMFILE, its control- + // socket accept loop died, and every session in the container stalled. + nonisolated(unsafe) var cleanedUp = false let cleanup = { @Sendable [log, port, path, action] in + guard !cleanedUp else { return } + cleanedUp = true log?.debug( "cleaning up", metadata: [ @@ -245,16 +284,34 @@ extension VsockProxy { do { try ProcessSupervisor.default.unregisterFd(clientFile.fileDescriptor) + } catch { + self.log?.error("Failed to unregister vsock proxy client fd: \(error)") + } + do { try ProcessSupervisor.default.unregisterFd(serverFile.fileDescriptor) + } catch { + self.log?.error("Failed to unregister vsock proxy server fd: \(error)") + } + do { try conn.close() + } catch { + self.log?.error("Failed to close vsock proxy client: \(error)") + } + do { try relayTo.close() } catch { - self.log?.error("Failed to clean up vsock proxy: \(error)") + self.log?.error("Failed to close vsock proxy server: \(error)") } c.resume() } - try! ProcessSupervisor.default.registerFd(clientFile.fileDescriptor, mask: [.input, .output]) { mask in + // [Nucleic vendored patch] These registrations were `try!` — an epoll_ctl failure + // (fd pressure, a stale registration) crashed vminitd, the VM's PID-1 agent, + // taking every session in the container down. Fail the one connection instead, + // releasing whatever was already set up so nothing leaks (the caller closes `conn`; + // `relayTo` and the first registration are released in the catch blocks below). + do { + try ProcessSupervisor.default.registerFd(clientFile.fileDescriptor, mask: [.input, .output]) { mask in if mask.readyToRead && !eofFromClient { let (fromEof, toEof) = Self.transferData( fromFile: &clientFile, @@ -305,8 +362,13 @@ extension VsockProxy { return cleanup() } } + } catch { + try? relayTo.close() + throw error + } - try! ProcessSupervisor.default.registerFd(serverFile.fileDescriptor, mask: [.input, .output]) { mask in + do { + try ProcessSupervisor.default.registerFd(serverFile.fileDescriptor, mask: [.input, .output]) { mask in if mask.readyToRead && !eofFromServer { let (fromEof, toEof) = Self.transferData( fromFile: &serverFile, @@ -357,6 +419,11 @@ extension VsockProxy { return cleanup() } } + } catch { + try? ProcessSupervisor.default.unregisterFd(clientFile.fileDescriptor) + try? relayTo.close() + throw error + } } catch { c.resume(throwing: error) }