Skip to content

Commit fa09b1c

Browse files
committed
fix(pool): harden pooled view-in-field — this-instance views, semantic IMemoryOwner, this-qualified owner (CodeRabbit + Codex)
Three precision/robustness fixes to the pooled view-in-a-field pass, each flagged by review (the first two by BOTH CodeRabbit and Codex): 1. FP: the view-field alias map keyed the LHS with FieldName, so `other._view = _owner.Memory` recorded THIS instance's `_view` as aliasing `_owner` — a later handler read of this object's `_view.Span` would then falsely fire. Use ThisFieldName for the LHS, matching the release/read paths (this/bare only). 2. Recall: an owner field written `System.Buffers.IMemoryOwner<byte>` (or an alias, or a concrete type implementing IMemoryOwner) was missed by the `StartsWith("IMemoryOwner")` string check, so releasedAt stayed empty and the case was skipped. New `IsMemoryOwnerType` resolves the field's type SYMBOL (interface or via AllInterfaces); the cheap StartsWith still short-circuits the common unqualified spelling. IDisposable stays syntactic (unresolved customs). 3. Recall: `this._owner.Memory` was not recognised as a view (ViewOwner needs an IdentifierNameSyntax receiver). New `FieldViewOwner` accepts a this/bare-field receiver (ThisFieldName) for the `IMemoryOwner<T>.Memory` borrow, so both `_owner.Memory` and `this._owner.Memory` alias, while `other._owner.Memory` (another instance) does not. Behaviour on the corpus is unchanged (the `_owner.Memory` view still aliases and fires OWN002, the guarded after stays silent); the FP fix only narrows the map. C# validated by CI. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Rg8kSk1YT14x7A1vo5zgED
1 parent 243c439 commit fa09b1c

1 file changed

Lines changed: 28 additions & 3 deletions

File tree

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

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1109,6 +1109,25 @@ e is InvocationExpressionSyntax i
11091109
return null;
11101110
}
11111111

1112+
// Is `t` System.Buffers.IMemoryOwner<T> — the interface itself, or a concrete type that implements
1113+
// it? Resolved on the SYMBOL, so a fully-qualified `System.Buffers.IMemoryOwner<T>`, a type alias, or
1114+
// a concrete owner type is recognised — not only the bare `IMemoryOwner` spelling (CodeRabbit/Codex).
1115+
static bool IsMemoryOwnerType(ITypeSymbol? t) =>
1116+
t is INamedTypeSymbol nt
1117+
&& ((nt.Name == "IMemoryOwner" && IsInNamespace(nt, "System", "Buffers"))
1118+
|| nt.AllInterfaces.Any(i => i.Name == "IMemoryOwner" && IsInNamespace(i, "System", "Buffers")));
1119+
1120+
// The owner FIELD a `Memory` view assignment aliases — `_owner.Memory` / `this._owner.Memory` of an
1121+
// IMemoryOwner<T> field. Uses the this/bare receiver restriction (ThisFieldName), so `other._owner.
1122+
// Memory` (another instance's owner) is NOT recorded and the this-qualified spelling IS (Codex). The
1123+
// borrow is recognised by the resolved `IMemoryOwner<T>.Memory` property symbol, not by name.
1124+
static string? FieldViewOwner(ExpressionSyntax e, SemanticModel model) =>
1125+
e is MemberAccessExpressionSyntax { Name.Identifier.Text: "Memory" } mem
1126+
&& model.GetSymbolInfo(mem).Symbol is IPropertySymbol { ContainingType: { Name: "IMemoryOwner" } ict }
1127+
&& IsInNamespace(ict, "System", "Buffers")
1128+
? ThisFieldName(mem.Expression)
1129+
: null;
1130+
11121131
// The tracked owner buffer a FULL-LENGTH view spans, else null. POOL005: `buf.AsSpan()` /
11131132
// `buf.AsMemory()` with NO arguments span `[0, Length)` — the WHOLE backing array — and so does
11141133
// `buf.AsSpan(0, buf.Length)` / `new Span<T>(buf, 0, buf.Length)`, whose length argument is the
@@ -1991,8 +2010,14 @@ or ImplicitObjectCreationExpressionSyntax
19912010
{
19922011
if (fd.Modifiers.Any(mm => mm.IsKind(SyntaxKind.StaticKeyword)))
19932012
continue;
2013+
// IDisposable is matched syntactically (so a project's own unresolved `…Stream` etc.
2014+
// still counts); IMemoryOwner is matched on the resolved SYMBOL so a qualified
2015+
// `System.Buffers.IMemoryOwner<T>` / alias / concrete owner type counts too (CodeRabbit/
2016+
// Codex). The cheap `StartsWith` short-circuits the common unqualified spelling first.
19942017
var ftype = fd.Declaration.Type.ToString();
1995-
if (!IsDisposableType(ftype) && !ftype.StartsWith("IMemoryOwner", StringComparison.Ordinal))
2018+
if (!IsDisposableType(ftype)
2019+
&& !ftype.StartsWith("IMemoryOwner", StringComparison.Ordinal)
2020+
&& !IsMemoryOwnerType(model.GetTypeInfo(fd.Declaration.Type).Type))
19962021
continue;
19972022
foreach (var v in fd.Declaration.Variables)
19982023
dispoFieldLine[v.Identifier.Text] = LineOf(v);
@@ -2024,8 +2049,8 @@ or ImplicitObjectCreationExpressionSyntax
20242049
var viewFieldOwner = new Dictionary<string, string>(StringComparer.Ordinal);
20252050
foreach (var a in assigns)
20262051
if (a.IsKind(SyntaxKind.SimpleAssignmentExpression)
2027-
&& FieldName(a.Left) is { } vf
2028-
&& ViewOwner(a.Right, model) is { } vo)
2052+
&& ThisFieldName(a.Left) is { } vf
2053+
&& FieldViewOwner(a.Right, model) is { } vo)
20292054
viewFieldOwner[vf] = vo;
20302055

20312056
// handler method names that are LIVE subscription targets. `+=` subscriptions are

0 commit comments

Comments
 (0)