fix(extractor): WPF002 Stop() teardown soundness — one doctrine with -= - #302
Merged
Conversation
The stopped set credits ANY <recv>.Stop() anywhere in the class — existence as proof of execution, the exact false-negative class #278 closed for -=. Reproduced on real C# through the extractor + core (DispatcherTimer stand-in — not IDisposable, so Stop() IS the release and WPF003 stays out of the way): six before.cs are falsely SILENT today (timer fact stamped released: true by a Stop() that nothing proves runs): * timer-stop-nonteardown-release — Stop() only in an arbitrary method * timer-stop-finalizer-release — Stop() only in the finalizer * timer-stop-unwired-lifecycle — Stop() in an unwired Window_Closing * timer-stop-uncalled-helper — Stop() in a helper no teardown calls * timer-stop-uncalled-lambda — Stop() in a stored, never-invoked lambda * timer-stop-param-guarded — Stop() behind a caller-parameter guard Each after.cs is the positive twin (Dispose / platform teardown / wired handler / transitively-called helper / wired lambda / field guard) and is silent both before and after the fix. The seventh case pins what already holds: timer-stop-wrong-receiver — a sibling's Stop() must not release the checked timer (receiver identity; flagged today, stays flagged). case.own reductions extend the wpf suite to 24/24; the P-022 parity fixtures (cfg 83 / diag 84 cases) are regenerated for the new corpus and the frozen-corpus Rust parity test stays green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK
📝 WalkthroughWalkthroughThe extractor now credits timer ChangesWPF timer Stop ownership
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant RoslynSyntax
participant TeardownPredicates
participant TimerAnalysis
RoslynSyntax->>TeardownPredicates: inspect Stop invocation
TeardownPredicates->>TeardownPredicates: verify teardown context and guard status
TeardownPredicates->>TimerAnalysis: provide eligible receiver
TimerAnalysis->>TimerAnalysis: record stopped timer or retain OWN001
Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…ctrine One predicate, one context model: InTeardownContext and IsParamGuardedRelease generalize from AssignmentExpressionSyntax to SyntaxNode (they only ever walked ancestors), and the stopped set now admits a receiver only when its .Stop() invocation passes BOTH — the exact gate the -= release has had since #278. A Stop() in an arbitrary method, a finalizer, an unwired lifecycle-looking handler, an uncalled helper or lambda, or behind a caller-parameter guard credits nothing; the timer fact keeps released: false and the honest OWN001 [resource: timer] stands. Receiver identity is unchanged: a sibling's Stop() never releases the checked timer. Verified on real C# through the extractor + core: all 7 red before.cs now flag OWN001 (released: false), all 7 after.cs twins stay silent (released: true); the extractor-S2 samples behave identically (TimerViewModel flagged / CleanTimerViewModel credited); full corpus benchmark 57/58 caught, 58/58 fixes clean, 0 false positives (the one miss is the documented pre-existing injected-source region-escape backlog case, which contains no Stop at all); build clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK
subscription-leaks-and-profiles.md records the extended doctrine: the WPF002 row's Stop() release uses the SAME teardown/guard context predicate as -=, with the seven timer-stop corpus cases listed beside the #278 subscription set. The extractor-header wording fix rode the green commit (it sits in Program.cs); tcplistener-stop-release.md is deliberately untouched — the flow-local TcpListener.Stop() release is a different mechanism, outside this slice. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK
PhysShell
force-pushed
the
claude/wpf002-stop-teardown
branch
from
July 19, 2026 03:01
6309312 to
c1d4fe6
Compare
Owner
Author
|
Независимый review — финальный verdict на head
Merge авторизован при post-draft гейтах (CodeRabbit/Codex review завершены; head неизменен; CI зелёный; mergeable clean; нет unresolved blocking threads). Косметический nit («Does this Generated by Claude Code |
PhysShell
marked this pull request as ready for review
July 19, 2026 03:15
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c1d4fe6f49
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🧹 Nitpick comments (1)
frontend/roslyn/OwnSharp.Extractor/Program.cs (1)
5196-5208: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winHandle conditional access for
.Stop()invocations.The current check strictly matches
MemberAccessExpressionSyntax, which misses idiomatic conditional-access teardowns like_timer?.Stop(). In a conditional access tree, the invocation's expression is aMemberBindingExpressionSyntax(.Stop), and the receiver is theExpressionof the enclosingConditionalAccessExpressionSyntax.Consider expanding this logic to capture both patterns, as
_timer?.Stop()is a common, safe cleanup practice that will currently be ignored, resulting in a false positive (unreleased timer).🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/roslyn/OwnSharp.Extractor/Program.cs` around lines 5196 - 5208, Update the stopped-timer detection loop around stopped and InTeardownContext to recognize both direct MemberAccessExpressionSyntax calls and conditional-access invocations whose MemberBindingExpressionSyntax is Stop. For conditional access, use the enclosing ConditionalAccessExpressionSyntax.Expression as the receiver when recording the stopped timer, while preserving the existing teardown and parameter-guard checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/roslyn/OwnSharp.Extractor/Program.cs`:
- Around line 5196-5208: Update the stopped-timer detection loop around stopped
and InTeardownContext to recognize both direct MemberAccessExpressionSyntax
calls and conditional-access invocations whose MemberBindingExpressionSyntax is
Stop. For conditional access, use the enclosing
ConditionalAccessExpressionSyntax.Expression as the receiver when recording the
stopped timer, while preserving the existing teardown and parameter-guard
checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d4c638e8-31be-4f0f-a03b-1a206f84b3a7
📒 Files selected for processing (39)
corpus/wpf/timer-stop-finalizer-release/after.cscorpus/wpf/timer-stop-finalizer-release/before.cscorpus/wpf/timer-stop-finalizer-release/case.owncorpus/wpf/timer-stop-finalizer-release/expected-diagnostics.txtcorpus/wpf/timer-stop-finalizer-release/notes.mdcorpus/wpf/timer-stop-nonteardown-release/after.cscorpus/wpf/timer-stop-nonteardown-release/before.cscorpus/wpf/timer-stop-nonteardown-release/case.owncorpus/wpf/timer-stop-nonteardown-release/expected-diagnostics.txtcorpus/wpf/timer-stop-nonteardown-release/notes.mdcorpus/wpf/timer-stop-param-guarded/after.cscorpus/wpf/timer-stop-param-guarded/before.cscorpus/wpf/timer-stop-param-guarded/case.owncorpus/wpf/timer-stop-param-guarded/expected-diagnostics.txtcorpus/wpf/timer-stop-param-guarded/notes.mdcorpus/wpf/timer-stop-uncalled-helper/after.cscorpus/wpf/timer-stop-uncalled-helper/before.cscorpus/wpf/timer-stop-uncalled-helper/case.owncorpus/wpf/timer-stop-uncalled-helper/expected-diagnostics.txtcorpus/wpf/timer-stop-uncalled-helper/notes.mdcorpus/wpf/timer-stop-uncalled-lambda/after.cscorpus/wpf/timer-stop-uncalled-lambda/before.cscorpus/wpf/timer-stop-uncalled-lambda/case.owncorpus/wpf/timer-stop-uncalled-lambda/expected-diagnostics.txtcorpus/wpf/timer-stop-uncalled-lambda/notes.mdcorpus/wpf/timer-stop-unwired-lifecycle/after.cscorpus/wpf/timer-stop-unwired-lifecycle/before.cscorpus/wpf/timer-stop-unwired-lifecycle/case.owncorpus/wpf/timer-stop-unwired-lifecycle/expected-diagnostics.txtcorpus/wpf/timer-stop-unwired-lifecycle/notes.mdcorpus/wpf/timer-stop-wrong-receiver/after.cscorpus/wpf/timer-stop-wrong-receiver/before.cscorpus/wpf/timer-stop-wrong-receiver/case.owncorpus/wpf/timer-stop-wrong-receiver/expected-diagnostics.txtcorpus/wpf/timer-stop-wrong-receiver/notes.mddocs/notes/subscription-leaks-and-profiles.mdfrontend/roslyn/OwnSharp.Extractor/Program.cstests/fixtures/cfg_parity.jsontests/fixtures/diag_parity.json
PhysShell
pushed a commit
that referenced
this pull request
Jul 25, 2026
…, enrollment split Four blockers plus the smaller fixes from the architecture review: 1. Migration plan no longer describes merged work as future. #258 is closed (spec/Bridge.md is the merged normative contract); #278 is closed by #293 and extended to WPF002 Stop() by #302 — those landed extractor predicates are named the current bounded implementation and the regression floor. Phase 2 is retargeted to the new post-cutover tracker #304 (summary-backed lifecycle release reachability); #278 stays the historical motivating incident, not a reusable implementation issue. 2. MVP guarded-effects policy defined: summaries preserve guards over simple boolean/null parameter predicates, callsite application substitutes statically-known constants, anything outside that vocabulary degrades to May/Unknown — never Must, never silence. Without this the summary layer loses to the landed predicates on the flagship Teardown(bool) case. input_contract semantics defined in the summary envelope. 3. Lifecycle reasoning split into two theorems: LifecycleEffect (release happens IF the root runs) vs LifecycleEnrollment (this instance provably reaches that root). Effect without enrollment is degraded/conditional, never clean — a perfect Dispose() nobody calls proves nothing. 4. Bridge-boundary authority table added: until parity+cutover #258/#259 own the boundary (MOS in own-bridge, byte-parity); after cutover a dedicated extraction slice per this proposal; wire schema, verdicts, and parity artifacts invariant across both. Also: OwnCFG claim corrected to intended-MIR-equivalent (today: plain succ edges, calls as instructions, AST re-export); Call-instruction vs Invoke-terminator model made explicit; evidence split into a single proof DAG vs a per-finding displayed witness. Refs #303 review; tracker: #304. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
PhysShell
pushed a commit
that referenced
this pull request
Jul 25, 2026
…cates Adversarial code reading of the landed teardown-context/guard predicates (enumerate the implicit axioms, attack each, verify against Program.cs). Full attack matrix — six confirmed hole families, eleven survived attacks, doctrine assessment, bounded fix directions — in docs/notes/teardown-predicate-adversarial-audit.md. Two P1 holes (silently swallowed leaks, the #238 doctrine violation) are pinned as red corpus fixtures; the CI corpus benchmark is the empirical arbiter (before.cs MISSED expected — recall floor is an absolute count; after.cs reuses proven-silent shapes): - subscription-teardown-early-return-guard: the SectorTS flag guard rewritten from `if (!flag) { -= }` to `if (flag) return; -=` — semantically identical, invisible to IsParamGuardedRelease (ancestors-only walk), while the symbol closure credits the helper regardless of argument values. The C# twin of the bridge's D7/INF-S3 defect. - subscription-disposing-else-branch-release: a `-=` in the ELSE of the canonical `if (disposing)` is credited (the exception classifies the parameter's use in the condition, never the branch holding the site) — yet it runs only on the finalizer path the extractor's own doctrine declares unreachable while the subscription is live. Both .own reductions are caught by the branch-sensitive core (wpf corpus 26/26) — extractor gaps, not core gaps. cfg/diag parity fixtures regenerated (additive); Rust parity green on the grown corpus. WPF002 Stop() shares the predicate, so the holes apply verbatim — timer twins land with the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015de4MezSeUnZBoWq1fFU5M
This was referenced Jul 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Что и зачем
Закрытие WPF002
Stop()-дыры — того же класса false negative, который #278 закрыл для-=: существование cleanup не является доказательством его исполнения. До этого PRstopped-сет кредитовал ЛЮБОЙ<recv>.Stop()где угодно в классе; таймеру достаточно былоStop()в произвольном методе, финализаторе, невайренномWindow_Closing, невызванном helper'е/lambda или за caller-parameter guard'ом, чтобы замолчать.Новый контракт: Stop existed AND targets the matching timer receiver AND executes in a proven teardown context AND is not caller-parameter-guarded → released. Реализация — не вторая копия модели, а буквально общий предикат:
InTeardownContextиIsParamGuardedReleaseобобщены сAssignmentExpressionSyntaxдоSyntaxNode(оба и так только ходили вверх по предкам), и сборstoppedгейтится ими же — со всей доктриной #278: точные teardown-roots (Dispose/DisposeAsync/OnClosed/…), code-wired lifecycle handlers, symbol-resolved транзитивные helper'ы, исключения finalizer/unwired-name/uncalled local-fn/lambda/param-guard (с каноничнымif (disposing)). Receiver identity не менялся: чужойStop()по-прежнему не освобождает проверяемый таймер.История (три ступени): red
8f936f5(7 corpus-фикстур + перегенерация P-022 parity) → green07ac2cf(общий предикат + gating + комментарии Program.cs) → docsc1d4fe6(subscription-leaks-and-profiles.md).Фикстуры на DispatcherTimer stand-in (in-file, как в samples/SampleTypes.cs): он не IDisposable —
Stop()и есть release (сам паттерн WPF002), и WPF003 (disposable-field) не контаминирует specificity-половину benchmark.Тип изменения
Как проверено
released: trueот постороннего Stop:timer-stop-nonteardown-release(произвольный метод) ·timer-stop-finalizer-release·timer-stop-unwired-lifecycle(Window_Closing без wiring) ·timer-stop-uncalled-helper·timer-stop-uncalled-lambda·timer-stop-param-guarded; седьмой —timer-stop-wrong-receiver— пин уже честного receiver-matchingreleased: false; 7/7 after (positive twins: Dispose / OnClosed / wired handler / транзитивный helper из Dispose / wired lambda / field-guard) — silent,released: trueviewmodel-escapes-to-app, документированный pre-existing backlog (injected-source region-escape, в файле нет ни одногоStop, диффом не затрагивается); других corpus-дельт вне добавленных timer-stop кейсов нет-=-матрица Soundness: OWN001 treats any-=in the class as a release — a-=behind a flag, or in a method nobody calls, silently swallows a real leak (heap-proven on SectorTS) #278 без изменения поведения: все subscription-* кейсы caught/clean как раньше; extractor-S2 samples идентичны (TimerViewModel flagged / CleanTimerViewModel credited)released: false(проверено по JSON-фактам)Связанные issue
Продолжение линии #278 (та же teardown-доктрина, перенесённая на второй release-механизм WPF002). Refs #238 (доктрина «худший случай exemption — честное предупреждение»).
Чеклист
Draft — не мержить, не self-accept. Гейт: независимый review — подтверждение, что (1) предикат действительно один (не копия), (2) все 6 негативных контекстов закрыты и позитивные twins не потеряны, (3) receiver identity не переопределён, (4)
-=-матрица не сдвинута, (5) вне scope ничего не протащено (XAML-wiring, call graph, CFG-teardown, схема OwnIR, Python core, Rust crates).🤖 Generated with Claude Code
https://claude.ai/code/session_01MdYFKUaygHz1T9H1qJ7BqK
Summary by CodeRabbit
Bug Fixes
Documentation
Tests