Skip to content

Commit c7a24f4

Browse files
committed
fix(extractor): an IMemoryOwner whose .Memory is handed off escapes the owner (mined ImageSharp; CodeQL-validated)
Cross-tool oracle (scripts/oracle_compare.py) on SixLabors/ImageSharp surfaced this: our flow-locals detector flagged `Image.WrapMemory`'s `memoryManager` (a `ByteMemoryManager : IMemoryOwner`) as an undisposed-local leak, but CodeQL (interprocedural) does NOT — the manager's `.Memory` is handed to `MemoryGroup.Wrap(...)` and lives on in the returned `Image`. A `Memory<T>` keeps its `IMemoryOwner` alive (it IS the backing), so passing `owner.Memory` as an argument transfers the owner's lifetime to the consumer; disposing it at method scope would be premature, and not disposing it here is not a leak. The flow escape-exclusion now untracks a local when its `.Memory` view is passed as an argument, scoped tightly: only `IMemoryOwner.Memory` (not any `local.Member`, so a `FileStream` whose `.Length` is read still leaks), and only for non-pool / non-`using` owners (a MemoryPool/`using` owner keeps its dangling-borrow tracking — OWN002 — intact). Mirrors the existing #80 escaping-ctor transfer and the D5 "ambiguous handoff -> conservatively exclude" rule. Regression sample MemoryOwnerEscapeSample.cs (--flow-locals): an IMemoryOwner whose `.Memory` is handed to a consumer stays silent ('handedOwner'); one whose `.Memory` is only read locally and never disposed still warns ('leakedOwner'). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent b787ffc commit c7a24f4

3 files changed

Lines changed: 61 additions & 2 deletions

File tree

‎.github/workflows/ci.yml‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,7 +458,8 @@ jobs:
458458
# Path-sensitive flow analysis of local IDisposables — bugs the flat D1
459459
# detector cannot catch (use-after-dispose, double-dispose, leak-on-path).
460460
dotnet run --project frontend/roslyn/OwnSharp.Extractor -- \
461-
frontend/roslyn/samples/FlowLocalsSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json"
461+
frontend/roslyn/samples/FlowLocalsSample.cs \
462+
frontend/roslyn/samples/MemoryOwnerEscapeSample.cs --flow-locals -o "$RUNNER_TEMP/flow.json"
462463
out=$(python -m ownlang ownir "$RUNNER_TEMP/flow.json" || true)
463464
echo "$out"
464465
echo "$out" | grep -q "OWN002" || { echo "FAIL: expected OWN002 (use-after-dispose)"; exit 1; }
@@ -564,7 +565,13 @@ jobs:
564565
# `ctorMoved`: a pooled buffer handed to a constructor whose result is RETURNED transfers
565566
# ownership to the returned wrapper -> escaped -> silent (mined FP on
566567
# Pipelines.Sockets.Unofficial: ArrayPoolBufferWriter.CreateNewSegment).
567-
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
568+
# P-016 escape-via-projection (mined: ImageSharp Image.WrapMemory; CodeQL agrees it is
569+
# no leak): an IMemoryOwner whose `.Memory` view is handed to a consumer as an argument
570+
# escapes the owner -> silent ('handedOwner', in the list below); one whose `.Memory` is
571+
# only READ locally and never disposed still leaks ('leakedOwner').
572+
echo "$out" | grep -qE "OWN001.*'leakedOwner'" \
573+
|| { echo "FAIL: expected OWN001 on the read-only, never-disposed IMemoryOwner"; exit 1; }
574+
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
568575
if echo "$out" | grep -q "'$ok'"; then echo "FAIL: silent/exempt case '$ok' was reported"; exit 1; fi
569576
done
570577
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)"

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2578,6 +2578,19 @@ or ImplicitObjectCreationExpressionSyntax } init
25782578
// returns `new ArrayPoolRefCountedSegment(pool, array, prev)`).
25792579
else if ((idn.Parent is AssignmentExpressionSyntax asg && asg.Right == idn)
25802580
|| (idn.Parent is ArgumentSyntax && !poolBuffers.Contains(nm) && !consumedArg)
2581+
// An IMemoryOwner's `.Memory` view handed off as an ARGUMENT escapes the
2582+
// OWNER: the Memory keeps the owner alive (it IS the backing), so a consumer
2583+
// that stores it — `MemoryGroup.Wrap(owner.Memory)` -> the returned Image —
2584+
// takes over the lifetime; the owner is not leaked here. Scoped to
2585+
// 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+
&& idn.Parent is MemberAccessExpressionSyntax { Name.Identifier.Text: "Memory" } projMem
2591+
&& projMem.Expression == idn
2592+
&& projMem.Parent is ArgumentSyntax
2593+
&& IsMemoryOwnerType(model.GetTypeInfo(idn).Type))
25812594
|| (poolBuffers.Contains(nm) && PassedToEscapingCtor(idn, model)))
25822595
escapedLocals.Add(nm);
25832596
}
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
using System;
2+
using System.Buffers;
3+
4+
namespace Own.Samples;
5+
6+
// P-016 escape-via-projection (mined: ImageSharp Image.WrapMemory; CodeQL agrees — no leak).
7+
//
8+
// An IMemoryOwner's `.Memory` view passed as an ARGUMENT hands the OWNER off: the Memory keeps
9+
// the owner alive (it IS the backing), so a consumer that stores it takes over the lifetime —
10+
// the owner is not leaked at method scope. Contrast: an owner whose `.Memory` is only READ
11+
// locally and never disposed IS a leak. Exercised with --flow-locals.
12+
13+
internal sealed class PixelOwner : IMemoryOwner<byte>
14+
{
15+
private readonly byte[] data = new byte[16];
16+
17+
public Memory<byte> Memory => this.data;
18+
19+
public void Dispose() { }
20+
}
21+
22+
internal static class MemoryOwnerEscape
23+
{
24+
private static void Store(Memory<byte> m) { }
25+
26+
// owner.Memory handed to a consumer (ambiguous transfer) -> owner escapes -> SILENT.
27+
public static void Transferred()
28+
{
29+
var handedOwner = new PixelOwner();
30+
Store(handedOwner.Memory); // .Memory passed as an arg -> ownership handed off
31+
}
32+
33+
// owner.Memory only READ locally (a length); owner never disposed -> real leak -> must WARN.
34+
public static int ReadOnlyLeak()
35+
{
36+
var leakedOwner = new PixelOwner();
37+
return leakedOwner.Memory.Length; // a local read, NOT a handoff
38+
}
39+
}

0 commit comments

Comments
 (0)