Skip to content

Fix MultiMapAsync disposing the reader before unbuffered results are enumerated #2229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
jinseojang0903 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix MultiMapAsync disposing the reader before unbuffered results are enumerated #2229

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions Dapper/SqlMapper.Async.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -934,17 +934,25 @@ private static async Task<IEnumerable<TReturn>> MultiMapAsync<TFirst, TSecond, T
var identity = new Identity<TFirst, TSecond, TThird, TFourth, TFifth, TSixth, TSeventh>(command.CommandText, command.CommandTypeDirect, cnn, typeof(TFirst), param?.GetType());
var info = GetCacheInfo(identity, param, command.AddToCache);
bool wasClosed = cnn.State == ConnectionState.Closed;
using var cmd = command.TrySetupAsyncCommand(cnn, info.ParamReader);
DbDataReader? reader = null;
try
{
if (wasClosed) await cnn.TryOpenAsync(command.CancellationToken).ConfigureAwait(false);
using var cmd = command.TrySetupAsyncCommand(cnn, info.ParamReader);
using var reader = await ExecuteReaderWithFlagsFallbackAsync(cmd, wasClosed, CommandBehavior.SequentialAccess | CommandBehavior.SingleResult, command.CancellationToken).ConfigureAwait(false);
if (!command.Buffered) wasClosed = false; // handing back open reader; rely on command-behavior
reader = await ExecuteReaderWithFlagsFallbackAsync(cmd, wasClosed, CommandBehavior.SequentialAccess | CommandBehavior.SingleResult, command.CancellationToken).ConfigureAwait(false);
var results = MultiMapImpl<TFirst, TSecond, TThird, TFourth, TFifth, TSixth, TSeventh, TReturn>(null, CommandDefinition.ForCallback(command.Parameters, command.Flags), map, splitOn, reader, identity, true);
return command.Buffered ? results.ToList() : results;
if (command.Buffered)
{
return results.ToList();
}
wasClosed = false; // handing back open reader; rely on command-behavior
var deferred = ExecuteReaderSync(reader, results);
reader = null; // to prevent it being disposed before the caller gets to see it

@fathiaomar fathiaomar Oct 1, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting reader = null prevents immediate disposal so unbuffered results can be streamed, but if the caller abandons or partially enumerates the returned IEnumerable, the reader and underlying connection won't be closed. How are connection leaks handled if iteration stops early?

@jinseojang0903 jinseojang0903 Oct 1, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the question! Early termination is covered: ExecuteReaderSync wraps the reader in a using inside an iterator, so the redder is disposed whenever the enumerator is disposed. That includes break or an exception inside a foreach, and LINQ operators like First(), Take() and Any(). If Dapper opened the connection itself, the reader is created with CommandBehavior.CloseConnection, so disposing the reader also closes the connection.

The only uncovered case is a caller who never enumerates the result, or who calls GetEnumerator() manually and never disposes it. That's the existing contract for buffered: false, and the single-type QueryAsync unbuffered path behaves the same way. This change follows that path exactly. Before this fix, the multi-map unbuffered path threw as soon as you enumerated it, so no new leak is introduced.

return deferred;
}
finally
{
using (reader) { /* dispose if non-null */ }
if (wasClosed) cnn.Close();
}
}
Expand Down Expand Up @@ -983,16 +991,25 @@ private static async Task<IEnumerable<TReturn>> MultiMapAsync<TReturn>(this IDbC
var identity = new IdentityWithTypes(command.CommandText, command.CommandTypeDirect, cnn, types[0], param?.GetType(), types);
var info = GetCacheInfo(identity, param, command.AddToCache);
bool wasClosed = cnn.State == ConnectionState.Closed;
using var cmd = command.TrySetupAsyncCommand(cnn, info.ParamReader);
DbDataReader? reader = null;
try
{
if (wasClosed) await cnn.TryOpenAsync(command.CancellationToken).ConfigureAwait(false);
using var cmd = command.TrySetupAsyncCommand(cnn, info.ParamReader);
using var reader = await ExecuteReaderWithFlagsFallbackAsync(cmd, wasClosed, CommandBehavior.SequentialAccess | CommandBehavior.SingleResult, command.CancellationToken).ConfigureAwait(false);
reader = await ExecuteReaderWithFlagsFallbackAsync(cmd, wasClosed, CommandBehavior.SequentialAccess | CommandBehavior.SingleResult, command.CancellationToken).ConfigureAwait(false);
var results = MultiMapImpl(null, default, types, map, splitOn, reader, identity, true);
return command.Buffered ? results.ToList() : results;
if (command.Buffered)
{
return results.ToList();
}
wasClosed = false; // handing back open reader; rely on command-behavior
var deferred = ExecuteReaderSync(reader, results);
reader = null; // to prevent it being disposed before the caller gets to see it
return deferred;
}
finally
{
using (reader) { /* dispose if non-null */ }
if (wasClosed) cnn.Close();
}
}
Expand All @@ -1010,6 +1027,21 @@ private static IEnumerable<T> ExecuteReaderSync<T>(DbDataReader reader, Func<DbD
}
}

