From e56a3629caf6a64219c3f13ede45218d52a6f41c Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 18:28:03 +0000 Subject: [PATCH 1/4] =?UTF-8?q?feat(examples):=20the=20flagship=20leak=20w?= =?UTF-8?q?here=20it=20lives=20=E2=80=94=20a=20WPF=20repro=20pair,=20witne?= =?UTF-8?q?ssed=20on=20Windows?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The console pair is the shape stripped to its bones; this is the same bug in its native habitat. A DocumentWindow subscribes to the process-lifetime settings hub in its constructor, an unsubscribe exists behind Cleanup(keepAlive), and the Closed handler passes true — so every closed window, with its whole visual tree, stays in the hub's delegate list. The fix variant moves the release into OnClosed, the method WPF itself calls at the end of a window's life. What is platform-bound is only what has to be. ANALYSIS is not: the subscription binds through System.ComponentModel and the release is recognised by teardown name, so gate A runs owen check on the pair on BOTH legs and gets the same verdict without the WindowsDesktop reference pack (verified locally with and without it). The projects compile anywhere (EnableWindowsTargeting), so XAML and code-behind defects surface at desk time rather than on the Windows runner. RUNNING the sample is the part that needs Windows, and that is exactly what the new Windows-only steps do: hold the app live, attach the witness through its public CLI, and require the JSON artifact to name the path AppSettings -> PropertyChanged -> _invocationList -> handler -> DocumentWindow. The ok side is held to the claim the fix actually makes — the hub retains nothing, asserted as the absence of a static-event root — not to "the heap is empty". WPF's own focus and input statics may legitimately hold the last closed window; calling that our leak would be the overreach Owen refuses everywhere else. Holding a sample for a witness needed a second mechanism. A CI runner's stdin is not a console, so the merged ReadLine hold falls straight through and the witness attaches to nothing. Both pairs now share a Hold helper: ReadLine stays the default (scripts/flagship-demo.sh keeps its FIFO, unchanged and re-verified), and setting OWEN_FLAGSHIP_STOP switches to a stop-file release. Either way the hold is bounded by OWEN_FLAGSHIP_HOLD_SECONDS (default 300), so a forgotten sample cannot outlive its job. Verified locally: both WPF projects build on Linux; owen check gives 1/OWN001 on bad and 0 on ok for all four variants; the merged demo script still passes both variants; the new Windows step's control flow (pid parsed from the hold line, stop-file release, witness attach, JSON assertions) was rehearsed end-to-end against the console sample, and both embedded validators were tested to accept the real artifact and reject the opposite one. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M --- .github/workflows/ci.yml | 113 ++++++++++++++++++ examples/flagship/README.md | 47 ++++++++ examples/flagship/console/bad/DocumentApp.cs | 8 +- examples/flagship/console/bad/Hold.cs | 49 ++++++++ examples/flagship/console/ok/DocumentApp.cs | 8 +- examples/flagship/console/ok/Hold.cs | 49 ++++++++ examples/flagship/wpf/bad/App.xaml | 4 + examples/flagship/wpf/bad/App.xaml.cs | 51 ++++++++ examples/flagship/wpf/bad/AppSettings.cs | 33 +++++ .../wpf/bad/BadDocumentWindows.csproj | 16 +++ examples/flagship/wpf/bad/DocumentWindow.xaml | 10 ++ .../flagship/wpf/bad/DocumentWindow.xaml.cs | 45 +++++++ examples/flagship/wpf/bad/Hold.cs | 44 +++++++ examples/flagship/wpf/ok/App.xaml | 4 + examples/flagship/wpf/ok/App.xaml.cs | 48 ++++++++ examples/flagship/wpf/ok/AppSettings.cs | 33 +++++ examples/flagship/wpf/ok/DocumentWindow.xaml | 10 ++ .../flagship/wpf/ok/DocumentWindow.xaml.cs | 37 ++++++ examples/flagship/wpf/ok/Hold.cs | 44 +++++++ .../flagship/wpf/ok/OkDocumentWindows.csproj | 16 +++ 20 files changed, 655 insertions(+), 14 deletions(-) create mode 100644 examples/flagship/console/bad/Hold.cs create mode 100644 examples/flagship/console/ok/Hold.cs create mode 100644 examples/flagship/wpf/bad/App.xaml create mode 100644 examples/flagship/wpf/bad/App.xaml.cs create mode 100644 examples/flagship/wpf/bad/AppSettings.cs create mode 100644 examples/flagship/wpf/bad/BadDocumentWindows.csproj create mode 100644 examples/flagship/wpf/bad/DocumentWindow.xaml create mode 100644 examples/flagship/wpf/bad/DocumentWindow.xaml.cs create mode 100644 examples/flagship/wpf/bad/Hold.cs create mode 100644 examples/flagship/wpf/ok/App.xaml create mode 100644 examples/flagship/wpf/ok/App.xaml.cs create mode 100644 examples/flagship/wpf/ok/AppSettings.cs create mode 100644 examples/flagship/wpf/ok/DocumentWindow.xaml create mode 100644 examples/flagship/wpf/ok/DocumentWindow.xaml.cs create mode 100644 examples/flagship/wpf/ok/Hold.cs create mode 100644 examples/flagship/wpf/ok/OkDocumentWindows.csproj diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51c60b0b..2c04009c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2382,6 +2382,27 @@ jobs: set -e echo "$out" [ "$rc" -eq 0 ] || { echo "FAIL: flagship ok must scan clean (exit 0), got $rc"; exit 1; } + - name: "flagship WPF repro: bad is OWN001, ok is clean — on BOTH platforms (A2)" + run: | + # The WPF pair is analysed everywhere, not only on Windows: the + # subscription binds through System.ComponentModel and the release is + # recognised by teardown NAME (`OnClosed`), so the verdict does not + # depend on the WindowsDesktop reference pack. Only RUNNING the + # sample needs Windows — that is the step below. + set +e + out=$(owen check "$GITHUB_WORKSPACE/examples/flagship/wpf/bad" --fail-on-finding 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 1 ] || { echo "FAIL: flagship WPF bad must exit 1 (findings), got $rc"; exit 1; } + echo "$out" | grep -q "OWN001" || { echo "FAIL: flagship WPF bad must be flagged OWN001"; exit 1; } + echo "$out" | grep -q "DocumentWindow" || { echo "FAIL: the finding must name the leaking window"; exit 1; } + set +e + out=$(owen check "$GITHUB_WORKSPACE/examples/flagship/wpf/ok" --fail-on-finding 2>&1) + rc=$? + set -e + echo "$out" + [ "$rc" -eq 0 ] || { echo "FAIL: flagship WPF ok must scan clean (exit 0), got $rc"; exit 1; } - name: "core internal crash surfaces as owen exit 5, politely — never a clean scan (A1)" if: runner.os == 'Linux' run: | @@ -2445,6 +2466,98 @@ jobs: echo "$out" [ "$rc" -eq 2 ] || { echo "FAIL: a failed attach must exit 2, never read as clean, got $rc"; exit 1; } dotnet "$GITHUB_WORKSPACE"/audit/runtime/RetentionPath/bin/Release/net8.0/RetentionPath.dll selftest + - name: "WPF flagship on Windows: the witness names the window the hub is holding (A2/A3)" + if: runner.os == 'Windows' + run: | + # The one thing that genuinely needs Windows: RUN the WPF sample and + # attach the witness to it. Everything else about this pair (build, + # XAML, the static verdict) is proven on every platform above. + # + # The pid comes from the app's own hold line, not from `$!`: under + # git-bash `$!` is the shell's job, not the dotnet.exe underneath it. + # The hold is released by creating a stop file — a runner's stdin is + # not a console, so a ReadLine hold would fall straight through. + dotnet build "$GITHUB_WORKSPACE/examples/flagship/wpf/bad" -c Release -v quiet + APP="$GITHUB_WORKSPACE/examples/flagship/wpf/bad/bin/Release/net8.0-windows/BadDocumentWindows.dll" + WITNESS="$GITHUB_WORKSPACE/audit/runtime/RetentionPath/bin/Release/net8.0/RetentionPath.dll" + STOP="$RUNNER_TEMP/wpf-stop" + LOG="$RUNNER_TEMP/wpf-app.log" + rm -f "$STOP" + OWEN_FLAGSHIP_HOLD=1 OWEN_FLAGSHIP_STOP="$STOP" OWEN_FLAGSHIP_HOLD_SECONDS=240 \ + dotnet "$APP" > "$LOG" 2>&1 & + for _ in $(seq 1 90); do grep -q "holding (pid" "$LOG" 2>/dev/null && break; sleep 1; done + cat "$LOG" + grep -q "holding (pid" "$LOG" || { echo "FAIL: the WPF sample never reached its hold point"; exit 1; } + grep -q "200 still subscribed" "$LOG" || { echo "FAIL: the sample must report 200 live subscriptions"; exit 1; } + PID=$(sed -n 's/.*holding (pid \([0-9]*\)).*/\1/p' "$LOG" | head -1) + set +e + dotnet "$WITNESS" roots --pid "$PID" --type Owen.Flagship.Wpf.DocumentWindow \ + --out "$RUNNER_TEMP/wpf-runtime.json" + WRC=$? + set -e + touch "$STOP" + cat "$RUNNER_TEMP/wpf-runtime.json" + [ "$WRC" -eq 1 ] || { echo "FAIL: witness must exit 1 (RETAINED) on the bad WPF sample, got $WRC"; exit 1; } + # JSON is the artifact; grep is not a parser. Semantic anchors only — + # addresses and hop counts are not a contract. + python3 - "$RUNNER_TEMP/wpf-runtime.json" <<'PY' + import json, sys + doc = json.load(open(sys.argv[1], encoding="utf-8")) + problems = [] + if doc.get("verdict") != "RETAINED": + problems.append(f"verdict {doc.get('verdict')!r}, want RETAINED") + roots = (doc.get("retained") or [{}])[0].get("roots") or [] + if "static-event" not in {r.get("kind") for r in roots}: + problems.append(f"no static-event root (got {sorted({r.get('kind') for r in roots})})") + text = " ".join(" ".join(r.get("path", [])) + str(r.get("holder", "")) for r in roots) + for anchor in ("AppSettings", "PropertyChanged", "_invocationList", "DocumentWindow"): + if anchor not in text: + problems.append(f"retention path lacks the {anchor!r} anchor") + for p in problems: + print(f"FAIL: {p}", file=sys.stderr) + sys.exit(1 if problems else 0) + PY + - name: "WPF flagship on Windows: the fixed variant releases the subscription (A2)" + if: runner.os == 'Windows' + run: | + # The ok side's claim is about OUR subscription, so that is what is + # asserted: the hub holds nothing, and no static-event root reaches a + # closed window. The witness exit code is deliberately NOT the gate + # here — WPF's own focus/input statics may legitimately hold the last + # closed window, which is a framework fact, not our leak. + dotnet build "$GITHUB_WORKSPACE/examples/flagship/wpf/ok" -c Release -v quiet + APP="$GITHUB_WORKSPACE/examples/flagship/wpf/ok/bin/Release/net8.0-windows/OkDocumentWindows.dll" + WITNESS="$GITHUB_WORKSPACE/audit/runtime/RetentionPath/bin/Release/net8.0/RetentionPath.dll" + STOP="$RUNNER_TEMP/wpf-ok-stop" + LOG="$RUNNER_TEMP/wpf-ok-app.log" + rm -f "$STOP" + OWEN_FLAGSHIP_HOLD=1 OWEN_FLAGSHIP_STOP="$STOP" OWEN_FLAGSHIP_HOLD_SECONDS=240 \ + dotnet "$APP" > "$LOG" 2>&1 & + for _ in $(seq 1 90); do grep -q "holding (pid" "$LOG" 2>/dev/null && break; sleep 1; done + cat "$LOG" + grep -q "0 still subscribed" "$LOG" || { echo "FAIL: the fixed sample must report 0 live subscriptions"; exit 1; } + PID=$(sed -n 's/.*holding (pid \([0-9]*\)).*/\1/p' "$LOG" | head -1) + set +e + dotnet "$WITNESS" roots --pid "$PID" --type Owen.Flagship.Wpf.DocumentWindow \ + --out "$RUNNER_TEMP/wpf-ok-runtime.json" + WRC=$? + set -e + touch "$STOP" + # rc 0/1 both mean the witness READ the heap and reached a verdict; + # anything else (2) means it never got to look, which is a failure of + # the step, not a property of the sample. + [ "$WRC" -le 1 ] || { echo "FAIL: witness could not read the process (exit $WRC)"; exit 1; } + cat "$RUNNER_TEMP/wpf-ok-runtime.json" + python3 - "$RUNNER_TEMP/wpf-ok-runtime.json" <<'PY' + import json, sys + doc = json.load(open(sys.argv[1], encoding="utf-8")) + roots = (doc.get("retained") or [{}])[0].get("roots") or [] + bad = [r for r in roots if r.get("kind") == "static-event"] + if bad: + print(f"FAIL: the fixed variant still has static-event retention: {bad}", file=sys.stderr) + sys.exit(1) + print(f"ok: verdict {doc.get('verdict')}, no static-event root") + PY - name: "flagship demo orchestrator end-to-end: bad DEMONSTRATED, ok VERIFIED (A3/A4)" if: runner.os == 'Linux' run: | diff --git a/examples/flagship/README.md b/examples/flagship/README.md index 994dec86..bdd34ea1 100644 --- a/examples/flagship/README.md +++ b/examples/flagship/README.md @@ -47,3 +47,50 @@ count goes to zero: the two halves of the same evidence. Both variants are smoke-checked in CI (gate A) against the installed `Owen.Cli` on Linux and Windows: `bad` must exit 1 with OWN001, `ok` must exit 0. + +## The same bug where it actually lives (`wpf/`) + +The console pair is the shape stripped to its bones. `wpf/` is that shape in +its native habitat: a `DocumentWindow` subscribed to the settings hub in its +constructor, an unsubscribe behind `Cleanup(keepAlive)`, and a `Closed` +handler that passes `true`. Every closed window — its whole visual tree — is +retained by the hub's delegate list. + +``` +dotnet run --project examples/flagship/wpf/bad (Windows) + → opened and closed 200 document windows; 200 still subscribed — every one + of them, with its whole visual tree, is retained by the static settings hub. + +owen check examples/flagship/wpf/bad --fail-on-finding + → OWN001 … DocumentWindow … (exit 1) +``` + +The fix (`wpf/ok/`) moves the release into `OnClosed` — the method WPF itself +calls at the end of a window's life — unconditionally. Same windows, same +subscription; the count goes to zero. + +Note what is and is not platform-bound. **Analysis is not**: the subscription +binds through `System.ComponentModel` and the release is recognised by +teardown name, so `owen check` reaches the same verdict on Linux, macOS and +Windows, and the projects themselves compile anywhere +(`EnableWindowsTargeting`). Only **running** the sample needs Windows, which +is where CI attaches the runtime witness and requires it to name the path: + +``` +AppSettings → PropertyChanged → _invocationList → handler → DocumentWindow +``` + +The `ok` side is held to the claim the fix actually makes — the hub retains +nothing — rather than to "the heap is empty": WPF's own focus and input +statics may legitimately hold the last closed window, and calling that our +leak would be the same overreach Owen refuses everywhere else. + +### Holding a sample for a witness + +Both pairs support `OWEN_FLAGSHIP_HOLD=1`, which parks the process after the +work is done and prints `holding (pid N)`. Release it by sending a line on +stdin (what `scripts/flagship-demo.sh` does through a FIFO) or, when stdin is +not a console — every CI runner — by setting `OWEN_FLAGSHIP_STOP=` and +creating that file. Either way the hold is bounded by +`OWEN_FLAGSHIP_HOLD_SECONDS` (default 300), so a forgotten sample cannot +outlive its job. diff --git a/examples/flagship/console/bad/DocumentApp.cs b/examples/flagship/console/bad/DocumentApp.cs index e8964d95..7841cf10 100644 --- a/examples/flagship/console/bad/DocumentApp.cs +++ b/examples/flagship/console/bad/DocumentApp.cs @@ -77,12 +77,6 @@ public static void Main() $"opened and closed 1000 views; " + $"{AppSettings.Instance.SubscriberCount} still subscribed — " + "every one of them is retained by the static publisher."); - if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1") - { - // Keep the heap alive for a runtime witness (the demo script and - // the CI end-to-end smoke attach retention-path to this process). - Console.WriteLine($"holding (pid {Environment.ProcessId}) — send a line to exit."); - Console.ReadLine(); - } + Hold.IfAsked(); } } diff --git a/examples/flagship/console/bad/Hold.cs b/examples/flagship/console/bad/Hold.cs new file mode 100644 index 00000000..d749d1a5 --- /dev/null +++ b/examples/flagship/console/bad/Hold.cs @@ -0,0 +1,49 @@ +// Keeping the heap alive for a runtime witness. +// +// Two release mechanisms, because the two callers differ. `scripts/flagship- +// demo.sh` keeps a FIFO on stdin and releases the sample with a line — the +// default. A CI runner's stdin is NOT a console, where a ReadLine hold would +// fall straight through and the witness would attach to nothing; those callers +// set OWEN_FLAGSHIP_STOP and release the sample by creating that file. Both +// paths are bounded by a deadline, so a stray sample can never wedge a job. +using System; +using System.IO; +using System.Threading; + +namespace Owen.Flagship; + +internal static class Hold +{ + private const int DefaultSeconds = 300; + + public static void IfAsked() + { + if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; + + string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); + int seconds = int.TryParse( + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 + ? s : DefaultSeconds; + + // The pid line is the orchestration contract: whoever launched this + // process waits for it before attaching. + Console.WriteLine($"holding (pid {Environment.ProcessId}) — " + + (stopFile is null + ? "send a line to exit." + : $"create {stopFile} to exit, or wait {seconds}s.")); + Console.Out.Flush(); + + if (stopFile is null) + { + Console.ReadLine(); + return; + } + + DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); + while (DateTime.UtcNow < deadline) + { + if (File.Exists(stopFile)) return; + Thread.Sleep(200); + } + } +} diff --git a/examples/flagship/console/ok/DocumentApp.cs b/examples/flagship/console/ok/DocumentApp.cs index 04b2164a..fb530f6a 100644 --- a/examples/flagship/console/ok/DocumentApp.cs +++ b/examples/flagship/console/ok/DocumentApp.cs @@ -65,12 +65,6 @@ public static void Main() Console.WriteLine( $"opened and closed 1000 views; " + $"{AppSettings.Instance.SubscriberCount} still subscribed."); - if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1") - { - // Keep the heap alive for a runtime witness (the demo script and - // the CI end-to-end smoke attach retention-path to this process). - Console.WriteLine($"holding (pid {Environment.ProcessId}) — send a line to exit."); - Console.ReadLine(); - } + Hold.IfAsked(); } } diff --git a/examples/flagship/console/ok/Hold.cs b/examples/flagship/console/ok/Hold.cs new file mode 100644 index 00000000..d749d1a5 --- /dev/null +++ b/examples/flagship/console/ok/Hold.cs @@ -0,0 +1,49 @@ +// Keeping the heap alive for a runtime witness. +// +// Two release mechanisms, because the two callers differ. `scripts/flagship- +// demo.sh` keeps a FIFO on stdin and releases the sample with a line — the +// default. A CI runner's stdin is NOT a console, where a ReadLine hold would +// fall straight through and the witness would attach to nothing; those callers +// set OWEN_FLAGSHIP_STOP and release the sample by creating that file. Both +// paths are bounded by a deadline, so a stray sample can never wedge a job. +using System; +using System.IO; +using System.Threading; + +namespace Owen.Flagship; + +internal static class Hold +{ + private const int DefaultSeconds = 300; + + public static void IfAsked() + { + if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; + + string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); + int seconds = int.TryParse( + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 + ? s : DefaultSeconds; + + // The pid line is the orchestration contract: whoever launched this + // process waits for it before attaching. + Console.WriteLine($"holding (pid {Environment.ProcessId}) — " + + (stopFile is null + ? "send a line to exit." + : $"create {stopFile} to exit, or wait {seconds}s.")); + Console.Out.Flush(); + + if (stopFile is null) + { + Console.ReadLine(); + return; + } + + DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); + while (DateTime.UtcNow < deadline) + { + if (File.Exists(stopFile)) return; + Thread.Sleep(200); + } + } +} diff --git a/examples/flagship/wpf/bad/App.xaml b/examples/flagship/wpf/bad/App.xaml new file mode 100644 index 00000000..4f551d0f --- /dev/null +++ b/examples/flagship/wpf/bad/App.xaml @@ -0,0 +1,4 @@ + diff --git a/examples/flagship/wpf/bad/App.xaml.cs b/examples/flagship/wpf/bad/App.xaml.cs new file mode 100644 index 00000000..8da7fbc8 --- /dev/null +++ b/examples/flagship/wpf/bad/App.xaml.cs @@ -0,0 +1,51 @@ +// The driver: open and close document windows the way a user would, then ask +// the publisher how many of them it is still holding. The program proves its +// own leak — no profiler required to see the number. +// +// Run it (Windows): dotnet run --project examples/flagship/wpf/bad +// Analyze it: owen check examples/flagship/wpf/bad --fail-on-finding +using System; +using System.Windows; +using System.Windows.Threading; + +namespace Owen.Flagship.Wpf; + +public partial class App : Application +{ + private const int Cycles = 200; + + protected override void OnStartup(StartupEventArgs e) + { + base.OnStartup(e); + // Show()/Close() are message-driven. Driving them straight from + // OnStartup would run them before the loop that delivers those + // messages exists, so the cycle is queued onto the dispatcher and + // runs once the application is pumping. + Dispatcher.BeginInvoke(new Action(OpenAndCloseDocuments), DispatcherPriority.ApplicationIdle); + } + + private void OpenAndCloseDocuments() + { + for (var i = 0; i < Cycles; i++) + { + var window = new DocumentWindow(AppSettings.Instance); + window.Show(); + window.Close(); + } + + // Give the GC every chance: whatever survives this is retained by a + // live reference, not by collection lag. + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Console.WriteLine( + $"opened and closed {Cycles} document windows; " + + $"{AppSettings.Instance.SubscriberCount} still subscribed — " + + "every one of them, with its whole visual tree, is retained by the " + + "static settings hub."); + + Hold.IfAsked(); + Shutdown(); + } +} diff --git a/examples/flagship/wpf/bad/AppSettings.cs b/examples/flagship/wpf/bad/AppSettings.cs new file mode 100644 index 00000000..66855324 --- /dev/null +++ b/examples/flagship/wpf/bad/AppSettings.cs @@ -0,0 +1,33 @@ +// The process-lifetime publisher — the WPF idiom, unchanged from a thousand +// real apps: one settings hub, reachable from anywhere, raising +// PropertyChanged so that every open view re-renders itself. +// +// Nothing here is wrong. The bug is in what subscribes to it and never leaves. +using System.ComponentModel; + +namespace Owen.Flagship.Wpf; + +/// Static settings hub; lives as long as the process does. +public sealed class AppSettings : INotifyPropertyChanged +{ + public static readonly AppSettings Instance = new(); + + public event PropertyChangedEventHandler? PropertyChanged; + + private string _theme = "Light"; + + public string Theme + { + get => _theme; + set + { + if (_theme == value) return; + _theme = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Theme))); + } + } + + /// How many handlers the publisher is still holding — the leak, + /// counted by the leaking program itself. + public int SubscriberCount => PropertyChanged?.GetInvocationList().Length ?? 0; +} diff --git a/examples/flagship/wpf/bad/BadDocumentWindows.csproj b/examples/flagship/wpf/bad/BadDocumentWindows.csproj new file mode 100644 index 00000000..93902fd3 --- /dev/null +++ b/examples/flagship/wpf/bad/BadDocumentWindows.csproj @@ -0,0 +1,16 @@ + + + + Exe + net8.0-windows + true + enable + Owen.Flagship.Wpf + + true + + diff --git a/examples/flagship/wpf/bad/DocumentWindow.xaml b/examples/flagship/wpf/bad/DocumentWindow.xaml new file mode 100644 index 00000000..c74f4f10 --- /dev/null +++ b/examples/flagship/wpf/bad/DocumentWindow.xaml @@ -0,0 +1,10 @@ + + + + + + diff --git a/examples/flagship/wpf/bad/DocumentWindow.xaml.cs b/examples/flagship/wpf/bad/DocumentWindow.xaml.cs new file mode 100644 index 00000000..d9447ad6 --- /dev/null +++ b/examples/flagship/wpf/bad/DocumentWindow.xaml.cs @@ -0,0 +1,45 @@ +// The flagship leak in its native habitat (the #278 shape, WPF edition). +// +// A document window subscribes to the process-lifetime settings hub in its +// constructor so it can restyle itself when the theme changes. An unsubscribe +// EXISTS — `Cleanup` — but it sits behind a parameter guard, and the close +// path calls `Cleanup(keepAlive: true)`, so the `-=` never runs. Every closed +// window stays reachable through the hub's delegate list: its whole visual +// tree, its data context, its images — all of it, one leaked window per +// document the user ever opened. +// +// This is exactly the bug class where "a matching -= exists somewhere in the +// class" reads as safe. Owen does not accept existence as evidence: a +// parameter-guarded `-=` in a non-teardown method cannot be proven to run, so +// the subscription is flagged (OWN001). +using System.ComponentModel; +using System.Windows; + +namespace Owen.Flagship.Wpf; + +public partial class DocumentWindow : Window +{ + private readonly AppSettings _settings; + + public DocumentWindow(AppSettings settings) + { + InitializeComponent(); + _settings = settings; + _settings.PropertyChanged += OnSettingsChanged; + Closed += (_, _) => Cleanup(keepAlive: true); + Render(); + } + + private void OnSettingsChanged(object? sender, PropertyChangedEventArgs e) => Render(); + + private void Render() => ThemeText.Text = $"theme: {_settings.Theme}"; + + public void Cleanup(bool keepAlive) + { + if (!keepAlive) + { + _settings.PropertyChanged -= OnSettingsChanged; + } + // detach only the cheap extras when the caller asked to keep the core + } +} diff --git a/examples/flagship/wpf/bad/Hold.cs b/examples/flagship/wpf/bad/Hold.cs new file mode 100644 index 00000000..2bd563f3 --- /dev/null +++ b/examples/flagship/wpf/bad/Hold.cs @@ -0,0 +1,44 @@ +// Keeping the heap alive for a runtime witness, without depending on stdin. +// +// The console flagship holds on `Console.ReadLine()`, which works because its +// demo script feeds it a FIFO. A CI runner's stdin is NOT a console: ReadLine +// returns null immediately, the process exits, and the witness attaches to +// nothing. So the hold here waits for a file the orchestrator creates, with a +// deadline so a stray sample can never wedge a job. +using System; +using System.IO; +using System.Threading; + +namespace Owen.Flagship.Wpf; + +internal static class Hold +{ + private const int DefaultSeconds = 300; + + /// With OWEN_FLAGSHIP_HOLD=1, print the pid and block until the + /// file named by OWEN_FLAGSHIP_STOP appears (or the deadline passes). + public static void IfAsked() + { + if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; + + string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); + int seconds = int.TryParse( + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 + ? s : DefaultSeconds; + + // The pid line is the orchestration contract: whoever launched this + // process waits for it before attaching. + Console.WriteLine($"holding (pid {Environment.ProcessId}) — " + + (stopFile is null + ? $"exits after {seconds}s." + : $"create {stopFile} to exit, or wait {seconds}s.")); + Console.Out.Flush(); + + DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); + while (DateTime.UtcNow < deadline) + { + if (stopFile != null && File.Exists(stopFile)) return; + Thread.Sleep(200); + } + } +} diff --git a/examples/flagship/wpf/ok/App.xaml b/examples/flagship/wpf/ok/App.xaml new file mode 100644 index 00000000..4f551d0f --- /dev/null +++ b/examples/flagship/wpf/ok/App.xaml @@ -0,0 +1,4 @@ + diff --git a/examples/flagship/wpf/ok/App.xaml.cs b/examples/flagship/wpf/ok/App.xaml.cs new file mode 100644 index 00000000..ea94a530 --- /dev/null +++ b/examples/flagship/wpf/ok/App.xaml.cs @@ -0,0 +1,48 @@ +// The driver, unchanged from `bad/`: open and close document windows the way a +// user would, then ask the publisher how many of them it is still holding. +// Same program, same windows, same subscription — the count is zero because +// the release moved into a teardown. +// +// Run it (Windows): dotnet run --project examples/flagship/wpf/ok +// Analyze it: owen check examples/flagship/wpf/ok --fail-on-finding +using System; +using System.Windows; +using System.Windows.Threading; + +namespace Owen.Flagship.Wpf; + +public partial class App : Application +{ + private const int Cycles = 200; + + protected override void OnStartup(StartupEventArgs e) + { + base.OnStartup(e); + // Show()/Close() are message-driven. Driving them straight from + // OnStartup would run them before the loop that delivers those + // messages exists, so the cycle is queued onto the dispatcher and + // runs once the application is pumping. + Dispatcher.BeginInvoke(new Action(OpenAndCloseDocuments), DispatcherPriority.ApplicationIdle); + } + + private void OpenAndCloseDocuments() + { + for (var i = 0; i < Cycles; i++) + { + var window = new DocumentWindow(AppSettings.Instance); + window.Show(); + window.Close(); + } + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Console.WriteLine( + $"opened and closed {Cycles} document windows; " + + $"{AppSettings.Instance.SubscriberCount} still subscribed."); + + Hold.IfAsked(); + Shutdown(); + } +} diff --git a/examples/flagship/wpf/ok/AppSettings.cs b/examples/flagship/wpf/ok/AppSettings.cs new file mode 100644 index 00000000..66855324 --- /dev/null +++ b/examples/flagship/wpf/ok/AppSettings.cs @@ -0,0 +1,33 @@ +// The process-lifetime publisher — the WPF idiom, unchanged from a thousand +// real apps: one settings hub, reachable from anywhere, raising +// PropertyChanged so that every open view re-renders itself. +// +// Nothing here is wrong. The bug is in what subscribes to it and never leaves. +using System.ComponentModel; + +namespace Owen.Flagship.Wpf; + +/// Static settings hub; lives as long as the process does. +public sealed class AppSettings : INotifyPropertyChanged +{ + public static readonly AppSettings Instance = new(); + + public event PropertyChangedEventHandler? PropertyChanged; + + private string _theme = "Light"; + + public string Theme + { + get => _theme; + set + { + if (_theme == value) return; + _theme = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Theme))); + } + } + + /// How many handlers the publisher is still holding — the leak, + /// counted by the leaking program itself. + public int SubscriberCount => PropertyChanged?.GetInvocationList().Length ?? 0; +} diff --git a/examples/flagship/wpf/ok/DocumentWindow.xaml b/examples/flagship/wpf/ok/DocumentWindow.xaml new file mode 100644 index 00000000..c74f4f10 --- /dev/null +++ b/examples/flagship/wpf/ok/DocumentWindow.xaml @@ -0,0 +1,10 @@ + + + + + + diff --git a/examples/flagship/wpf/ok/DocumentWindow.xaml.cs b/examples/flagship/wpf/ok/DocumentWindow.xaml.cs new file mode 100644 index 00000000..5fe3dde8 --- /dev/null +++ b/examples/flagship/wpf/ok/DocumentWindow.xaml.cs @@ -0,0 +1,37 @@ +// The fix for the flagship WPF leak: teardown belongs in a teardown. +// +// The subscription is released in `OnClosed` — the method WPF itself calls at +// the end of a window's life — unconditionally, with no parameter to get it +// wrong. Owen treats a `-=` in a real teardown as a provable release, so this +// variant scans clean; at runtime the hub's delegate list is empty once the +// windows are closed. +// +// The whole diff against `bad/` is where the `-=` lives. +using System; +using System.ComponentModel; +using System.Windows; + +namespace Owen.Flagship.Wpf; + +public partial class DocumentWindow : Window +{ + private readonly AppSettings _settings; + + public DocumentWindow(AppSettings settings) + { + InitializeComponent(); + _settings = settings; + _settings.PropertyChanged += OnSettingsChanged; + Render(); + } + + private void OnSettingsChanged(object? sender, PropertyChangedEventArgs e) => Render(); + + private void Render() => ThemeText.Text = $"theme: {_settings.Theme}"; + + protected override void OnClosed(EventArgs e) + { + _settings.PropertyChanged -= OnSettingsChanged; + base.OnClosed(e); + } +} diff --git a/examples/flagship/wpf/ok/Hold.cs b/examples/flagship/wpf/ok/Hold.cs new file mode 100644 index 00000000..2bd563f3 --- /dev/null +++ b/examples/flagship/wpf/ok/Hold.cs @@ -0,0 +1,44 @@ +// Keeping the heap alive for a runtime witness, without depending on stdin. +// +// The console flagship holds on `Console.ReadLine()`, which works because its +// demo script feeds it a FIFO. A CI runner's stdin is NOT a console: ReadLine +// returns null immediately, the process exits, and the witness attaches to +// nothing. So the hold here waits for a file the orchestrator creates, with a +// deadline so a stray sample can never wedge a job. +using System; +using System.IO; +using System.Threading; + +namespace Owen.Flagship.Wpf; + +internal static class Hold +{ + private const int DefaultSeconds = 300; + + /// With OWEN_FLAGSHIP_HOLD=1, print the pid and block until the + /// file named by OWEN_FLAGSHIP_STOP appears (or the deadline passes). + public static void IfAsked() + { + if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; + + string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); + int seconds = int.TryParse( + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 + ? s : DefaultSeconds; + + // The pid line is the orchestration contract: whoever launched this + // process waits for it before attaching. + Console.WriteLine($"holding (pid {Environment.ProcessId}) — " + + (stopFile is null + ? $"exits after {seconds}s." + : $"create {stopFile} to exit, or wait {seconds}s.")); + Console.Out.Flush(); + + DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); + while (DateTime.UtcNow < deadline) + { + if (stopFile != null && File.Exists(stopFile)) return; + Thread.Sleep(200); + } + } +} diff --git a/examples/flagship/wpf/ok/OkDocumentWindows.csproj b/examples/flagship/wpf/ok/OkDocumentWindows.csproj new file mode 100644 index 00000000..93902fd3 --- /dev/null +++ b/examples/flagship/wpf/ok/OkDocumentWindows.csproj @@ -0,0 +1,16 @@ + + + + Exe + net8.0-windows + true + enable + Owen.Flagship.Wpf + + true + + From c3390927afb2465e3b1de055aaad147461b5187a Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 18:46:17 +0000 Subject: [PATCH 2/4] fix(examples): hold the WPF samples with the message loop running, not parked MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The first Windows CI round was green and still wrong, which is the more interesting kind of failure. The bad side reported exactly what it should (200/200 durably retained, [static-event], 100% through one reference), but the FIXED sample reported 200 windows retained via [gc-handle] at 41 hops — after correctly reporting "0 still subscribed". Our release had run; the windows were still pinned. The cause was the sample, not WPF and not the witness. `Window.Close()` finishes through the dispatcher, and the hold parked the UI thread in a Thread.Sleep loop. That froze WPF mid-teardown, so the witness — faithfully — photographed framework book-keeping and called it retention. A runtime witness is only as honest as the moment you take the picture. Both WPF samples now: * measure only after the dispatcher has gone idle (the count and the GC happen in a SystemIdle continuation, not immediately after the loop), and * hold with the message loop STILL RUNNING — a DispatcherTimer polling the stop file instead of a blocking sleep. The console pair keeps its blocking hold: it has no dispatcher, so there is nothing to starve. The ok-side CI check gets the same honesty treatment. It asserts only what the fix claims — no `static-event` root, i.e. the hub retains nothing — and PRINTS whatever else still holds the type rather than asserting it. A green check there means "your subscription is gone", never "the heap is empty"; promoting a framework root to a leak would be the overreach Owen refuses in static analysis, and it should not sneak in through a demo. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M --- .github/workflows/ci.yml | 18 +++++--- examples/flagship/README.md | 19 ++++++-- examples/flagship/wpf/bad/App.xaml.cs | 36 ++++++++++++++-- examples/flagship/wpf/bad/Hold.cs | 62 +++++++++++++++------------ examples/flagship/wpf/ok/App.xaml.cs | 34 ++++++++++++++- examples/flagship/wpf/ok/Hold.cs | 62 +++++++++++++++------------ 6 files changed, 162 insertions(+), 69 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2c04009c..eb714a59 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2549,14 +2549,22 @@ jobs: [ "$WRC" -le 1 ] || { echo "FAIL: witness could not read the process (exit $WRC)"; exit 1; } cat "$RUNNER_TEMP/wpf-ok-runtime.json" python3 - "$RUNNER_TEMP/wpf-ok-runtime.json" <<'PY' - import json, sys + import collections, json, sys doc = json.load(open(sys.argv[1], encoding="utf-8")) roots = (doc.get("retained") or [{}])[0].get("roots") or [] - bad = [r for r in roots if r.get("kind") == "static-event"] - if bad: - print(f"FAIL: the fixed variant still has static-event retention: {bad}", file=sys.stderr) + kinds = collections.Counter(r.get("kind") for r in roots) + if kinds.get("static-event"): + print("FAIL: the fixed variant still has static-event retention " + f"({kinds['static-event']} root(s)) — the release did not run", + file=sys.stderr) sys.exit(1) - print(f"ok: verdict {doc.get('verdict')}, no static-event root") + # Reported, not asserted: anything the FRAMEWORK still holds is a + # different root kind and a different claim. Printing it keeps the + # step honest — a green check here means "our subscription is gone", + # never "the heap is empty". + print(f"ok: no static-event root — the hub retains nothing. " + f"Witness verdict for the window type: {doc.get('verdict')}; " + f"other roots seen: {dict(kinds) or 'none'}") PY - name: "flagship demo orchestrator end-to-end: bad DEMONSTRATED, ok VERIFIED (A3/A4)" if: runner.os == 'Linux' diff --git a/examples/flagship/README.md b/examples/flagship/README.md index bdd34ea1..91d4cb61 100644 --- a/examples/flagship/README.md +++ b/examples/flagship/README.md @@ -80,10 +80,21 @@ is where CI attaches the runtime witness and requires it to name the path: AppSettings → PropertyChanged → _invocationList → handler → DocumentWindow ``` -The `ok` side is held to the claim the fix actually makes — the hub retains -nothing — rather than to "the heap is empty": WPF's own focus and input -statics may legitimately hold the last closed window, and calling that our -leak would be the same overreach Owen refuses everywhere else. +The `ok` side is held to the claim the fix actually makes — **the hub retains +nothing**, checked as the absence of a `static-event` root — rather than to +"the heap is empty". Anything WPF itself still holds surfaces under a +different root kind, and the CI step prints it instead of asserting it: a +green check there means *your* subscription is gone, never that nothing +anywhere references the window. Owen refuses that overreach here for the same +reason it refuses it in static analysis. + +One measurement trap is worth naming, because it cost a CI round. `Close()` +finishes through the dispatcher, so a sample that parks its UI thread to wait +for a witness (`Thread.Sleep`, `Console.ReadLine`) freezes WPF mid-teardown — +and the witness then faithfully reports framework book-keeping as retention. +Both WPF samples therefore hold with the message loop still running +(`DispatcherTimer`), and only count once the dispatcher has gone idle. A +runtime witness is only as honest as the moment you take the picture. ### Holding a sample for a witness diff --git a/examples/flagship/wpf/bad/App.xaml.cs b/examples/flagship/wpf/bad/App.xaml.cs index 8da7fbc8..41dc58c1 100644 --- a/examples/flagship/wpf/bad/App.xaml.cs +++ b/examples/flagship/wpf/bad/App.xaml.cs @@ -33,8 +33,17 @@ private void OpenAndCloseDocuments() window.Close(); } - // Give the GC every chance: whatever survives this is retained by a - // live reference, not by collection lag. + // Measure only after WPF has finished tearing the closed windows down. + // `Close()` completes through the dispatcher, so counting (or holding) + // right here would report the framework mid-teardown rather than the + // steady state — which is exactly what a witness would then see. + Dispatcher.BeginInvoke(new Action(ReportAndHold), DispatcherPriority.SystemIdle); + } + + private void ReportAndHold() + { + // Whatever survives this is retained by a live reference, not by + // collection lag. GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect(); @@ -45,7 +54,26 @@ private void OpenAndCloseDocuments() "every one of them, with its whole visual tree, is retained by the " + "static settings hub."); - Hold.IfAsked(); - Shutdown(); + if (!Hold.Requested) + { + Shutdown(); + return; + } + + // Hold with the message loop STILL RUNNING (see Hold.cs): a parked UI + // thread is indistinguishable, to a witness, from a leak. + Hold.Announce(); + DateTime deadline = DateTime.UtcNow.AddSeconds(Hold.Seconds); + var timer = new DispatcherTimer(DispatcherPriority.Background) + { + Interval = TimeSpan.FromMilliseconds(200), + }; + timer.Tick += (_, _) => + { + if (!Hold.ShouldRelease(deadline)) return; + timer.Stop(); + Shutdown(); + }; + timer.Start(); } } diff --git a/examples/flagship/wpf/bad/Hold.cs b/examples/flagship/wpf/bad/Hold.cs index 2bd563f3..27ede247 100644 --- a/examples/flagship/wpf/bad/Hold.cs +++ b/examples/flagship/wpf/bad/Hold.cs @@ -1,13 +1,19 @@ -// Keeping the heap alive for a runtime witness, without depending on stdin. +// Keeping the heap alive for a runtime witness — WITHOUT blocking the UI +// thread, and without depending on stdin. // -// The console flagship holds on `Console.ReadLine()`, which works because its -// demo script feeds it a FIFO. A CI runner's stdin is NOT a console: ReadLine -// returns null immediately, the process exits, and the witness attaches to -// nothing. So the hold here waits for a file the orchestrator creates, with a -// deadline so a stray sample can never wedge a job. +// Two hazards live here, and the first one bit: +// +// * `Window.Close()` finishes through the dispatcher. A hold that parks the +// UI thread (Thread.Sleep, Console.ReadLine) freezes WPF mid-teardown, and +// a witness attaching then sees framework book-keeping — closed windows +// still pinned by pending dispatcher work — instead of the steady state. +// So the hold is a DispatcherTimer: the loop keeps pumping while we wait. +// * A CI runner's stdin is NOT a console, so a ReadLine hold falls straight +// through and the witness attaches to nothing. The release is a file the +// orchestrator creates, bounded by a deadline so a stray sample can never +// wedge a job. using System; using System.IO; -using System.Threading; namespace Owen.Flagship.Wpf; @@ -15,30 +21,32 @@ internal static class Hold { private const int DefaultSeconds = 300; - /// With OWEN_FLAGSHIP_HOLD=1, print the pid and block until the - /// file named by OWEN_FLAGSHIP_STOP appears (or the deadline passes). - public static void IfAsked() - { - if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; + public static bool Requested => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1"; + + public static string? StopFile => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); - string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); - int seconds = int.TryParse( - Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 - ? s : DefaultSeconds; + public static int Seconds => + int.TryParse(Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), + out int s) && s > 0 ? s : DefaultSeconds; - // The pid line is the orchestration contract: whoever launched this - // process waits for it before attaching. + /// The pid line is the orchestration contract: whoever launched + /// this process waits for it before attaching. + public static void Announce() + { + string? stop = StopFile; Console.WriteLine($"holding (pid {Environment.ProcessId}) — " - + (stopFile is null - ? $"exits after {seconds}s." - : $"create {stopFile} to exit, or wait {seconds}s.")); + + (stop is null + ? $"exits after {Seconds}s." + : $"create {stop} to exit, or wait {Seconds}s.")); Console.Out.Flush(); + } - DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); - while (DateTime.UtcNow < deadline) - { - if (stopFile != null && File.Exists(stopFile)) return; - Thread.Sleep(200); - } + public static bool ShouldRelease(DateTime deadlineUtc) + { + if (DateTime.UtcNow >= deadlineUtc) return true; + string? stop = StopFile; + return stop != null && File.Exists(stop); } } diff --git a/examples/flagship/wpf/ok/App.xaml.cs b/examples/flagship/wpf/ok/App.xaml.cs index ea94a530..8b350a79 100644 --- a/examples/flagship/wpf/ok/App.xaml.cs +++ b/examples/flagship/wpf/ok/App.xaml.cs @@ -34,6 +34,17 @@ private void OpenAndCloseDocuments() window.Close(); } + // Measure only after WPF has finished tearing the closed windows down. + // `Close()` completes through the dispatcher, so counting (or holding) + // right here would report the framework mid-teardown rather than the + // steady state — which is exactly what a witness would then see. + Dispatcher.BeginInvoke(new Action(ReportAndHold), DispatcherPriority.SystemIdle); + } + + private void ReportAndHold() + { + // Whatever survives this is retained by a live reference, not by + // collection lag. GC.Collect(); GC.WaitForPendingFinalizers(); GC.Collect(); @@ -42,7 +53,26 @@ private void OpenAndCloseDocuments() $"opened and closed {Cycles} document windows; " + $"{AppSettings.Instance.SubscriberCount} still subscribed."); - Hold.IfAsked(); - Shutdown(); + if (!Hold.Requested) + { + Shutdown(); + return; + } + + // Hold with the message loop STILL RUNNING (see Hold.cs): a parked UI + // thread is indistinguishable, to a witness, from a leak. + Hold.Announce(); + DateTime deadline = DateTime.UtcNow.AddSeconds(Hold.Seconds); + var timer = new DispatcherTimer(DispatcherPriority.Background) + { + Interval = TimeSpan.FromMilliseconds(200), + }; + timer.Tick += (_, _) => + { + if (!Hold.ShouldRelease(deadline)) return; + timer.Stop(); + Shutdown(); + }; + timer.Start(); } } diff --git a/examples/flagship/wpf/ok/Hold.cs b/examples/flagship/wpf/ok/Hold.cs index 2bd563f3..27ede247 100644 --- a/examples/flagship/wpf/ok/Hold.cs +++ b/examples/flagship/wpf/ok/Hold.cs @@ -1,13 +1,19 @@ -// Keeping the heap alive for a runtime witness, without depending on stdin. +// Keeping the heap alive for a runtime witness — WITHOUT blocking the UI +// thread, and without depending on stdin. // -// The console flagship holds on `Console.ReadLine()`, which works because its -// demo script feeds it a FIFO. A CI runner's stdin is NOT a console: ReadLine -// returns null immediately, the process exits, and the witness attaches to -// nothing. So the hold here waits for a file the orchestrator creates, with a -// deadline so a stray sample can never wedge a job. +// Two hazards live here, and the first one bit: +// +// * `Window.Close()` finishes through the dispatcher. A hold that parks the +// UI thread (Thread.Sleep, Console.ReadLine) freezes WPF mid-teardown, and +// a witness attaching then sees framework book-keeping — closed windows +// still pinned by pending dispatcher work — instead of the steady state. +// So the hold is a DispatcherTimer: the loop keeps pumping while we wait. +// * A CI runner's stdin is NOT a console, so a ReadLine hold falls straight +// through and the witness attaches to nothing. The release is a file the +// orchestrator creates, bounded by a deadline so a stray sample can never +// wedge a job. using System; using System.IO; -using System.Threading; namespace Owen.Flagship.Wpf; @@ -15,30 +21,32 @@ internal static class Hold { private const int DefaultSeconds = 300; - /// With OWEN_FLAGSHIP_HOLD=1, print the pid and block until the - /// file named by OWEN_FLAGSHIP_STOP appears (or the deadline passes). - public static void IfAsked() - { - if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; + public static bool Requested => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1"; + + public static string? StopFile => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); - string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); - int seconds = int.TryParse( - Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 - ? s : DefaultSeconds; + public static int Seconds => + int.TryParse(Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), + out int s) && s > 0 ? s : DefaultSeconds; - // The pid line is the orchestration contract: whoever launched this - // process waits for it before attaching. + /// The pid line is the orchestration contract: whoever launched + /// this process waits for it before attaching. + public static void Announce() + { + string? stop = StopFile; Console.WriteLine($"holding (pid {Environment.ProcessId}) — " - + (stopFile is null - ? $"exits after {seconds}s." - : $"create {stopFile} to exit, or wait {seconds}s.")); + + (stop is null + ? $"exits after {Seconds}s." + : $"create {stop} to exit, or wait {Seconds}s.")); Console.Out.Flush(); + } - DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); - while (DateTime.UtcNow < deadline) - { - if (stopFile != null && File.Exists(stopFile)) return; - Thread.Sleep(200); - } + public static bool ShouldRelease(DateTime deadlineUtc) + { + if (DateTime.UtcNow >= deadlineUtc) return true; + string? stop = StopFile; + return stop != null && File.Exists(stop); } } From 62bb43928eb559539eb21c3abd76db87aa58db1b Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 19:00:48 +0000 Subject: [PATCH 3/4] test(ci): assert the whole claim on the fixed WPF sample, not the part that passed anyway MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With the dispatcher-pump fix in, windows-latest reports what the fix actually achieves: the bad sample is RETAINED via [static-event] with all 200 windows on one reference, and the FIXED sample is ABSENT — every window collected, not one root of any kind. The [gc-handle] retention seen in the previous round was the measurement artifact, not a WPF fact. So the ok-side check is raised to the same user-level contract the console pair already carries: witness exit 0, verdict ABSENT or OBSERVED_ONLY, zero durable roots. Which of the two verdicts appears stays unpinned — that is a GC timing detail, not a public contract — but "something durable holds it" is now a failure instead of a footnote. The previous assertion (no static-event root) was true, narrow, and green while the sample was misbehaving. That is the failure mode worth naming: an assertion scoped tightly enough to survive anything tests nothing. Both directions of the new validator were checked locally — it accepts the real ABSENT artifact and rejects a RETAINED/gc-handle one. README records the correction rather than quietly dropping it: the trap that produced the false reading, why the samples now hold with the message loop running, and that a witness is only as honest as the moment the picture is taken. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M --- .github/workflows/ci.yml | 43 ++++++++++++++++++++----------------- examples/flagship/README.md | 34 ++++++++++++++++------------- 2 files changed, 42 insertions(+), 35 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index eb714a59..3a1b388a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2520,11 +2520,17 @@ jobs: - name: "WPF flagship on Windows: the fixed variant releases the subscription (A2)" if: runner.os == 'Windows' run: | - # The ok side's claim is about OUR subscription, so that is what is - # asserted: the hub holds nothing, and no static-event root reaches a - # closed window. The witness exit code is deliberately NOT the gate - # here — WPF's own focus/input statics may legitimately hold the last - # closed window, which is a framework fact, not our leak. + # Same user-level contract as the console pair: nothing DURABLY + # retained — exit 0, verdict ABSENT or OBSERVED_ONLY, zero durable + # roots. Which of the two verdicts appears is not pinned (that would + # over-specify a GC timing detail); on windows-latest the closed + # windows are collected outright, so it reads ABSENT. + # + # An earlier round asserted only "no static-event root", because the + # fixed sample appeared to keep 200 windows alive through a + # [gc-handle] path. That was this sample parking its UI thread while + # WPF was still tearing the windows down — a measurement artifact, + # not a framework fact — and the weak assertion was hiding it. dotnet build "$GITHUB_WORKSPACE/examples/flagship/wpf/ok" -c Release -v quiet APP="$GITHUB_WORKSPACE/examples/flagship/wpf/ok/bin/Release/net8.0-windows/OkDocumentWindows.dll" WITNESS="$GITHUB_WORKSPACE/audit/runtime/RetentionPath/bin/Release/net8.0/RetentionPath.dll" @@ -2543,28 +2549,25 @@ jobs: WRC=$? set -e touch "$STOP" - # rc 0/1 both mean the witness READ the heap and reached a verdict; - # anything else (2) means it never got to look, which is a failure of - # the step, not a property of the sample. - [ "$WRC" -le 1 ] || { echo "FAIL: witness could not read the process (exit $WRC)"; exit 1; } + [ "$WRC" -eq 0 ] || { echo "FAIL: witness must exit 0 (nothing durably retained) on the fixed sample, got $WRC"; exit 1; } cat "$RUNNER_TEMP/wpf-ok-runtime.json" python3 - "$RUNNER_TEMP/wpf-ok-runtime.json" <<'PY' import collections, json, sys doc = json.load(open(sys.argv[1], encoding="utf-8")) roots = (doc.get("retained") or [{}])[0].get("roots") or [] kinds = collections.Counter(r.get("kind") for r in roots) - if kinds.get("static-event"): - print("FAIL: the fixed variant still has static-event retention " - f"({kinds['static-event']} root(s)) — the release did not run", - file=sys.stderr) + problems = [] + if doc.get("verdict") not in ("ABSENT", "OBSERVED_ONLY"): + problems.append(f"verdict {doc.get('verdict')!r}, want ABSENT or OBSERVED_ONLY") + durable = [k for k in kinds if k not in ("stack", "finalizer")] + if durable: + problems.append(f"durable retainer(s) on the fixed sample: {durable}") + for p in problems: + print(f"FAIL: {p}", file=sys.stderr) + if problems: sys.exit(1) - # Reported, not asserted: anything the FRAMEWORK still holds is a - # different root kind and a different claim. Printing it keeps the - # step honest — a green check here means "our subscription is gone", - # never "the heap is empty". - print(f"ok: no static-event root — the hub retains nothing. " - f"Witness verdict for the window type: {doc.get('verdict')}; " - f"other roots seen: {dict(kinds) or 'none'}") + print(f"ok: nothing durably retains the window — verdict " + f"{doc.get('verdict')}, roots seen: {dict(kinds) or 'none'}") PY - name: "flagship demo orchestrator end-to-end: bad DEMONSTRATED, ok VERIFIED (A3/A4)" if: runner.os == 'Linux' diff --git a/examples/flagship/README.md b/examples/flagship/README.md index 91d4cb61..b0a4fad5 100644 --- a/examples/flagship/README.md +++ b/examples/flagship/README.md @@ -80,21 +80,25 @@ is where CI attaches the runtime witness and requires it to name the path: AppSettings → PropertyChanged → _invocationList → handler → DocumentWindow ``` -The `ok` side is held to the claim the fix actually makes — **the hub retains -nothing**, checked as the absence of a `static-event` root — rather than to -"the heap is empty". Anything WPF itself still holds surfaces under a -different root kind, and the CI step prints it instead of asserting it: a -green check there means *your* subscription is gone, never that nothing -anywhere references the window. Owen refuses that overreach here for the same -reason it refuses it in static analysis. - -One measurement trap is worth naming, because it cost a CI round. `Close()` -finishes through the dispatcher, so a sample that parks its UI thread to wait -for a witness (`Thread.Sleep`, `Console.ReadLine`) freezes WPF mid-teardown — -and the witness then faithfully reports framework book-keeping as retention. -Both WPF samples therefore hold with the message loop still running -(`DispatcherTimer`), and only count once the dispatcher has gone idle. A -runtime witness is only as honest as the moment you take the picture. +The `ok` side is held to the same user-level contract as the console pair: +**nothing durably retains the window** — the witness exits 0, the verdict is +`ABSENT` or `OBSERVED_ONLY`, and there is not one durable root. Which of the +two verdicts appears is deliberately not pinned; on `windows-latest` the +closed windows are collected outright, so it reads `ABSENT`. + +One measurement trap is worth naming, because it cost a CI round and briefly +made WPF look guilty. `Close()` finishes through the dispatcher, so a sample +that parks its UI thread to wait for a witness (`Thread.Sleep`, +`Console.ReadLine`) freezes WPF mid-teardown — and the witness then +faithfully reports framework book-keeping as retention. The fixed sample +appeared to hold 200 windows through a `[gc-handle]` path at 41 hops. It held +none of them: the picture had been taken with the app frozen halfway through +closing. Both WPF samples now hold with the message loop still running +(`DispatcherTimer`) and count only once the dispatcher has gone idle. + +A runtime witness is only as honest as the moment you take the picture — and +an assertion narrow enough to pass regardless ("no `static-event` root") is +how that dishonesty stays green. The check asserts the whole claim now. ### Holding a sample for a witness From 6e32bc8921c134763e4870036dbe3035e4338e76 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 26 Jul 2026 20:55:16 +0000 Subject: [PATCH 4/4] fix(examples): make the hold deadline true on every release path, not just the documented one MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two P2 review findings, both correct, and both the same defect wearing different clothes: the hold PROMISED a bounded lifetime and a stdin release, and delivered each only on one path. * The console helper parsed OWEN_FLAGSHIP_HOLD_SECONDS and then blocked in Console.ReadLine() forever when no stop file was set — the default path scripts/flagship-demo.sh uses. The value was computed and never consulted, while the file header claimed "both paths are bounded by a deadline". * The WPF helper never read stdin at all, so a reader following the documented "send a line to exit" would have waited out the full 300s. Rather than documenting the gaps, the promise is now true. All four samples share one release contract: a line on stdin, the stop file, or the deadline — and the deadline applies to every path. Stdin is read on a BACKGROUND thread, which is what makes the deadline enforceable at all and, in the WPF samples, keeps the dispatcher pumping (a blocking read there would recreate the frozen- teardown artifact this branch just finished fixing). A null read is deliberately NOT a release. With stdin closed or redirected from nothing — every CI runner — Console.ReadLine() returns null immediately, and treating that as "the user pressed Enter" would end the hold before a witness could attach. Verified locally: stdin at EOF with a 3s deadline holds for 3s and exits (neither instantly nor forever); the stop file releases in ~1s with stdin at EOF; the merged demo script still releases through its FIFO promptly (5s end-to-end) and both variants pass; all four samples keep their owen verdicts; full suite and ruff green. Also gives every fenced block in the flagship README a language (MD040, CodeRabbit), including the pre-existing console ones, so the file is consistent rather than half-fixed. Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M --- examples/flagship/README.md | 30 +++++--- examples/flagship/console/bad/Hold.cs | 98 +++++++++++++++++++-------- examples/flagship/console/ok/Hold.cs | 98 +++++++++++++++++++-------- examples/flagship/wpf/bad/Hold.cs | 59 ++++++++++++---- examples/flagship/wpf/ok/Hold.cs | 59 ++++++++++++---- 5 files changed, 248 insertions(+), 96 deletions(-) diff --git a/examples/flagship/README.md b/examples/flagship/README.md index b0a4fad5..4f265f40 100644 --- a/examples/flagship/README.md +++ b/examples/flagship/README.md @@ -13,7 +13,7 @@ an unsubscribe that *exists* but sits behind `if (!keepAlive)` — and every close path calls `Cleanup(keepAlive: true)`. The `-=` never runs; every closed view stays pinned to the publisher forever. -``` +```console dotnet run --project examples/flagship/console/bad → opened and closed 1000 views; 1000 still subscribed — every one of them is retained by the static publisher. @@ -32,7 +32,7 @@ method is not evidence (the corpus pins this predicate family — Move the release where it provably runs: `Dispose()`, unconditionally, called on every close path. -``` +```console dotnet run --project examples/flagship/console/ok → opened and closed 1000 views; 0 still subscribed. @@ -56,7 +56,7 @@ constructor, an unsubscribe behind `Cleanup(keepAlive)`, and a `Closed` handler that passes `true`. Every closed window — its whole visual tree — is retained by the hub's delegate list. -``` +```console dotnet run --project examples/flagship/wpf/bad (Windows) → opened and closed 200 document windows; 200 still subscribed — every one of them, with its whole visual tree, is retained by the static settings hub. @@ -76,7 +76,7 @@ Windows, and the projects themselves compile anywhere (`EnableWindowsTargeting`). Only **running** the sample needs Windows, which is where CI attaches the runtime witness and requires it to name the path: -``` +```text AppSettings → PropertyChanged → _invocationList → handler → DocumentWindow ``` @@ -103,9 +103,19 @@ how that dishonesty stays green. The check asserts the whole claim now. ### Holding a sample for a witness Both pairs support `OWEN_FLAGSHIP_HOLD=1`, which parks the process after the -work is done and prints `holding (pid N)`. Release it by sending a line on -stdin (what `scripts/flagship-demo.sh` does through a FIFO) or, when stdin is -not a console — every CI runner — by setting `OWEN_FLAGSHIP_STOP=` and -creating that file. Either way the hold is bounded by -`OWEN_FLAGSHIP_HOLD_SECONDS` (default 300), so a forgotten sample cannot -outlive its job. +work is done and prints `holding (pid N)`. All four samples honour the same +three release paths: + +| Release | For | +| --- | --- | +| a line on stdin | interactive runs, and `scripts/flagship-demo.sh` through its FIFO | +| `OWEN_FLAGSHIP_STOP=`, then create that file | callers whose stdin is not a console — every CI runner | +| `OWEN_FLAGSHIP_HOLD_SECONDS` (default 300) | the backstop: it applies to *every* path, so a forgotten sample cannot outlive its job | + +Two details are load-bearing rather than incidental. Stdin is read on a +**background** thread — a blocking read would ignore the deadline it claims to +honour, and in the WPF samples it would also starve the dispatcher the hold +depends on. And a **null** read is not a release: with stdin closed or +redirected from nothing, `Console.ReadLine()` returns null immediately, so +treating that as "the user pressed Enter" would end the hold before a witness +could attach — the exact failure the stop file exists to avoid. diff --git a/examples/flagship/console/bad/Hold.cs b/examples/flagship/console/bad/Hold.cs index d749d1a5..7b65400d 100644 --- a/examples/flagship/console/bad/Hold.cs +++ b/examples/flagship/console/bad/Hold.cs @@ -1,11 +1,21 @@ // Keeping the heap alive for a runtime witness. // -// Two release mechanisms, because the two callers differ. `scripts/flagship- -// demo.sh` keeps a FIFO on stdin and releases the sample with a line — the -// default. A CI runner's stdin is NOT a console, where a ReadLine hold would -// fall straight through and the witness would attach to nothing; those callers -// set OWEN_FLAGSHIP_STOP and release the sample by creating that file. Both -// paths are bounded by a deadline, so a stray sample can never wedge a job. +// Three release paths, one deadline, and a null read that means nothing: +// +// * A LINE ON STDIN — what `scripts/flagship-demo.sh` sends through its +// FIFO. Read on a BACKGROUND thread so the deadline below still applies: +// a blocking `Console.ReadLine()` here would wait forever if the writer +// never sends anything, which is exactly the "forgotten sample" this +// helper claims to prevent. +// * THE STOP FILE (OWEN_FLAGSHIP_STOP) — for callers whose stdin is not a +// console. Every CI runner is one of those. +// * THE DEADLINE (OWEN_FLAGSHIP_HOLD_SECONDS, default 300) — applies to +// every path, so a stray sample can never outlive its job. +// +// A NULL read is deliberately not a release: with stdin closed or redirected +// from nothing, `Console.ReadLine()` returns null immediately, and treating +// that as "the user pressed Enter" would end the hold before a witness could +// attach. using System; using System.IO; using System.Threading; @@ -16,34 +26,64 @@ internal static class Hold { private const int DefaultSeconds = 300; + private static volatile bool _lineReceived; + + public static bool Requested => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1"; + + public static string? StopFile => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); + + public static int Seconds => + int.TryParse(Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), + out int s) && s > 0 ? s : DefaultSeconds; + + /// Hold the process until any release path fires. Safe to call + /// unconditionally: it returns at once unless a hold was asked for. public static void IfAsked() { - if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; - - string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); - int seconds = int.TryParse( - Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 - ? s : DefaultSeconds; - - // The pid line is the orchestration contract: whoever launched this - // process waits for it before attaching. - Console.WriteLine($"holding (pid {Environment.ProcessId}) — " - + (stopFile is null - ? "send a line to exit." - : $"create {stopFile} to exit, or wait {seconds}s.")); + if (!Requested) return; + + Announce(); + DateTime deadline = DateTime.UtcNow.AddSeconds(Seconds); + while (!ShouldRelease(deadline)) Thread.Sleep(200); + } + + /// The pid line is the orchestration contract: whoever launched + /// this process waits for it before attaching. + public static void Announce() + { + string? stop = StopFile; + Console.WriteLine($"holding (pid {Environment.ProcessId}) — send a line to exit" + + (stop is null ? $", or wait {Seconds}s." : $", create {stop}, or wait {Seconds}s.")); Console.Out.Flush(); + WatchStdin(); + } - if (stopFile is null) - { - Console.ReadLine(); - return; - } + public static bool ShouldRelease(DateTime deadlineUtc) + { + if (_lineReceived || DateTime.UtcNow >= deadlineUtc) return true; + string? stop = StopFile; + return stop != null && File.Exists(stop); + } - DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); - while (DateTime.UtcNow < deadline) + private static void WatchStdin() + { + var reader = new Thread(() => + { + try + { + if (Console.ReadLine() != null) _lineReceived = true; + } + catch (IOException) + { + // no stdin at all — the stop file and the deadline still apply + } + }) { - if (File.Exists(stopFile)) return; - Thread.Sleep(200); - } + IsBackground = true, // never keeps the process alive by itself + Name = "owen-flagship-stdin", + }; + reader.Start(); } } diff --git a/examples/flagship/console/ok/Hold.cs b/examples/flagship/console/ok/Hold.cs index d749d1a5..7b65400d 100644 --- a/examples/flagship/console/ok/Hold.cs +++ b/examples/flagship/console/ok/Hold.cs @@ -1,11 +1,21 @@ // Keeping the heap alive for a runtime witness. // -// Two release mechanisms, because the two callers differ. `scripts/flagship- -// demo.sh` keeps a FIFO on stdin and releases the sample with a line — the -// default. A CI runner's stdin is NOT a console, where a ReadLine hold would -// fall straight through and the witness would attach to nothing; those callers -// set OWEN_FLAGSHIP_STOP and release the sample by creating that file. Both -// paths are bounded by a deadline, so a stray sample can never wedge a job. +// Three release paths, one deadline, and a null read that means nothing: +// +// * A LINE ON STDIN — what `scripts/flagship-demo.sh` sends through its +// FIFO. Read on a BACKGROUND thread so the deadline below still applies: +// a blocking `Console.ReadLine()` here would wait forever if the writer +// never sends anything, which is exactly the "forgotten sample" this +// helper claims to prevent. +// * THE STOP FILE (OWEN_FLAGSHIP_STOP) — for callers whose stdin is not a +// console. Every CI runner is one of those. +// * THE DEADLINE (OWEN_FLAGSHIP_HOLD_SECONDS, default 300) — applies to +// every path, so a stray sample can never outlive its job. +// +// A NULL read is deliberately not a release: with stdin closed or redirected +// from nothing, `Console.ReadLine()` returns null immediately, and treating +// that as "the user pressed Enter" would end the hold before a witness could +// attach. using System; using System.IO; using System.Threading; @@ -16,34 +26,64 @@ internal static class Hold { private const int DefaultSeconds = 300; + private static volatile bool _lineReceived; + + public static bool Requested => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1"; + + public static string? StopFile => + Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); + + public static int Seconds => + int.TryParse(Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), + out int s) && s > 0 ? s : DefaultSeconds; + + /// Hold the process until any release path fires. Safe to call + /// unconditionally: it returns at once unless a hold was asked for. public static void IfAsked() { - if (Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") != "1") return; - - string? stopFile = Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_STOP"); - int seconds = int.TryParse( - Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 - ? s : DefaultSeconds; - - // The pid line is the orchestration contract: whoever launched this - // process waits for it before attaching. - Console.WriteLine($"holding (pid {Environment.ProcessId}) — " - + (stopFile is null - ? "send a line to exit." - : $"create {stopFile} to exit, or wait {seconds}s.")); + if (!Requested) return; + + Announce(); + DateTime deadline = DateTime.UtcNow.AddSeconds(Seconds); + while (!ShouldRelease(deadline)) Thread.Sleep(200); + } + + /// The pid line is the orchestration contract: whoever launched + /// this process waits for it before attaching. + public static void Announce() + { + string? stop = StopFile; + Console.WriteLine($"holding (pid {Environment.ProcessId}) — send a line to exit" + + (stop is null ? $", or wait {Seconds}s." : $", create {stop}, or wait {Seconds}s.")); Console.Out.Flush(); + WatchStdin(); + } - if (stopFile is null) - { - Console.ReadLine(); - return; - } + public static bool ShouldRelease(DateTime deadlineUtc) + { + if (_lineReceived || DateTime.UtcNow >= deadlineUtc) return true; + string? stop = StopFile; + return stop != null && File.Exists(stop); + } - DateTime deadline = DateTime.UtcNow.AddSeconds(seconds); - while (DateTime.UtcNow < deadline) + private static void WatchStdin() + { + var reader = new Thread(() => + { + try + { + if (Console.ReadLine() != null) _lineReceived = true; + } + catch (IOException) + { + // no stdin at all — the stop file and the deadline still apply + } + }) { - if (File.Exists(stopFile)) return; - Thread.Sleep(200); - } + IsBackground = true, // never keeps the process alive by itself + Name = "owen-flagship-stdin", + }; + reader.Start(); } } diff --git a/examples/flagship/wpf/bad/Hold.cs b/examples/flagship/wpf/bad/Hold.cs index 27ede247..37599aec 100644 --- a/examples/flagship/wpf/bad/Hold.cs +++ b/examples/flagship/wpf/bad/Hold.cs @@ -1,19 +1,23 @@ // Keeping the heap alive for a runtime witness — WITHOUT blocking the UI -// thread, and without depending on stdin. +// thread, and without trusting stdin to exist. // -// Two hazards live here, and the first one bit: +// Three hazards live here, and the first one bit: // // * `Window.Close()` finishes through the dispatcher. A hold that parks the // UI thread (Thread.Sleep, Console.ReadLine) freezes WPF mid-teardown, and // a witness attaching then sees framework book-keeping — closed windows // still pinned by pending dispatcher work — instead of the steady state. -// So the hold is a DispatcherTimer: the loop keeps pumping while we wait. -// * A CI runner's stdin is NOT a console, so a ReadLine hold falls straight -// through and the witness attaches to nothing. The release is a file the -// orchestrator creates, bounded by a deadline so a stray sample can never -// wedge a job. +// So the release is polled from a DispatcherTimer and stdin is read on a +// BACKGROUND thread: the message loop never stops pumping. +// * A CI runner's stdin is NOT a console. `Console.ReadLine()` returns null +// there immediately, so a null read is deliberately NOT a release signal — +// otherwise the sample would exit before the witness could attach. Only an +// actual line counts. +// * Any hold can be forgotten. The deadline applies to every release path, +// so a stray sample can never outlive its job. using System; using System.IO; +using System.Threading; namespace Owen.Flagship.Wpf; @@ -21,6 +25,8 @@ internal static class Hold { private const int DefaultSeconds = 300; + private static volatile bool _lineReceived; + public static bool Requested => Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1"; @@ -31,22 +37,47 @@ internal static class Hold int.TryParse(Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 ? s : DefaultSeconds; - /// The pid line is the orchestration contract: whoever launched - /// this process waits for it before attaching. + /// Print the pid line — the orchestration contract: whoever + /// launched this process waits for it before attaching — and start + /// watching stdin for the interactive release. public static void Announce() { string? stop = StopFile; - Console.WriteLine($"holding (pid {Environment.ProcessId}) — " - + (stop is null - ? $"exits after {Seconds}s." - : $"create {stop} to exit, or wait {Seconds}s.")); + Console.WriteLine($"holding (pid {Environment.ProcessId}) — send a line to exit" + + (stop is null ? $", or wait {Seconds}s." : $", create {stop}, or wait {Seconds}s.")); Console.Out.Flush(); + WatchStdin(); } + /// True once ANY release path has fired: a line on stdin, the stop + /// file, or the deadline. public static bool ShouldRelease(DateTime deadlineUtc) { - if (DateTime.UtcNow >= deadlineUtc) return true; + if (_lineReceived || DateTime.UtcNow >= deadlineUtc) return true; string? stop = StopFile; return stop != null && File.Exists(stop); } + + private static void WatchStdin() + { + var reader = new Thread(() => + { + try + { + // null = stdin is closed or redirected from nothing (every CI + // runner). That is not a release; the deadline and the stop + // file cover those callers. + if (Console.ReadLine() != null) _lineReceived = true; + } + catch (IOException) + { + // no stdin at all — same story + } + }) + { + IsBackground = true, // never keeps the process alive by itself + Name = "owen-flagship-stdin", + }; + reader.Start(); + } } diff --git a/examples/flagship/wpf/ok/Hold.cs b/examples/flagship/wpf/ok/Hold.cs index 27ede247..37599aec 100644 --- a/examples/flagship/wpf/ok/Hold.cs +++ b/examples/flagship/wpf/ok/Hold.cs @@ -1,19 +1,23 @@ // Keeping the heap alive for a runtime witness — WITHOUT blocking the UI -// thread, and without depending on stdin. +// thread, and without trusting stdin to exist. // -// Two hazards live here, and the first one bit: +// Three hazards live here, and the first one bit: // // * `Window.Close()` finishes through the dispatcher. A hold that parks the // UI thread (Thread.Sleep, Console.ReadLine) freezes WPF mid-teardown, and // a witness attaching then sees framework book-keeping — closed windows // still pinned by pending dispatcher work — instead of the steady state. -// So the hold is a DispatcherTimer: the loop keeps pumping while we wait. -// * A CI runner's stdin is NOT a console, so a ReadLine hold falls straight -// through and the witness attaches to nothing. The release is a file the -// orchestrator creates, bounded by a deadline so a stray sample can never -// wedge a job. +// So the release is polled from a DispatcherTimer and stdin is read on a +// BACKGROUND thread: the message loop never stops pumping. +// * A CI runner's stdin is NOT a console. `Console.ReadLine()` returns null +// there immediately, so a null read is deliberately NOT a release signal — +// otherwise the sample would exit before the witness could attach. Only an +// actual line counts. +// * Any hold can be forgotten. The deadline applies to every release path, +// so a stray sample can never outlive its job. using System; using System.IO; +using System.Threading; namespace Owen.Flagship.Wpf; @@ -21,6 +25,8 @@ internal static class Hold { private const int DefaultSeconds = 300; + private static volatile bool _lineReceived; + public static bool Requested => Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD") == "1"; @@ -31,22 +37,47 @@ internal static class Hold int.TryParse(Environment.GetEnvironmentVariable("OWEN_FLAGSHIP_HOLD_SECONDS"), out int s) && s > 0 ? s : DefaultSeconds; - /// The pid line is the orchestration contract: whoever launched - /// this process waits for it before attaching. + /// Print the pid line — the orchestration contract: whoever + /// launched this process waits for it before attaching — and start + /// watching stdin for the interactive release. public static void Announce() { string? stop = StopFile; - Console.WriteLine($"holding (pid {Environment.ProcessId}) — " - + (stop is null - ? $"exits after {Seconds}s." - : $"create {stop} to exit, or wait {Seconds}s.")); + Console.WriteLine($"holding (pid {Environment.ProcessId}) — send a line to exit" + + (stop is null ? $", or wait {Seconds}s." : $", create {stop}, or wait {Seconds}s.")); Console.Out.Flush(); + WatchStdin(); } + /// True once ANY release path has fired: a line on stdin, the stop + /// file, or the deadline. public static bool ShouldRelease(DateTime deadlineUtc) { - if (DateTime.UtcNow >= deadlineUtc) return true; + if (_lineReceived || DateTime.UtcNow >= deadlineUtc) return true; string? stop = StopFile; return stop != null && File.Exists(stop); } + + private static void WatchStdin() + { + var reader = new Thread(() => + { + try + { + // null = stdin is closed or redirected from nothing (every CI + // runner). That is not a release; the deadline and the stop + // file cover those callers. + if (Console.ReadLine() != null) _lineReceived = true; + } + catch (IOException) + { + // no stdin at all — same story + } + }) + { + IsBackground = true, // never keeps the process alive by itself + Name = "owen-flagship-stdin", + }; + reader.Start(); + } }