corpus(wpf): a -= that exists is not a -= that runs — regression guard for #278#279
corpus(wpf): a -= that exists is not a -= that runs — regression guard for #278#279PhysShell wants to merge 1 commit into
-= that exists is not a -= that runs — regression guard for #278#279Conversation
A regression guard for the release-reachability soundness gap: the subscription is to a STATIC publisher, a matching `-=` does exist in the class — so the extractor's "any matching `-=` in the class releases it" model falls silent — but it never runs. It sits behind a bool parameter every caller passes as `true`, in a method that is not a teardown, and that a whole subsystem never calls at all. before.cs is RED today: it is a real leak and own-check says nothing. That is the point of the case. after.cs must stay silent (the release is unconditional and in Dispose), so the fix cannot trade the false negative for a false positive. The .own reduction shows the core already handles this correctly — it reports OWN001 "not released before return (leaks on at least one path)" and points at the guarded branch. The bug is upstream, in the extractor's release-matching. Reduced from SectorTS GTD.cs:5192/:5259 and proven on the heap with a ClrMD root walk: 66.3% of the heap still reachable from the GC roots after 31 documents, held via [PinnedHandle] -> static KernelProperty -> GBProperty -> PropertyChangedEventHandler -> GTD. Detaching after each document makes the process memory-flat (peak RSS 2.71 GB -> 0.61 GB, byte-identical output), which confirms the diagnosis. Prior art: corpus/wpf/subscription-explicit-delegate-release/notes.md:28-41 already records the model as not flow-sensitive, but scoped it to a rebinding setter and deferred it. This case shows the surface is wider and gives it a real instance. CI: recall floor is unaffected (--min-recall is an absolute floor of caught cases, and this one is not caught yet); after.cs adds no false positive. The cfg/diag parity fixtures are regenerated because the corpus grew; tests/run_tests.py and cargo test are both green. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PjQnE1FDucd6vBVQbFswiE
|
Warning Review limit reached
Next review available in: 10 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✨ Finishing Touches🧪 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 |
![chatgpt-codex-connector[bot]](https://cdn.github.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f0b068f5d
ℹ️ 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".
| **Regression guard.** `scripts/benchmark.py` runs the real C# through the extractor + core: | ||
|
|
||
| * `before.cs` must be **caught** (OWN001) — it is a genuine leak. *Currently it is not: this is the bug.* | ||
| * `after.cs` must be **silent** — the release is unconditional and in `Dispose`. It must stay silent |
There was a problem hiding this comment.
When relying on this as the #278 regression guard, scripts/benchmark.py does not actually require this before.cs to be caught: it only gates the aggregate caught count against --min-recall 25, and this commit explicitly leaves that floor unchanged, while tests/test_wpf.py only checks the hand-written .own reduction that already passes. Because before.cs is known silent today, CI can stay green with this exact false negative indefinitely; add an explicit per-case expected-fail/ratchet or otherwise make the benchmark fail once this case is still missed.
Useful? React with 👍 / 👎.
|
@coderabbitai review |
1 participant
Adds
corpus/wpf/unsubscribe-behind-a-flag/, the regression guard #278 asks for.What the case pins
A document subscribes to a static publisher in its constructor. A matching
-=does exist in the class — so the extractor's release model ("released by a matchingtarget -= handlerin the class",OwnSharp.Extractor/Program.cs:13) falls silent — but it never runs:Three independent reasons the release is unreachable, any one of which is enough: it is not in a teardown (P-001:51 and P-004:33 both require
Dispose/OnClosed/Unloaded— the extractor is looser than its own spec); it is behind a parameter guard callers pass astrue; and whole subsystems never call the method.before.csis RED today — that is the pointbefore.csis a genuine leak and own-check reports nothing.after.csmoves the-=intoDisposeand drops the flag, and must stay silent — so the fix cannot trade the false negative for a false positive. The two files differ only in whether the release is provably reached; the+=and-=name the same(receiver, handler)in both. A model that keys on the mere existence of a matching-=cannot tell them apart.The core already gets this right
The
.ownreduction is worth reading — it models the flag as the branch that returns before the release, and the checker nails it:So this is not a core-logic gap. It is upstream, in the extractor emitting
released: truefrom the existence of a-=without asking whether it runs.Provenance — heap-proven, not hypothetical
Reduced from SectorTS
BrokerDataClasses/GTD.cs:5192(subscribe to the staticAppData.Properties.GBProperty) and:5259(UnregisterEventHandlers(bool UnregOnlyGoodys)).GTDService.cspassestrueat five sites;DocCloud/**— including eight AutoMapper.ConstructUsing(x => new GTD(null, null))profiles that build a document per mapping — never calls it at all.A ClrMD root walk on a live process, after 31 documents:
Detaching after each document makes the process memory-flat: peak RSS 2.71 GB → 0.61 GB on the same 389 documents, with byte-identical output. That confirms the diagnosis rather than merely being consistent with it.
Note that OWN001 does fire on
KDT.cs:88— the same static publisher, no-=anywhere. It is only the presence of an unreachable-=that silences it.Prior art, honestly
corpus/wpf/subscription-explicit-delegate-release/notes.md:28-41(Codex P2 on #163) already records that the release model is not flow-sensitive and calls the gap pre-existing. But it scoped the gap to a rebinding setter, deferred it, and never filed it. This case shows the surface is much wider — a parameter guard, a non-teardown method, and an uncalled method are all ordinary code — and gives the gap its first real instance.CI impact
--min-recall 25is an absolute floor of caught cases; this case is not caught yet, socaughtdoes not move.after.csis silent under the current model and must stay so.cfg_parity.json,diag_parity.json) — the corpus grew by one case.python tests/run_tests.py→ exit 0,wpf: 10/10 cases match expected codes.cd rust && cargo test→ exit 0.python scripts/benchmark.py --selftest→ OK.Closes nothing on its own — it is the red test that #278's fix turns green.
🤖 Generated with Claude Code
https://claude.ai/code/session_01PjQnE1FDucd6vBVQbFswiE