// wraps an already-materialized (but not yet enumerated) multi-map sequence so that the reader
// it depends on stays open until the caller actually finishes enumerating it; needed because the
// multi-map sequence is produced by a lazy (yield-based) iterator that doesn't run until enumerated,
// which happens *after* this async method has already returned control to the caller.
private static IEnumerable<TReturn> ExecuteReaderSync<TReturn>(DbDataReader reader, IEnumerable<TReturn> results)
{
using (reader)
{
foreach (var item in results)
{
yield return item;
}
}
}

/// <summary>
/// Execute a command that returns multiple result sets, and access each in turn.
/// </summary>
Expand Down
41 changes: 41 additions & 0 deletions tests/Dapper.Tests/AsyncTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,26 @@ public async Task TestMultiMapWithSplitAsync()
Assert.Equal("def", product.Category.Name);
}

[Fact]
public async Task TestMultiMapWithSplitUnbufferedAsync()
{
const string sql = "select 1 as id, 'abc' as name, 2 as id, 'def' as name";
var productQuery = await connection.QueryAsync<Product, Category, Product>(sql, (prod, cat) =>
{
prod.Category = cat;
return prod;
}, buffered: false).ConfigureAwait(false);

// the reader must still be alive when we start enumerating, even though
// the QueryAsync call above has already completed
var product = productQuery.First();
Assert.Equal(1, product.Id);
Assert.Equal("abc", product.Name);
Assert.NotNull(product.Category);
Assert.Equal(2, product.Category.Id);
Assert.Equal("def", product.Category.Name);
}

[Fact]
public async Task TestMultiMapArbitraryWithSplitAsync()
{
Expand All @@ -320,6 +340,27 @@ public async Task TestMultiMapArbitraryWithSplitAsync()
Assert.Equal("def", product.Category.Name);
}

[Fact]
public async Task TestMultiMapArbitraryWithSplitUnbufferedAsync()
{
const string sql = "select 1 as id, 'abc' as name, 2 as id, 'def' as name";
var productQuery = await connection.QueryAsync<Product>(sql, new[] { typeof(Product), typeof(Category) }, (objects) =>
{
var prod = (Product)objects[0];
prod.Category = (Category)objects[1];
return prod;
}, buffered: false).ConfigureAwait(false);

// the reader must still be alive when we start enumerating, even though
// the QueryAsync call above has already completed
var product = productQuery.First();
Assert.Equal(1, product.Id);
Assert.Equal("abc", product.Name);
Assert.NotNull(product.Category);
Assert.Equal(2, product.Category.Id);
Assert.Equal("def", product.Category.Name);
}

[Fact]
public async Task TestMultiMapWithSplitClosedConnAsync()
{
Expand Down