From 00e7f0e5121c4c9ce1df7384d5f57e0099eebb68 Mon Sep 17 00:00:00 2001 From: Alexander Marek Date: Mon, 17 Aug 2026 16:49:44 +0200 Subject: [PATCH] Make the EfMap mapping cache thread-safe EfMap.Get caches one DbMapping per model in a static Dictionary, reading with TryGetValue and writing with an unsynchronised indexer assignment. Two threads arriving on a cold cache can therefore read the dictionary while another is writing it, which throws or returns the wrong entry. The failure does not surface as a dictionary error. MappedDataReader resolves each entity type inside a try/catch (it has to, so unmapped derived types are skipped), so a mapping lookup that blows up is swallowed per type and the resulting empty collection is reported as "No table mappings provided." That sends you looking at your model rather than at a race. Reproduced by running integration tests that bulk insert from more than one thread: serially they pass, concurrently they fail with that message. Switch to a ConcurrentDictionary and GetOrAdd. Two threads may still each build a DbMapping on first use with one result discarded, which is wasteful but correct, and unlike a Lazy it does not cache a failed build. Co-Authored-By: Claude Opus 5 --- src/EntityFramework.MappingAPI/EfMap.cs | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/EntityFramework.MappingAPI/EfMap.cs b/src/EntityFramework.MappingAPI/EfMap.cs index ab24b6b..258ed11 100644 --- a/src/EntityFramework.MappingAPI/EfMap.cs +++ b/src/EntityFramework.MappingAPI/EfMap.cs @@ -1,6 +1,6 @@ using EntityFramework.MappingAPI.Mappings; using System; -using System.Collections.Generic; +using System.Collections.Concurrent; using System.Data.Entity; using System.Data.Entity.Infrastructure; @@ -12,9 +12,14 @@ namespace EntityFramework.MappingAPI internal class EfMap { /// - /// + /// Mappings are cached per model for the lifetime of the process. A concurrent dictionary is + /// required, not just convenient: BulkInsert is reachable from several threads at once (any + /// app that bulk inserts from concurrent jobs or requests), and a plain Dictionary being + /// written on one thread while another reads it can throw or return the wrong entry. The + /// caller then sees an unrelated failure - via MappedDataReader, which catches per entity + /// type, that surfaces as the misleading "No table mappings provided." /// - private static readonly Dictionary Mappings = new Dictionary(); + private static readonly ConcurrentDictionary Mappings = new ConcurrentDictionary(); /// /// @@ -60,14 +65,11 @@ public static DbMapping Get(DbContext context) cacheKey = iDbModelCacheKeyProvider.CacheKey; } - DbMapping mapping; - if (Mappings.TryGetValue(cacheKey, out mapping)) - return mapping; - - mapping = new DbMapping(context); - - Mappings[cacheKey] = mapping; - return mapping; + // GetOrAdd rather than TryGetValue-then-assign: two threads racing here used to corrupt + // the cache. They may still both build a DbMapping on first use and one result is + // discarded, which is wasteful but correct - and it keeps a failed build from being + // cached, which a Lazy would not. + return Mappings.GetOrAdd(cacheKey, _ => new DbMapping(context)); } } }