Skip to content

Commit d45a597

Browse files
claudeclaude
committed
merge main: 3 Npgsql FP-fixes (#88 AppDomain, #89 EventSource counters, #90 CTS-alias) for the npgsql re-mine
2 parents 3a6c809 + f9605da commit d45a597

6 files changed

Lines changed: 514 additions & 5 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 68 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,9 @@ jobs:
142142
frontend/roslyn/samples/ResolvedDisposableSample.cs \
143143
frontend/roslyn/samples/FieldReleaseSample.cs \
144144
frontend/roslyn/samples/StaticClassEscapeSample.cs \
145+
frontend/roslyn/samples/EventSourceCountersSample.cs \
146+
frontend/roslyn/samples/AppDomainShutdownSample.cs \
147+
frontend/roslyn/samples/AliasDisposeSample.cs \
145148
-o "$RUNNER_TEMP/facts.json"
146149
cat "$RUNNER_TEMP/facts.json"
147150
- name: Check facts through the core
@@ -239,6 +242,25 @@ jobs:
239242
fi
240243
echo "$out" | grep -q "pooled buffer 'leakedBuf'" \
241244
|| { echo "FAIL: a pooled field rented but never returned must still warn"; exit 1; }
245+
# field disposed through a local ALIAS (mined: Npgsql NpgsqlDataSource): `var cts = _cts;
246+
# cts.Dispose();` (and the `this._f` / `cts?.Dispose()` shapes) releases the field -> the
247+
# aliased fields must be SILENT.
248+
if echo "$out" | grep -qE "'_aliased'|'_aliasedQ'"; then
249+
echo "FAIL: a field disposed through a local alias was wrongly reported as undisposed"; exit 1
250+
fi
251+
# controls: an alias that is never disposed, and an alias REBOUND to a new object, must
252+
# both STILL leak (the recognition needs an actual dispose on an un-reassigned alias).
253+
echo "$out" | grep -qE "AliasDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'_neverDisposed'" \
254+
|| { echo "FAIL: a field aliased but never disposed must still warn"; exit 1; }
255+
echo "$out" | grep -qE "AliasDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'_rebound'" \
256+
|| { echo "FAIL: a field whose alias was rebound to a new object must still warn"; exit 1; }
257+
# Codex control: an alias rebound through a ref/out ARGUMENT must still leak.
258+
echo "$out" | grep -qE "AliasDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'_refRebound'" \
259+
|| { echo "FAIL: a field whose alias was rebound via a ref/out argument must still warn"; exit 1; }
260+
# Codex/CodeRabbit control: aliases are symbol-scoped, not name-keyed — an unrelated
261+
# same-named local disposed in another method must NOT credit the field, so it still leaks.
262+
echo "$out" | grep -qE "AliasDisposeSample\.cs:[0-9]+:.*\[OWN001\].*'_scopedLeak'" \
263+
|| { echo "FAIL: a same-named local in another scope must not be miscredited (symbol-scoped aliases)"; exit 1; }
242264
# WPF004: an ignored `X.Subscribe(...)` result leaks; the captured+
243265
# disposed one stays silent. "ignored" is unique to the WPF004 message.
244266
echo "$out" | grep -q "MessengerViewModel.cs" \
@@ -317,6 +339,20 @@ jobs:
317339
|| { echo "FAIL: expected OWN014 region escape on the static-event instance subscription"; exit 1; }
318340
echo "$out" | grep -q "region escape" \
319341
|| { echo "FAIL: expected the region-escape wording on the static-event capture"; exit 1; }
342+
# P-004 process-lifetime AppDomain-event exemption (mined: Npgsql PoolManager): a
343+
# NON-CAPTURING handler on a process-host AppDomain event (ProcessExit/DomainUnload/
344+
# UnhandledException/FirstChanceException) is a shutdown/diagnostics hook meant to live
345+
# for the process -> NOT a region escape -> silent. (Scoped to ShutdownCleanup, the
346+
# exempt class — CapturingShutdownSubscriber in the same file MUST still raise OWN014.)
347+
if echo "$out" | grep -qE "OWN014.*'ShutdownCleanup'"; then
348+
echo "FAIL: ShutdownCleanup's non-capturing AppDomain subscriptions were wrongly reported as OWN014"; exit 1
349+
fi
350+
# scope guards: a lambda on a NON-AppDomain static event still escapes; and an AppDomain
351+
# handler that CAPTURES instance state is still pinned to the process -> still OWN014 (Codex).
352+
echo "$out" | grep -qE "OWN014.*NonAppDomainSubscriber" \
353+
|| { echo "FAIL: a lambda on a non-AppDomain static event must still raise OWN014 (exemption stays scoped)"; exit 1; }
354+
echo "$out" | grep -qE "OWN014.*CapturingShutdownSubscriber" \
355+
|| { echo "FAIL: an instance-capturing AppDomain handler must still raise OWN014 (exemption is non-capturing only)"; exit 1; }
320356
# the unsubscribed variant (a matching `-=`, released capture) is mitigated
321357
# -> silent. Must NOT be reported.
322358
if echo "$out" | grep -q "CleanStaticEventViewModel"; then
@@ -330,6 +366,24 @@ jobs:
330366
if echo "$out" | grep -q "StaticAllocationCounter"; then
331367
echo "FAIL: a static-method handler on a static event was wrongly reported as a region escape"; exit 1
332368
fi
369+
# P-004 EventSource diagnostic-counter exemption (mined: Npgsql NpgsqlEventSource): a
370+
# DiagnosticCounter (EventCounter / PollingCounter / Incrementing{Event,Polling}Counter)
371+
# built with `this` is registered to the parent EventSource and shares its process
372+
# lifetime -> idiomatically never field-disposed -> must NOT be flagged as an undisposed leak.
373+
if echo "$out" | grep -qE "'_bytesPerSecond'|'_totalBytes'|'_commandDuration'|'_totalCommands'"; then
374+
echo "FAIL: an EventSource-owned DiagnosticCounter field was wrongly reported as an undisposed leak"; exit 1
375+
fi
376+
# scope control (Codex): the exemption keys off the DiagnosticCounter type handed to
377+
# `this`, NOT the EventSource class — so a plain owned IDisposable field in the same
378+
# EventSource (`_scratch`) must STILL raise the OWN001 disposable-field leak.
379+
echo "$out" | grep -qE "EventSourceCountersSample\.cs:[0-9]+:.*\[OWN001\].*'_scratch'" \
380+
|| { echo "FAIL: a non-counter owned IDisposable field in an EventSource must still warn (exemption stays counter-type-scoped)"; exit 1; }
381+
# declared-type control (Codex): a field DECLARED as a plain IDisposable that is ALSO
382+
# assigned a counter once (`_mixed = new EventCounter(..., this)`) must STILL leak its
383+
# earlier `new MemoryStream()` — the exemption requires the DECLARED field type to be a
384+
# DiagnosticCounter, so a name-only skip cannot hide the non-counter resource.
385+
echo "$out" | grep -qE "EventSourceCountersSample\.cs:[0-9]+:.*\[OWN001\].*'_mixed'" \
386+
|| { echo "FAIL: a field declared as a non-counter IDisposable that is later assigned a counter must still leak (exemption requires the DECLARED type to be a DiagnosticCounter)"; exit 1; }
333387
# P-004 process-lived-subscriber exemption (mined: ScreenToGif App +
334388
# Translator): the WPF `App` singleton hooking the process-lived
335389
# AppDomain.UnhandledException promotes nothing, so the static-source region
@@ -482,7 +536,8 @@ jobs:
482536
# Path-sensitive flow analysis of local IDisposables — bugs the flat D1
483537
# detector cannot catch (use-after-dispose, double-dispose, leak-on-path).
484538
dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \
485-
frontend/roslyn/samples/FlowLocalsSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json"
539+
frontend/roslyn/samples/FlowLocalsSample.cs \
540+
frontend/roslyn/samples/MemoryOwnerEscapeSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json"
486541
out=$(python -m ownlang ownir "$RUNNER_TEMP/flow.json" || true)
487542
echo "$out"
488543
echo "$out" | grep -q "OWN002" || { echo "FAIL: expected OWN002 (use-after-dispose)"; exit 1; }
@@ -588,7 +643,18 @@ jobs:
588643
# `ctorMoved`: a pooled buffer handed to a constructor whose result is RETURNED transfers
589644
# ownership to the returned wrapper -> escaped -> silent (mined FP on
590645
# Pipelines.Sockets.Unofficial: ArrayPoolBufferWriter.CreateNewSegment).
591-
for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured shaClean stopped defer ctorMoved; do
646+
# P-016 escape-via-projection (mined: ImageSharp Image.WrapMemory; CodeQL agrees it is
647+
# no leak): an IMemoryOwner whose `.Memory` view is handed to a consumer as an argument
648+
# escapes the owner -> silent ('handedOwner', in the list below); one whose `.Memory` is
649+
# only READ locally and never disposed still leaks ('leakedOwner').
650+
echo "$out" | grep -qE "OWN001.*'leakedOwner'" \
651+
|| { echo "FAIL: expected OWN001 on the read-only, never-disposed IMemoryOwner"; exit 1; }
652+
# boundary: the projection-escape is scoped to `new`'d owners — a MemoryPool RENTAL whose
653+
# .Memory is handed off after Dispose must KEEP its use-after-dispose tracking, NOT be
654+
# silenced (Codex/CodeRabbit P1; benchmark memorypool-double-dispose parity).
655+
echo "$out" | grep -qE "OWN002.*'pooled'" \
656+
|| { echo "FAIL: a MemoryPool owner's .Memory used after Dispose must still trip OWN002"; exit 1; }
657+
for ok in clean looped esc exemptTask whileClean asyncDisposed asyncDisposedCfg tfClean tfCatch tfRet tfNull cda daci cif ctorLater lamPrior other doClean swAll ncf captured shaClean stopped defer ctorMoved handedOwner; do
592658
if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi
593659
done
594660
echo "OK: flow-sensitive OWN001/002/003 on real C# (path-sensitive, loops via while/foreach/for, try/finally sequential, never-vs-every-path wording, dispose-optional exempt, beyond flat)"

0 commit comments

Comments
 (0)