nucleic/nucleic
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge nucleic/lucid-river-toad-6efj into dev

Browse Source
This commit is contained in:
abkslm
2026-07-29 17:31:35 -07:00
parent 1a21d9f0dc
commit ee64dd0752
4 changed files with 130 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Sources/NucleicCore/Container/Windows/WslcBrokerClient.swift
+67 -5
View File
@@ -25,6 +25,42 @@ public actor WslcBrokerClient {
let exit: @Sendable (Int32) -> Void
}
/// A process event that arrived before its ``WslcProcessHandle`` existed.
///
/// This is not a theoretical window. The broker enqueues the `proc.exec` response *before*
/// starting the process precisely so the response cannot be overtaken — but a client still
/// cannot register on time, because resolving the request's continuation only SCHEDULES it,
/// while the notification drain is a separate task already reading the next line. A command
/// that finishes immediately therefore lands `proc.stdout` and `proc.exit` before
/// ``register(procId:sinks:)`` runs, and the old code silently dropped both — so `wait()`
/// never returned and the caller hung forever.
///
/// `runCapturing` is the most exposed path, since every short probe it runs (cgroup reads,
/// shim re-seeding) is exactly the fast case. Observed on hardware through the C# spike, which
/// hung twice on it (docs/WINDOWS_PORT.md §13.3).
private enum PendingProcEvent {
case stdout(Data)
case stderr(Data)
case exit(Int32)
}
/// Buffered events per not-yet-registered procId, drained by ``register(procId:sinks:)``.
private var pendingProcEvents: [Int64: [PendingProcEvent]] = [:]
/// The window is a continuation hop, so a handful of events is the realistic maximum; the cap
/// only exists so a procId that never registers (a broker bug) cannot grow without bound.
private static let maxPendingEventsPerProc = 512
private func buffer(_ event: PendingProcEvent, for procId: Int64) {
var events = pendingProcEvents[procId] ?? []
let isExit: Bool = if case .exit = event { true } else { false }
// Always keep the exit — losing it is the hang this mechanism exists to prevent, whereas
// dropping overflow output only truncates a transcript.
guard isExit || events.count < Self.maxPendingEventsPerProc else { return }
events.append(event)
pendingProcEvents[procId] = events
}
private let brokerdPath: String
private var child: ChildProcess?
private var connection: JSONRPCConnection?
@@ -114,6 +150,9 @@ public actor WslcBrokerClient {
// hit the same recovery path a forced stream close serves (docs/WINDOWS_PORT.md §2.3).
for (_, sinks) in procSinks { sinks.exit(-1) }
procSinks.removeAll()
// Events buffered for handles that never registered belong to the dead broker's procId
// space, which the next broker reuses from 1 — keeping them would misdeliver.
pendingProcEvents.removeAll()
// Exponential-backoff restart loop (§2.3); a successful start() resets the ladder.
while !shutdownRequested {
let backoff = restartBackoffNanos
@@ -147,26 +186,49 @@ public actor WslcBrokerClient {
func register(procId: Int64, sinks: ProcSinks) {
procSinks[procId] = sinks
// Deliver anything that arrived before this handle existed — see `pendingProcEvents`.
guard let buffered = pendingProcEvents.removeValue(forKey: procId) else { return }
for event in buffered {
switch event {
case .stdout(let data): sinks.stdout(data)
case .stderr(let data): sinks.stderr(data)
case .exit(let code):
// The process is already over. Drop the sink as `route` would have.
procSinks[procId] = nil
sinks.exit(code)
return
}
}
}
func unregister(procId: Int64) {
procSinks[procId] = nil
pendingProcEvents[procId] = nil
}
private func route(_ note: JSONRPCConnection.Notification) async {
let params = note.params
switch note.method {
case "proc.stdout", "proc.stderr":
guard let procId = params["procId"]?.intValue,
guard let rawId = params["procId"]?.intValue,
let b64 = params["b64"]?.stringValue,
let bytes = Data(base64Encoded: b64),
let sinks = procSinks[Int64(procId)]
let bytes = Data(base64Encoded: b64)
else { return }
let procId = Int64(rawId)
guard let sinks = procSinks[procId] else {
buffer(note.method == "proc.stderr" ? .stderr(bytes) : .stdout(bytes), for: procId)
return
}
(note.method == "proc.stderr" ? sinks.stderr : sinks.stdout)(bytes)
case "proc.exit":
guard let procId = params["procId"]?.intValue else { return }
guard let rawId = params["procId"]?.intValue else { return }
let procId = Int64(rawId)
let code = Int32(params["code"]?.intValue ?? -1)
procSinks.removeValue(forKey: Int64(procId))?.exit(code)
guard let sinks = procSinks.removeValue(forKey: procId) else {
buffer(.exit(code), for: procId)
return
}
sinks.exit(code)
case "session.down":
await onSessionDown(params["reason"]?.stringValue ?? "unknown")
case "image.pullProgress":
Sources/NucleicCore/Container/Windows/WslcContainerEngine.swift
+26 -7
View File
@@ -387,13 +387,32 @@ public actor WslcContainerEngine: SandboxEngine {
return try await client.call(method, params)
} catch let error as WslcBrokerClient.BrokerLost {
throw ContainerError.unavailable(error.description)
} catch let JSONRPCConnection.RPCError.server(_, message) {
switch method {
case "image.pull": throw ContainerError.imagePullFailed(message)
case "container.create", "container.start":
throw ContainerError.startFailed(message)
case "proc.exec": throw ContainerError.notRunning(message)
default: throw ContainerError.startFailed(message)
} catch let JSONRPCConnection.RPCError.server(_, message, kind) {
// Prefer the broker's own `data.kind` — it knows why it failed, whereas inferring from
// the method name is a guess that was sometimes wrong (a `proc.exec` refused because
// the image cannot drop privileges is not `notRunning`).
switch kind {
case "not_found", "not_running": throw ContainerError.notRunning(message)
case "image_pull_failed": throw ContainerError.imagePullFailed(message)
// Both mean the sandbox as a whole is not usable right now, which is what
// `.unavailable` says. `session_exists` additionally implies a manual
// `wsl --shutdown` when D13 Tier 1 could not clear it — the message carries that.
case "wslc_unavailable", "session_exists": throw ContainerError.unavailable(message)
// `unsupported` is a PERMANENT capability gap (no pty, an unmappable signal, an image
// whose setpriv cannot drop privileges), so retrying cannot help — but `ContainerError`
// has no case that says so, and `.startFailed` reads as transient. Mapped here for
// now with the reason preserved in the message; a dedicated `.unsupported` case is the
// right fix and needs the Darwin definition plus its LinuxSupport mirror to move
// together, so it is not being slipped in from the Windows side. See §13.3.
case "unsupported": throw ContainerError.startFailed(message)
case "already_exists", "start_failed": throw ContainerError.startFailed(message)
default:
// Older broker, or a failure it did not classify: fall back to the method.
switch method {
case "image.pull": throw ContainerError.imagePullFailed(message)
case "proc.exec": throw ContainerError.notRunning(message)
default: throw ContainerError.startFailed(message)
}
}
}
}
Sources/NucleicCore/JSONRPCConnection.swift
+10 -2
View File
@@ -42,7 +42,14 @@ public actor JSONRPCConnection {
public enum RPCError: Error, Sendable, Equatable {
/// The peer returned an `error` object for one of our requests.
case server(code: Int, message: String)
///
/// `kind` is `error.data.kind` when the peer sent one, and nil otherwise. The wslc broker
/// sets it on every failure (docs/WINDOWS_PORT.md §3.3) so hostd can branch on a stable
/// token — `not_running`, `already_exists`, `unsupported` — instead of guessing from which
/// method failed or, worse, matching on message text that is often empty or unhelpful
/// (§13.3 has a real example: a COM message reading "The text associated with this error
/// code could not be found").
case server(code: Int, message: String, kind: String?)
/// The connection closed before the response arrived.
case connectionClosed
}
@@ -167,7 +174,8 @@ public actor JSONRPCConnection {
continuation.resume(
throwing: RPCError.server(
code: error["code"]?.intValue ?? -1,
message: error["message"]?.stringValue ?? "RPC error"))
message: error["message"]?.stringValue ?? "RPC error",
kind: error["data"]?["kind"]?.stringValue))
} else {
continuation.resume(returning: object["result"] ?? .null)
}
docs/WINDOWS_PORT.md
+27 -8
View File
@@ -113,11 +113,30 @@ proc/session/pull notifications, exponential-backoff restart + `onReattach`),
ensure with the same registry-auth rule as the macOS engine, the identical in-guest
control-plane probe script, stats-delta resource sampling, channel-scoped
`reconcileDisk`), `WslcProcessHandle` (broker events → the shared `LineSplitter` framing;
`forceCloseStreams` on broker loss). Known gap: `JSONRPCConnection` drops JSON-RPC
`error.data`, so the broker's `data.kind` reaches Swift as message text only — a
Darwin-first `JSONRPCConnection` extension would fix it. `checkDefaultImageUpdate` returns
`forceCloseStreams` on broker loss). `checkDefaultImageUpdate` returns
`.idle` until the registry-HEAD logic relocates behind the broker (M2).
**Two fixes after the first live run [Windows ✓ — NucleicCore compiles on titan]:**
- **`error.data` is no longer dropped.** `JSONRPCConnection.RPCError.server` gained
`kind: String?` (populated from `error.data.kind`), and `WslcContainerEngine.callMapped` now
branches on it instead of inferring from which method failed — a guess that was sometimes wrong,
e.g. a `proc.exec` refused because the image cannot drop privileges is not `notRunning`. The
method-based mapping stays as the fallback for an older broker. Only one call site matched that
enum case, and the `JSONRPCConnection` tests assert the error *type*, not its payload, so the
added associated value is contained. One thing deliberately NOT done: `unsupported` denotes a
**permanent** capability gap and `ContainerError` has no case that says so, so it maps to
`.startFailed` (which reads as transient) with the reason in the message. A dedicated
`.unsupported` case is the right fix and needs the Darwin definition and its `LinuxSupport`
mirror to move together — not something to slip in from the Windows side.
- **A dropped-notification hang, the mirror of the broker bug in §13.3.** `WslcProcessHandle`
registers its sinks only *after* `proc.exec` returns, but resolving that request's continuation
merely *schedules* it while the notification drain is a separate task already reading the next
line. For a command that finishes instantly, `proc.stdout` and `proc.exit` arrived first and
`route` silently discarded both — so `wait()` never returned. `runCapturing` was the most
exposed path, since every short probe it runs is exactly that case. Fixed by buffering events
for not-yet-registered procIds and draining them in `register(procId:sinks:)`, with the exit
always kept and the buffer cleared on broker death (procIds restart at 1).
**Item 7 — Control plane. Done end-to-end [macOS ✓ incl. MCPApprovalServerTests 46/46 +
the 1449-test adapter suites][Linux ✓][Windows leg CI-pending].**
Guest: `control-bridge.js` dials `NUCLEIC_CONTROL_HOST`/`NUCLEIC_CONTROL_PORT` when set
@@ -1854,11 +1873,11 @@ same "roster is process-local" gap in a different costume.
(`echo`) put `proc.stdout`/`proc.exit` on the ordered outbound queue *ahead* of the response
carrying the `procId` that names them, so a client waiting for that exit waited forever. Fixed
by enqueueing the response before `IWslcProcess.StartAsync` — the same reason wslc itself splits
`CreateProcess` from `Start`, applied at the RPC boundary. **Note for item 6:**
`WslcBrokerClient.swift` should be checked for the mirror-image hazard — completing the
response's continuation only *schedules* it, so a reader loop can dispatch the next notification
before the procId is recorded. The spike hit exactly that and has to accept notifications for
procIds it has not yet learned.
`CreateProcess` from `Start`, applied at the RPC boundary. **The mirror-image hazard was then
confirmed and fixed in `WslcBrokerClient.swift`** (see item 6): the Swift client had the same
hang for the same reason, because completing the response's continuation only *schedules* it
while the notification drain is a separate task. It now buffers events for procIds it has not
yet learned. The C# spike, being single-flight, takes the simpler route of accepting any procId.
2. **That fix introduced a double response.** When `StartAsync` then failed, the generic handler
emitted a JSON-RPC *error* under an id that had already been answered — two responses for one
id. Now a post-response start failure is reported the way the process itself would: the reason