Skip to content

Fix cache corruption due to CollectCacheGarbage call during GridReader/Async queries - #2218

Open
aklein53 wants to merge 1 commit into
DapperLib:mainfrom
aklein53:fix/multimap-param-cache-poisoning
Open

aklein53 wants to merge 1 commit into
DapperLib:mainfrom
aklein53:fix/multimap-param-cache-poisoning

Conversation

@aklein53

Copy link
Copy Markdown

This fixes a couple of issues where GetCacheInfo is called with null exampleParameters for an Identity that has non-null parameters. For example, deferred GridReader and async MultiMap calls do an initial call with non-null example parameters, and then do a second call with null example parameters. Usually this works fine because the cache entry is created in the first call with the correct reader and the second call results in a cache hit based on the Identity. However, with multiple concurrent Dapper calls happening at once, CollectCacheGarbage can sometimes be hit after the first GetCacheInfo call and before the second. This results in the second call (with null example parameters) creating a cache entry with a bad reader that breaks all future usages of that Identity until that entry is removed from the cache.

Fixes #1169

…exampleParameters for an Identity that has non-null parameters. For example, deferred GridReader and async MultiMap calls do an initial call with non-null example parameters, and then do a second call with null example parameters. Usually this works fine because the cache entry is created in the first call with the correct reader and the second call results in a cache hit based on the Identity. However, with multiple concurrent Dapper calls happening at once, CollectCacheGarbage can sometimes be hit after the first GetCacheInfo call and before the second. This results in the second call (with null example parameters) creating a cache entry with a bad reader that breaks all future usages of that Identity until that entry is removed from the cache.
@lahma

lahma commented Oct 2, 2026

Copy link
Copy Markdown

We hit this in production code and in CI (Dapper 2.1.79, Npgsql 10): paged queries going through the async QueryAsync(sql, Type[], map, param) overload with DynamicParameters intermittently started failing with column "sites" does not exist (the @sites parameter was never bound) or with Npgsql refusing to write DynamicParameters.ParameterNames as a parameter value - and once it happened, every later execution of that SQL failed until the process restarted. Tracing it led to exactly the window this PR describes: the second GetCacheInfo lookup with a null example after CollectCacheGarbage has removed the fresh, never-hit entry.

I verified the same approach independently before finding this PR (choosing the reader from identity.ParametersType rather than from the example instance), so +1 for getting this merged. A couple of observations from that work that may be useful:

  • The generic multi-map overloads are affected too, just more narrowly: CommandDefinition.ForCallback only keeps the parameters when they are a DynamicParameters, so a custom IDynamicParameters (or a dictionary) still reaches the second lookup as null.
  • A deterministic repro doesn't need a fake connection: an IDynamicParameters whose AddParameters calls SqlMapper.PurgeQueryCache() evicts the entry at exactly the right moment, and works against the existing SQLite provider. I have tests in that shape covering the Type[] overload with a custom IDynamicParameters, the Type[] overload with a DynamicParameters subclass (the case we hit), and the generic QueryAsync<T1, T2, TReturn> overload - each fails on main and passes with this fix. Happy to contribute them on top of this PR if that helps.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Dapper looses parameters

2 participants