diff --git a/PATCHES.md b/PATCHES.md index 6e7e202..0f193d3 100644 --- a/PATCHES.md +++ b/PATCHES.md @@ -41,6 +41,72 @@ in-tree means the patch can't be lost to a dependency re-resolve. were dropped, and the corresponding `.testTarget(...)` entries removed from `Package.swift`. The library/executable targets we build are untouched. +5. **`Sources/Containerization/LinuxProcess.swift` — non-blocking stdio relay.** + Upstream's `setupIO` relays guest stdout/stderr with `FileHandle.availableData`, a **blocking** + read, from inside a `readabilityHandler`. Those handlers run on Foundation's shared readability + queue, so if one exec's guest stdout wedged mid-stream that blocking read parked the shared thread + and head-of-line-blocked **every** other exec's stdout/stderr relay across all containers — one + stuck session froze the others. The patch marks each connected fd `O_NONBLOCK` and drains it via a + new `nucleicDrainNonBlocking` (returns bytes + EOF, never blocks; EAGAIN just waits for the next + readable event). A wedged stream is now contained to its own exec. Marked `[Nucleic vendored patch]` + (the two static helpers `nucleicSetNonBlocking`/`nucleicDrainNonBlocking` and the two rewritten + `readabilityHandler` blocks). Requires host-side POSIX `read`/`fcntl`/`errno`. + +6. **`Sources/Containerization/LinuxProcess.swift` — atomic stdio-or-abort start.** + In `start()`, after `setupIO` returns, if a *configured* stdio stream never connected from the + guest (its `FileHandle` is nil — patch #3's logged failure), the patch tears the just-created exec + back down (`agent.deleteProcess`) and throws instead of calling `startProcess`. Upstream proceeds + and runs a process with a dead stream (stdin never delivered → hangs; stdout never read → the "no + output, just a spinner" 60s stall in Nucleic Control). Now that permanent silent stall surfaces as + a clean, retryable start error. Marked `[Nucleic vendored patch]` (the guard block before + `startProcess`). + +7. **`Sources/Containerization/Vminitd.swift` — bounded teardown RPC.** + `deleteProcess` now sends a 30s `CallOptions.timeout` (upstream sends none, so it can block + forever on a wedged agent channel). Nucleic calls `LinuxProcess.delete()` after every turn to + reclaim the per-exec vsock/gRPC connection `exec()` dials; an unbounded `deleteProcess` would let + that reclaim hang and the connection leak. On the thrown deadline, `performDeletion` still closes + the agent connection. Marked `[Nucleic vendored patch]` (the `callOpts` block in `deleteProcess`). + NOTE: this pairs with a Nucleic-side change in `ContainerizedProcessHandle` (call `delete()` after + 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. + +### GUEST-side patches (require rebuilding the initfs — see below) + +Patches #1–#7 are host-side (the `Containerization` library), shipped by a normal `swift build`. +Patches #8+ live in `vminitd/` (the guest agent), which rides in the initfs OCI image. They are INERT +until that image is rebuilt from this source and published, and `ContainerEngine.vminitReference` +points at it. That is now automated: **`.github/workflows/vminit-image.yml`** builds vminitd from this +vendored tree and pushes `ghcr.io/abkslm/vminit:`; `vminitReference` is pinned to that custom +image. Bump the `-nucleicN` tag suffix and re-run the workflow whenever a guest patch changes. + +8. **`vminitd/Sources/VminitdCore/ManagedProcess.swift` — offload the blocking start off the event loop.** + `ManagedProcess.start()` did synchronous, potentially slow pipe reads (waiting for `vmexec` to + return the pid, then for the error pipe to close) while holding `state`'s Mutex, ON the calling + task — which is the gRPC handler's event-loop thread. A slow start therefore parked the loop and + head-of-line-blocked sibling execs' control RPCs sharing it. The patch splits the body into a + synchronous `startBlocking()` and an async `start()` that runs it on `DispatchQueue.global` via a + checked continuation, keeping the loop responsive. Safe because the body has no `await` and + `ManagedProcess` is `Sendable`. Marked `[Nucleic vendored patch]`. + +### PLANNED guest patch (design recorded; NOT yet implemented) + +9. **Per-exec cgroups (memory/cpu/pids isolation).** Today the whole container shares ONE cgroup + (`/container/`): `vmexec run` places the init there via the OCI `cgroupsPath` + `applyResources` + (`RunCommand.swift`), and each exec joins it via `loadFromPid(init.pid).addProcess` in + `ManagedProcess.start`. So one session's runaway RSS trips the VM OOM-killer against a *random* + sibling. Target layout (cgroup v2): make `/container/` an intermediary (enable + `cgroup.subtree_control` — `Cgroup2Manager.toggleSubtreeControllers` already skips the leaf so this + composes), move init to a leaf `/container//init`, and place each exec in its own leaf + `/container//` with generous `memory.high`/`memory.max`/`cpu.max`/`pids.max` so a + runaway session is throttled/OOM-killed *within its own cgroup*, siblings untouched — WITHOUT + hard-partitioning RAM (soft limits preserve burst). This is CROSS-CUTTING, not a one-file patch: + the per-exec limits must be carried on the exec RPC (the `CreateProcess`/exec OCI spec has no + resources field today), which means a protobuf field (`SandboxContext`) + host-side plumbing + (`Vminitd.createProcess` / `ContainerEngine.exec`) in addition to the vminitd cgroup restructure + (`ManagedContainer`, `ManagedProcess`, `vmexec/RunCommand`). Sequence it after #8 lands via CI, and + validate in a real container (a wrong v2 hierarchy fails at runtime, not at compile). + ## Re-vendoring a newer upstream commit 1. `git clone` upstream (or copy `.build/checkouts/containerization` after bumping the URL pin @@ -49,7 +115,13 @@ in-tree means the patch can't be lost to a dependency re-resolve. --exclude=images/ / third_party/containerization/` 3. Remove the `.testTarget(...)` blocks from `third_party/containerization/Package.swift`. 4. Re-apply patch #1 (the `vmExtensions` field + the `vmConfig.extensions = …` forward), patch #2 - (`LinuxProcess.killProcessGroup(_:)`), and patch #3 (the `setupIO` stdio-connection log + its - `import os` / `nucleicIOLog`). Grep for `[Nucleic vendored patch]` to find every site. + (`LinuxProcess.killProcessGroup(_:)`), patch #3 (the `setupIO` stdio-connection log + its + `import os` / `nucleicIOLog`), patch #5 (the non-blocking stdio relay: `nucleicSetNonBlocking` / + `nucleicDrainNonBlocking` + the rewritten `readabilityHandler` blocks), and patch #6 (the atomic + 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. Patch #9 (per-exec cgroups) is design-only so + far — see its entry. After re-applying any `vminitd/` patch, re-run `.github/workflows/vminit-image.yml` + to rebuild + publish the custom init image, and bump `ContainerEngine.vminitReference`. 5. Update the commit hash above and in the root `Package.swift` comment. 6. `swift build` and run the balloon tests. diff --git a/Sources/Containerization/LinuxProcess.swift b/Sources/Containerization/LinuxProcess.swift index 375d361..7f21306 100644 --- a/Sources/Containerization/LinuxProcess.swift +++ b/Sources/Containerization/LinuxProcess.swift @@ -128,6 +128,42 @@ public final class LinuxProcess: Sendable { } extension LinuxProcess { + /// [Nucleic vendored patch] Put a connected stdio FileHandle's fd into non-blocking mode so the + /// relay's reads (``nucleicDrainNonBlocking``) can never park the shared readability queue. No-op + /// if the handle is nil. + static func nucleicSetNonBlocking(_ handle: FileHandle?) { + guard let fd = handle?.fileDescriptor else { return } + let flags = fcntl(fd, F_GETFL, 0) + if flags >= 0 { _ = fcntl(fd, F_SETFL, flags | O_NONBLOCK) } + } + + /// [Nucleic vendored patch] Drain `fd` (already O_NONBLOCK) without ever blocking. Returns the + /// bytes read this pass plus whether the stream hit EOF (or a hard error). On EAGAIN it returns + /// what it has with `eof == false`; the readability `DispatchSource` fires again when more data + /// arrives. Upstream read with `FileHandle.availableData`, a *blocking* read: if one exec's guest + /// stdout wedged mid-stream, that read parked Foundation's shared readability thread and + /// head-of-line-blocked EVERY other exec's stdout/stderr relay (the "one stuck session freezes the + /// others" failure). A non-blocking drain can never park that thread, so a wedged stream is + /// contained to its own exec. + static func nucleicDrainNonBlocking(_ fd: Int32) -> (data: Data, eof: Bool) { + var out = Data() + var buf = [UInt8](repeating: 0, count: 64 * 1024) + while true { + let n = buf.withUnsafeMutableBytes { read(fd, $0.baseAddress, $0.count) } + if n > 0 { + out.append(contentsOf: buf[0.. [FileHandle?] { let handles = try await Timeout.run(seconds: 3) { try await withThrowingTaskGroup(of: (Int, FileHandle?).self) { group in @@ -170,36 +206,43 @@ extension LinuxProcess { let (stream, cc) = AsyncStream.makeStream() if let stdout = self.ioSetup.stdout { configuredStreams += 1 + // [Nucleic vendored patch] Non-blocking relay (see nucleicDrainNonBlocking): mark the + // connected fd O_NONBLOCK and drain it without a blocking read, so a wedged guest stdout + // can't head-of-line-block sibling execs' relays on Foundation's shared readability queue. + Self.nucleicSetNonBlocking(handles[1]) handles[1]?.readabilityHandler = { handle in - do { - let data = handle.availableData - if data.isEmpty { - // This block is called when the producer (the guest) closes - // the fd it is writing into. - handles[1]?.readabilityHandler = nil - cc.yield() - return + let (data, eof) = Self.nucleicDrainNonBlocking(handle.fileDescriptor) + if !data.isEmpty { + do { + try stdout.writer.write(data) + } catch { + self.logger?.error("failed to write to stdout: \(error)") } - try stdout.writer.write(data) - } catch { - self.logger?.error("failed to write to stdout: \(error)") + } + if eof { + // The guest closed the fd it was writing into. + handles[1]?.readabilityHandler = nil + cc.yield() } } } if let stderr = self.ioSetup.stderr { configuredStreams += 1 + // [Nucleic vendored patch] Non-blocking relay — same rationale as stdout above. + Self.nucleicSetNonBlocking(handles[2]) handles[2]?.readabilityHandler = { handle in - do { - let data = handle.availableData - if data.isEmpty { - handles[2]?.readabilityHandler = nil - cc.yield() - return + let (data, eof) = Self.nucleicDrainNonBlocking(handle.fileDescriptor) + if !data.isEmpty { + do { + try stderr.writer.write(data) + } catch { + self.logger?.error("failed to write to stderr: \(error)") } - try stderr.writer.write(data) - } catch { - self.logger?.error("failed to write to stderr: \(error)") + } + if eof { + handles[2]?.readabilityHandler = nil + cc.yield() } } } @@ -288,6 +331,27 @@ extension LinuxProcess { ) let result = try await t.value + + // [Nucleic vendored patch] Atomic stdio-or-abort. If a *configured* stdio stream never + // connected from the guest (its FileHandle came back nil — the failure logged in setupIO), + // starting the process would run it with a dead stream: stdin never delivered (it hangs) + // or stdout/stderr never read ("no output, just a spinner" — the 60s stall in Nucleic + // Control). Rather than launch a black-hole process, tear the just-created exec back down + // and fail fast so the caller gets a clean, retryable start error instead of an eternal + // silent stall the watchdog has to guess at. + let configured = [ + self.ioSetup.stdin != nil, self.ioSetup.stdout != nil, self.ioSetup.stderr != nil, + ] + let streamLabels = ["stdin", "stdout", "stderr"] + if let missing = (0..<3).first(where: { configured[$0] && result[$0] == nil }) { + try? await self.agent.deleteProcess(id: self.id, containerID: self.owningContainer) + throw ContainerizationError( + .internalError, + message: + "process \(self.id): \(streamLabels[missing]) stream never connected from the guest before start; aborting so the stdio transport stall surfaces as a retryable start error" + ) + } + let pid = try await self.agent.startProcess( id: self.id, containerID: self.owningContainer diff --git a/Sources/Containerization/Vminitd.swift b/Sources/Containerization/Vminitd.swift index ff691f8..90c516a 100644 --- a/Sources/Containerization/Vminitd.swift +++ b/Sources/Containerization/Vminitd.swift @@ -323,7 +323,16 @@ extension Vminitd: VirtualMachineAgent { $0.containerID = containerID } } - _ = try await client.deleteProcess(request) + // [Nucleic vendored patch] Bound the teardown RPC so a wedged agent channel can't hang an + // exec's cleanup forever. Nucleic fires `LinuxProcess.delete()` after every turn to reclaim + // the per-exec connection; if `deleteProcess` never returned, that reclaim task would leak + // and the connection would stay open — reintroducing the very accumulation the delete exists + // to prevent. Generous: a healthy delete returns in milliseconds, so this only trips a + // genuinely stuck channel, and `LinuxProcess.performDeletion` still closes the agent + // connection on the thrown deadline. + var callOpts = GRPCCore.CallOptions.defaults + callOpts.timeout = .seconds(30) + _ = try await client.deleteProcess(request, options: callOpts) } public func closeProcessStdin(id: String, containerID: String?) async throws { diff --git a/vminitd/Sources/VminitdCore/ManagedProcess.swift b/vminitd/Sources/VminitdCore/ManagedProcess.swift index ba4cd2d..af6dc2e 100644 --- a/vminitd/Sources/VminitdCore/ManagedProcess.swift +++ b/vminitd/Sources/VminitdCore/ManagedProcess.swift @@ -147,7 +147,27 @@ final class ManagedProcess: ContainerProcess, Sendable { } extension ManagedProcess { + /// [Nucleic vendored patch] Run the blocking start sequence OFF the cooperative executor / gRPC + /// event loop. `startBlocking()` does synchronous, potentially slow pipe reads (waiting for + /// `vmexec` to hand back the pid, then for the error pipe to close) while holding `state`'s Mutex. + /// Upstream ran that directly on the calling task, so a slow exec start parked the event loop and + /// head-of-line-blocked sibling execs' control RPCs multiplexed on the same loop (each host `exec` + /// dials its own connection, but NIO pins several connections per loop). Dispatching to a worker + /// keeps the loop responsive; the body has no `await` and `ManagedProcess` is `Sendable`, so it is + /// safe off-actor, and the per-exec Mutex still serializes only this exec's own operations. func start() async throws -> Int32 { + try await withCheckedThrowingContinuation { (cont: CheckedContinuation) in + DispatchQueue.global(qos: .userInitiated).async { + do { + cont.resume(returning: try self.startBlocking()) + } catch { + cont.resume(throwing: error) + } + } + } + } + + private func startBlocking() throws -> Int32 { do { return try self.state.withLock { log.info(