Merge campaign-launcher-406: fix #406 — truthful client exit self-report + bounded stderr capture
The crashed-client 'graceful' status line was the CLIENT's own Dispose-path self-report, not the launcher's observation; Run() now latches the escaping failure and the shutdown report writes reason:'crashed'. Sessions also gain a bounded client.err.log beside status.jsonl on both spawn paths. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
commit
0b05b58514
13 changed files with 1150 additions and 38 deletions
|
|
@ -103,26 +103,62 @@ amendment. Immediate workaround (confirmed live): drag-resize the
|
|||
windowed client — resize events rebuild the swapchain (#387) and the
|
||||
retail UI rescales from its 800x600 authored canvas.
|
||||
|
||||
## #406 — Launcher records a crashed client as `exited{code:0,reason:"graceful"}`
|
||||
## #406 — CLOSED: Launcher records a crashed client as `exited{code:0,reason:"graceful"}`
|
||||
|
||||
**Status:** OPEN (Campaign CC gate round 1, 2026-08-16)
|
||||
**Status:** DONE (this commit, 2026-08-16)
|
||||
**Severity:** MEDIUM (diagnosis-misleading, not data-loss)
|
||||
|
||||
Found while diagnosing #405: the client process died with exit code
|
||||
`0xE0434352` (.NET unhandled exception, stack on stderr), but the
|
||||
launcher's session status stream recorded `{"e":"exited","code":0,
|
||||
"reason":"graceful"}` — the exact opposite of what happened. Running the
|
||||
identical binary + session config from a console shows the true nonzero
|
||||
exit code, so the corruption is in the launcher's session-orchestrator
|
||||
exit observation (wrong process handle/exit-code read, or a default that
|
||||
masks the real code), not in the client. §LA1 explicitly promises
|
||||
`exited{code,reason}` carries the real termination; a launcher that
|
||||
reports "graceful" for a crash sends any future gate/automation
|
||||
diagnosis in the wrong direction (it did exactly that this round until
|
||||
the console repro). Investigate the launcher-side session orchestrator's
|
||||
exit capture; a test should pin a nonzero-exit child producing
|
||||
`exited{code:<nonzero>,reason:"crashed"|"failed"}` per the LA contract's
|
||||
vocabulary.
|
||||
"reason":"graceful"}` — the exact opposite of what happened.
|
||||
|
||||
Root cause was NOT in the launcher's process supervision (its own
|
||||
OS-level exit-code read was always correct) — it was in the CLIENT's own
|
||||
self-report. `GameWindow.Dispose()` (`src/AcDream.App/Rendering/GameWindow.cs`)
|
||||
runs unconditionally via `Program.cs`'s `using var window = new
|
||||
GameWindow(...)` even when invoked mid-unwind of an exception that
|
||||
escaped `Run()`'s Silk.NET frame loop — the resource-shutdown transaction
|
||||
itself can converge cleanly (nothing it tears down touches the crash),
|
||||
so `CompleteShutdown` had no way to tell "normal `Run()` return" from "an
|
||||
exception is propagating through me right now" and always wrote the
|
||||
hardcoded `exited{code:0,reason:"graceful"}`. Fixed by latching
|
||||
`_runFailure` in `Run()`'s existing `catch (Exception failure)` block
|
||||
(right before the `throw;` that already existed for the
|
||||
`_constructionCleanup.RetainFrom(failure)` ledger) and consulting it from
|
||||
a new `ReportExited` method that is now the ONE call site for the
|
||||
terminal status write: `exited{code:1,reason:"crashed"}` when a crash was
|
||||
observed, `exited{code:0,reason:"graceful"}` on a real graceful
|
||||
Dispose(), `exited{code:1,reason:"shutdown-incomplete"}` unchanged for a
|
||||
non-crash teardown failure. `"crashed"` is a new value for the already-
|
||||
free-text `reason` field (§LA1's `exited{code,reason}` vocabulary pins
|
||||
the EVENT name, not an enum of `reason` strings — `StatusEventParser`
|
||||
already round-trips any string there) so no wire-contract amendment was
|
||||
needed. Pinned as a source-shape test (`GameWindowCrashStatusTests`) since
|
||||
`GameWindow` cannot be constructed without a live GPU/window.
|
||||
|
||||
Sibling gap fixed in the same commit: the launcher previously discarded
|
||||
the child's stdout/stderr entirely, which is why diagnosing this exact
|
||||
crash required a manual console re-run. Added
|
||||
`BoundedProcessOutputCapture` (`src/AcDream.Launcher.Core/Launching/`) —
|
||||
a 2 MiB-capped, additive-only sink mirroring `SessionStatusWriter`'s
|
||||
open-append-flush-close-per-write posture (a long-lived write handle is
|
||||
NOT actually concurrently readable on Windows even with
|
||||
`FileShare.Read` — confirmed by isolated repro) — wired into BOTH
|
||||
`SystemChildProcess` (`ProcessStartInfo.RedirectStandardError` +
|
||||
`ErrorDataReceived`; used on Linux for every child and on Windows for
|
||||
graphical/non-console children, i.e. exactly this bug's own App/GUI
|
||||
scenario) and `WindowsSystemChildProcess` (a real native pipe via a new
|
||||
`CreateChildOutputPipe`, mirroring the existing stdin pipe in the
|
||||
opposite direction, drained on a background pump thread; used on Windows
|
||||
for console-capable children, i.e. Headless). The capture path is opt-in
|
||||
via a new `LauncherProcessSpec.StderrLogPath` (null = behave exactly as
|
||||
before) threaded through `SessionConfigComposer` → `client.err.log`
|
||||
beside `status.jsonl` in the per-session directory →
|
||||
`LauncherExecutableSet.CreatePlaySpec`/`CreateProbeSpec` →
|
||||
`LauncherOrchestrator`. Real end-to-end tests
|
||||
(`LauncherProcessSupervisorTests`) spawn an actual child via both code
|
||||
paths and assert the captured file.
|
||||
|
||||
## #405 — CLOSED: chargen/summary preview leases missing Transfer killed every retail-UI window load
|
||||
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue