From 963af37d177ee5e0349ca46135a4f888daa865e1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 10:54:37 +0000 Subject: [PATCH 1/2] feat(p1a): recognise ADO.NET owned-returning members (ExecuteReader/CreateCommand/BeginTransaction) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1a ADO.NET tranche — the single most common real-world .NET resource leak: a DbDataReader from `cmd.ExecuteReader()` (or a DbCommand from `CreateCommand()`, a DbTransaction from `BeginTransaction()`) dropped without Dispose holds the reader / command / server-side cursor open until finalization. These are INSTANCE methods on a connection/command, unlike the existing static File/crypto/Xml/Json factories — but the acquire path (op="acquire") is static-vs-instance agnostic and the receiver is not an argument, so nothing is dropped (spike-confirmed). Recognise them in `IsOwningFactory` by method name + the RESOLVED return type implementing the System.Data contract interface (IDataReader / IDbCommand / IDbTransaction), which is provider-agnostic: it covers every concrete provider (SqlDataReader, NpgsqlDataReader, …), the abstract base (DbDataReader/DbCommand/DbTransaction), and the interface itself. Guarded by !AnyDisposableArgument (args are CommandBehavior/IsolationLevel enums anyway). Corpus fixture `ado-executereader-leak`: before.cs leaks the reader (OWN001), after.cs disposes it with `using` (clean), case.own reduces the pattern. Locally verified: case.own → OWN001 (test_corpus 21/21), full suite green, mypy --strict, ruff. The C# extractor branch and the before.cs/after.cs end-to-end run are verified by CI (no dotnet locally): golden C# compiles, C# leak extractor, and the corpus benchmark (before caught + after clean, no specificity regression). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .../ado-executereader-leak/after.cs | 14 +++++++++++ .../ado-executereader-leak/before.cs | 17 +++++++++++++ .../ado-executereader-leak/case.own | 18 ++++++++++++++ .../expected-diagnostics.txt | 1 + .../ado-executereader-leak/notes.md | 15 ++++++++++++ frontend/roslyn/OwnSharp.Extractor/Program.cs | 24 +++++++++++++++++++ 6 files changed, 89 insertions(+) create mode 100644 corpus/real-world/ado-executereader-leak/after.cs create mode 100644 corpus/real-world/ado-executereader-leak/before.cs create mode 100644 corpus/real-world/ado-executereader-leak/case.own create mode 100644 corpus/real-world/ado-executereader-leak/expected-diagnostics.txt create mode 100644 corpus/real-world/ado-executereader-leak/notes.md diff --git a/corpus/real-world/ado-executereader-leak/after.cs b/corpus/real-world/ado-executereader-leak/after.cs new file mode 100644 index 00000000..1faa3a7b --- /dev/null +++ b/corpus/real-world/ado-executereader-leak/after.cs @@ -0,0 +1,14 @@ +using System.Data.Common; + +// FIX: own the reader for the scope with `using`, so it is disposed on every exit path. +static class AdoReaderLeak +{ + static int Run(DbCommand cmd) + { + using var reader = cmd.ExecuteReader(); // disposed at scope exit -> clean + var n = 0; + while (reader.Read()) + n++; + return n; + } +} diff --git a/corpus/real-world/ado-executereader-leak/before.cs b/corpus/real-world/ado-executereader-leak/before.cs new file mode 100644 index 00000000..d69020ec --- /dev/null +++ b/corpus/real-world/ado-executereader-leak/before.cs @@ -0,0 +1,17 @@ +using System.Data.Common; + +// A DbDataReader returned by DbCommand.ExecuteReader() is a fresh owned IDisposable the caller +// must dispose -- dropping it leaks the reader and holds the underlying server-side cursor open +// until finalization. The command here is a borrowed parameter (the caller owns it), so the ONLY +// leak is `reader`. This is the single most common real-world ADO.NET resource leak. +static class AdoReaderLeak +{ + static int Run(DbCommand cmd) + { + var reader = cmd.ExecuteReader(); // fresh owned DbDataReader -> OWN001 (never disposed) + var n = 0; + while (reader.Read()) + n++; + return n; // BUG: reader never disposed + } +} diff --git a/corpus/real-world/ado-executereader-leak/case.own b/corpus/real-world/ado-executereader-leak/case.own new file mode 100644 index 00000000..1d4ca220 --- /dev/null +++ b/corpus/real-world/ado-executereader-leak/case.own @@ -0,0 +1,18 @@ +// OwnLang model of the canonical ADO.NET reader leak (P1a, ADO.NET tranche). A DbDataReader +// from DbCommand.ExecuteReader() is a fresh owned IDisposable the caller must dispose; here it +// is acquired, read, and never released — the generic OWN001 leak. The command is a borrowed +// parameter and the reader does not escape (only a count is returned), so it stays tracked. +// See notes.md for the recognition rule (return type implements System.Data.IDataReader). +module Corpus +resource Reader { + acquire open + release dispose + kind "disposable" + emit_type "DbDataReader" + emit_acquire "{args}.ExecuteReader()" + emit_release "{0}.Dispose()" +} +fn Run(cmd: int) { + let reader = acquire Reader(cmd); // var reader = cmd.ExecuteReader() + // rows read via reader.Read(); no `release reader;` — never disposed (OWN001) +} diff --git a/corpus/real-world/ado-executereader-leak/expected-diagnostics.txt b/corpus/real-world/ado-executereader-leak/expected-diagnostics.txt new file mode 100644 index 00000000..ed2a1929 --- /dev/null +++ b/corpus/real-world/ado-executereader-leak/expected-diagnostics.txt @@ -0,0 +1 @@ +OWN001 diff --git a/corpus/real-world/ado-executereader-leak/notes.md b/corpus/real-world/ado-executereader-leak/notes.md new file mode 100644 index 00000000..cec0c1ff --- /dev/null +++ b/corpus/real-world/ado-executereader-leak/notes.md @@ -0,0 +1,15 @@ +# ado-executereader-leak + +`DbCommand.ExecuteReader()` returns a fresh **owned** `DbDataReader` the caller must +dispose. Dropping it leaks the reader and keeps the server-side cursor/connection busy +until finalization — the single most common real-world ADO.NET resource leak. + +- **before.cs** — `var reader = cmd.ExecuteReader();` used and never disposed → `OWN001`. + The command is a borrowed parameter, so the only leak is the reader. +- **after.cs** — `using var reader = …` disposes it on every path → clean. + +Recognised by the extractor's `IsOwningFactory` (P1a, ADO.NET tranche): matched by method +name + the resolved return type implementing `System.Data.IDataReader`, so it covers every +provider (`SqlDataReader`, `NpgsqlDataReader`, …), the abstract `DbDataReader`, and the +interface. Sibling members `CreateCommand` (→ `IDbCommand`) and `BeginTransaction` +(→ `IDbTransaction`) are recognised the same way. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index ba602b2c..e8d9f1a3 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2299,9 +2299,33 @@ static bool IsOwningFactory(ExpressionSyntax? e, SemanticModel model) // overloads have no disposable arg and still resolve. (precision over recall.) && !AnyDisposableArgument(i, model)) return true; + // ADO.NET owned-returning members — the canonical real-world disposable leak. These are + // INSTANCE methods on a connection/command/transaction, but the acquire path (op="acquire") + // does not care static-vs-instance, and the receiver is not an argument so it is never + // dropped. Provider types vary (SqlCommand / NpgsqlCommand / SqliteCommand / ...), so match + // by method name + the RESOLVED return type implementing the System.Data contract interface, + // which covers every provider, the abstract base (DbDataReader/DbCommand/DbTransaction), and + // the interface itself: + // * ExecuteReader() -> a DbDataReader the caller must dispose (also frees the cursor) + // * CreateCommand() -> a DbCommand the caller must dispose + // * BeginTransaction() -> a DbTransaction the caller must dispose + // Arguments are non-disposable (CommandBehavior / IsolationLevel enums); the guard keeps any + // odd overload from dropping a disposable input. + if (!AnyDisposableArgument(i, model) + && ((sym.Name == "ExecuteReader" && ImplementsSystemDataInterface(sym.ReturnType, "IDataReader")) + || (sym.Name == "CreateCommand" && ImplementsSystemDataInterface(sym.ReturnType, "IDbCommand")) + || (sym.Name == "BeginTransaction" && ImplementsSystemDataInterface(sym.ReturnType, "IDbTransaction")))) + return true; return false; } +// True if `t` IS, or implements, the named `System.Data` interface (e.g. IDataReader). Covers a +// provider's concrete type (SqlDataReader), the abstract base (DbDataReader), and the interface. +static bool ImplementsSystemDataInterface(ITypeSymbol? t, string iface) => + t is not null + && ((t.Name == iface && IsInNamespace(t as INamedTypeSymbol, "System", "Data")) + || t.AllInterfaces.Any(i => i.Name == iface && IsInNamespace(i, "System", "Data"))); + // True if any argument to the call resolves to a type implementing IDisposable — a disposable // the callee might NOT take ownership of, so an enclosing owned-factory claim must be declined // rather than silently drop that argument from leak tracking. From 4ce410a910f1f96dc84e23c178e7416af63b84ae Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 27 Jun 2026 11:00:35 +0000 Subject: [PATCH 2/2] fix(p1a): pin the ADO factory match to an ADO receiver, not just the return type The ADO branch matched on method name + the RETURN type's System.Data interface only, so a non-ADO helper exposing an `ExecuteReader`/`CreateCommand`/ `BeginTransaction` that returns a borrowed/cached IDataReader/IDbCommand/ IDbTransaction would be minted as an owned acquire -> a false OWN001 when the caller correctly does not dispose the borrowed object. (Codex P2.) Pin BOTH ends, like every other factory branch verifies its declaring type: the RECEIVER (containing type) must implement the matching System.Data interface too -- IDbCommand for ExecuteReader, IDbConnection for CreateCommand/BeginTransaction -- in addition to the return-type interface. Still provider-agnostic (DbCommand / SqlCommand / NpgsqlCommand all implement IDbCommand). The corpus fixture is unaffected (DbCommand implements IDbCommand): case.own -> OWN001 still holds (corpus 21/21), full suite green, mypy --strict, ruff. C# branch verified by CI (golden C# / leak extractor / corpus benchmark). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019846YSZ35c7CdkWQ1qX5gm --- .../ado-executereader-leak/notes.md | 9 ++++-- frontend/roslyn/OwnSharp.Extractor/Program.cs | 32 ++++++++++++------- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/corpus/real-world/ado-executereader-leak/notes.md b/corpus/real-world/ado-executereader-leak/notes.md index cec0c1ff..15759b48 100644 --- a/corpus/real-world/ado-executereader-leak/notes.md +++ b/corpus/real-world/ado-executereader-leak/notes.md @@ -9,7 +9,10 @@ until finalization — the single most common real-world ADO.NET resource leak. - **after.cs** — `using var reader = …` disposes it on every path → clean. Recognised by the extractor's `IsOwningFactory` (P1a, ADO.NET tranche): matched by method -name + the resolved return type implementing `System.Data.IDataReader`, so it covers every +name + **both** the receiver and the return type implementing the `System.Data` contract +interfaces — the receiver an `IDbCommand` and the return an `IDataReader` — so it covers every provider (`SqlDataReader`, `NpgsqlDataReader`, …), the abstract `DbDataReader`, and the -interface. Sibling members `CreateCommand` (→ `IDbCommand`) and `BeginTransaction` -(→ `IDbTransaction`) are recognised the same way. +interface, while a non-ADO helper that merely exposes an `ExecuteReader` returning a borrowed +reader is not mistaken for an owned factory. Sibling members `CreateCommand` +(`IDbConnection` → `IDbCommand`) and `BeginTransaction` (`IDbConnection` → `IDbTransaction`) +are recognised the same way. diff --git a/frontend/roslyn/OwnSharp.Extractor/Program.cs b/frontend/roslyn/OwnSharp.Extractor/Program.cs index e8d9f1a3..f353ad0d 100644 --- a/frontend/roslyn/OwnSharp.Extractor/Program.cs +++ b/frontend/roslyn/OwnSharp.Extractor/Program.cs @@ -2300,21 +2300,29 @@ static bool IsOwningFactory(ExpressionSyntax? e, SemanticModel model) && !AnyDisposableArgument(i, model)) return true; // ADO.NET owned-returning members — the canonical real-world disposable leak. These are - // INSTANCE methods on a connection/command/transaction, but the acquire path (op="acquire") - // does not care static-vs-instance, and the receiver is not an argument so it is never - // dropped. Provider types vary (SqlCommand / NpgsqlCommand / SqliteCommand / ...), so match - // by method name + the RESOLVED return type implementing the System.Data contract interface, - // which covers every provider, the abstract base (DbDataReader/DbCommand/DbTransaction), and - // the interface itself: - // * ExecuteReader() -> a DbDataReader the caller must dispose (also frees the cursor) - // * CreateCommand() -> a DbCommand the caller must dispose - // * BeginTransaction() -> a DbTransaction the caller must dispose + // INSTANCE methods on a connection/command, but the acquire path (op="acquire") does not care + // static-vs-instance, and the receiver is not an argument so it is never dropped. Provider + // types vary (SqlCommand / NpgsqlCommand / SqliteCommand / ...), so match by method name + the + // RESOLVED types implementing the System.Data contract interfaces, which covers every provider, + // the abstract base (DbDataReader/DbCommand/DbTransaction), and the interface itself. BOTH the + // RECEIVER and the RETURN are pinned (like every other factory branch verifies its declaring + // type), so a non-ADO helper that merely exposes an `ExecuteReader` returning a borrowed/cached + // IDataReader is NOT mistaken for an owned factory (Codex): + // * IDbCommand.ExecuteReader() -> a DbDataReader the caller must dispose (frees the cursor) + // * IDbConnection.CreateCommand() -> a DbCommand the caller must dispose + // * IDbConnection.BeginTransaction() -> a DbTransaction the caller must dispose // Arguments are non-disposable (CommandBehavior / IsolationLevel enums); the guard keeps any // odd overload from dropping a disposable input. if (!AnyDisposableArgument(i, model) - && ((sym.Name == "ExecuteReader" && ImplementsSystemDataInterface(sym.ReturnType, "IDataReader")) - || (sym.Name == "CreateCommand" && ImplementsSystemDataInterface(sym.ReturnType, "IDbCommand")) - || (sym.Name == "BeginTransaction" && ImplementsSystemDataInterface(sym.ReturnType, "IDbTransaction")))) + && ((sym.Name == "ExecuteReader" + && ImplementsSystemDataInterface(sym.ContainingType, "IDbCommand") + && ImplementsSystemDataInterface(sym.ReturnType, "IDataReader")) + || (sym.Name == "CreateCommand" + && ImplementsSystemDataInterface(sym.ContainingType, "IDbConnection") + && ImplementsSystemDataInterface(sym.ReturnType, "IDbCommand")) + || (sym.Name == "BeginTransaction" + && ImplementsSystemDataInterface(sym.ContainingType, "IDbConnection") + && ImplementsSystemDataInterface(sym.ReturnType, "IDbTransaction")))) return true; return false; }