Skip to content

Commit 397395d

Browse files
committed
fix(extractor): scope projection-escape to new'd owners only — keep pool-rental tracking (Codex/CodeRabbit P1)
The first cut excluded only poolBuffers (ArrayPool) and usingMemoryOwners (using-declared MemoryPool), but a `var` MemoryPool.Rent owner is in neither — so its `.Memory` handoff wrongly untracked it, dropping the dangling-borrow / double-dispose findings (OWN002/OWN003). The corpus benchmark caught it: memorypool-double-dispose regressed (recall 23 -> 22). Scope the projection-escape to a new `newedDisposables` whitelist — locals created via `new` (the WrapMemory `new ByteMemoryManager(...)` shape). A pool rental of ANY kind (ArrayPool or MemoryPool, `var` or `using`) is never `new`'d, so it keeps its full use-after-dispose tracking; the renter owns the Return/Dispose. Stricter and more self-evident than enumerating pool sets. Sample/CI gain a MemoryPool-owner boundary case: `pooled.Dispose(); Store(pooled.Memory)` must still trip OWN002 (not be silenced). handedOwner stays silent, leakedOwner still warns. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent c7a24f4 commit 397395d

3 files changed

Lines changed: 25 additions & 4 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -571,6 +571,11 @@ jobs:
571571
# only READ locally and never disposed still leaks ('leakedOwner').
572572
echo "$out" | grep -qE "OWN001.*'leakedOwner'" \
573573
|| { echo "FAIL: expected OWN001 on the read-only, never-disposed IMemoryOwner"; exit 1; }
574+
# boundary: the projection-escape is scoped to `new`'d owners — a MemoryPool RENTAL whose
575+
# .Memory is handed off after Dispose must KEEP its use-after-dispose tracking, NOT be
576+
# silenced (Codex/CodeRabbit P1; benchmark memorypool-double-dispose parity).
577+
echo "$out" | grep -qE "OWN002.*'pooled'" \
578+
|| { echo "FAIL: a MemoryPool owner's .Memory used after Dispose must still trip OWN002"; exit 1; }
574579
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
575580
if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi
576581
done

‎frontend/roslyn/OwnSharp.Extractor/Program.cs‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2461,6 +2461,7 @@ or ImplicitObjectCreationExpressionSyntax
24612461
var candidates = new HashSet<string>();
24622462
var poolBuffers = new HashSet<string>(); // candidates that are ArrayPool<T> buffers
24632463
var usingMemoryOwners = new HashSet<string>(); // `using`-declared MemoryPool owners
2464+
var newedDisposables = new HashSet<string>(); // candidates created via `new` (NOT a pool rental / factory)
24642465
foreach (var ld in mbody.DescendantNodes().OfType<LocalDeclarationStatementSyntax>())
24652466
{
24662467
if (ld.UsingKeyword != default)
@@ -2481,7 +2482,10 @@ or ImplicitObjectCreationExpressionSyntax
24812482
or ImplicitObjectCreationExpressionSyntax } init
24822483
&& model.GetTypeInfo(init.Value).Type is { } dt
24832484
&& ImplementsIDisposable(dt) && !IsDisposeOptional(dt))
2485+
{
24842486
candidates.Add(v.Identifier.Text);
2487+
newedDisposables.Add(v.Identifier.Text);
2488+
}
24852489
else if (IsPoolRent(v.Initializer?.Value, model)) // an ArrayPool<T> buffer
24862490
{
24872491
candidates.Add(v.Identifier.Text);
@@ -2583,10 +2587,12 @@ or ImplicitObjectCreationExpressionSyntax } init
25832587
// that stores it — `MemoryGroup.Wrap(owner.Memory)` -> the returned Image —
25842588
// takes over the lifetime; the owner is not leaked here. Scoped to
25852589
// IMemoryOwner.`Memory` (not any `local.Member`, so a FileStream whose
2586-
// `.Length` is read still leaks) and to non-pool / non-`using` owners (a
2587-
// MemoryPool / `using` owner keeps its dangling-borrow tracking). Mined FP on
2588-
// ImageSharp Image.WrapMemory; CodeQL agrees it is no leak (interprocedural).
2589-
|| (!poolBuffers.Contains(nm) && !usingMemoryOwners.Contains(nm)
2590+
// `.Length` is read still leaks) and to `new`'d owners ONLY: a pool rental —
2591+
// ArrayPool or MemoryPool, `var` or `using` — keeps its dangling-borrow / use-
2592+
// after-dispose (OWN002/OWN003) tracking through `.Memory` handoffs, since the
2593+
// RENTER owns the Return/Dispose (Codex/CodeRabbit; benchmark memorypool-double-
2594+
// dispose). Mined FP on ImageSharp Image.WrapMemory; CodeQL agrees — no leak.
2595+
|| (newedDisposables.Contains(nm)
25902596
&& idn.Parent is MemberAccessExpressionSyntax { Name.Identifier.Text: "Memory" } projMem
25912597
&& projMem.Expression == idn
25922598
&& projMem.Parent is ArgumentSyntax

‎frontend/roslyn/samples/MemoryOwnerEscapeSample.cs‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,4 +36,14 @@ public static int ReadOnlyLeak()
3636
var leakedOwner = new PixelOwner();
3737
return leakedOwner.Memory.Length; // a local read, NOT a handoff
3838
}
39+
40+
// A MemoryPool RENTAL (not a `new`'d owner) keeps its dangling-borrow tracking through a
41+
// `.Memory` handoff: projection-escape is scoped to `new`'d owners only, so using `pooled.Memory`
42+
// after Dispose still trips OWN002 — the rule must NOT silence it (Codex/CodeRabbit P1).
43+
public static void PoolOwnerNotEscaped()
44+
{
45+
var pooled = MemoryPool<byte>.Shared.Rent(16);
46+
pooled.Dispose();
47+
Store(pooled.Memory); // use of .Memory AFTER Dispose -> OWN002, must NOT be silenced
48+
}
3949
}

0 commit comments

Comments
 (0)