diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 51c60b0b..3a1b388a 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,109 @@ 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: | + # 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" + 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" + [ "$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) + 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) + 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' run: | diff --git a/examples/flagship/README.md b/examples/flagship/README.md index 994dec86..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. @@ -47,3 +47,75 @@ 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. + +```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. + +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: + +```text +AppSettings → PropertyChanged → _invocationList → handler → DocumentWindow +``` + +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 + +Both pairs support `OWEN_FLAGSHIP_HOLD=1`, which parks the process after the +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/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..7b65400d --- /dev/null +++ b/examples/flagship/console/bad/Hold.cs @@ -0,0 +1,89 @@ +// Keeping the heap alive for a runtime witness. +// +// 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; + +namespace Owen.Flagship; + +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 (!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(); + } + + public static bool ShouldRelease(DateTime deadlineUtc) + { + 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 + { + if (Console.ReadLine() != null) _lineReceived = true; + } + catch (IOException) + { + // no stdin at all — the stop file and the deadline still apply + } + }) + { + IsBackground = true, // never keeps the process alive by itself + Name = "owen-flagship-stdin", + }; + reader.Start(); + } +} 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..7b65400d --- /dev/null +++ b/examples/flagship/console/ok/Hold.cs @@ -0,0 +1,89 @@ +// Keeping the heap alive for a runtime witness. +// +// 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; + +namespace Owen.Flagship; + +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 (!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(); + } + + public static bool ShouldRelease(DateTime deadlineUtc) + { + 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 + { + if (Console.ReadLine() != null) _lineReceived = true; + } + catch (IOException) + { + // no stdin at all — the stop file and the deadline still apply + } + }) + { + IsBackground = true, // never keeps the process alive by itself + Name = "owen-flagship-stdin", + }; + reader.Start(); + } +} 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..41dc58c1 --- /dev/null +++ b/examples/flagship/wpf/bad/App.xaml.cs @@ -0,0 +1,79 @@ +// 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(); + } + + // 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(); + + 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."); + + 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/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..37599aec --- /dev/null +++ b/examples/flagship/wpf/bad/Hold.cs @@ -0,0 +1,83 @@ +// Keeping the heap alive for a runtime witness — WITHOUT blocking the UI +// thread, and without trusting stdin to exist. +// +// 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 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; + +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; + + /// 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}) — 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 (_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/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..8b350a79 --- /dev/null +++ b/examples/flagship/wpf/ok/App.xaml.cs @@ -0,0 +1,78 @@ +// 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(); + } + + // 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(); + + Console.WriteLine( + $"opened and closed {Cycles} document windows; " + + $"{AppSettings.Instance.SubscriberCount} still subscribed."); + + 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/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..37599aec --- /dev/null +++ b/examples/flagship/wpf/ok/Hold.cs @@ -0,0 +1,83 @@ +// Keeping the heap alive for a runtime witness — WITHOUT blocking the UI +// thread, and without trusting stdin to exist. +// +// 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 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; + +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; + + /// 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}) — 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 (_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/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 + +