diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs index 8f3956cab..7575ce4cc 100644 --- a/Dapper/SqlMapper.cs +++ b/Dapper/SqlMapper.cs @@ -168,7 +168,9 @@ where pair.Value > 1 select Tuple.Create(pair.Key, pair.Value); } - private static Dictionary typeMap; + // replaced wholesale (never mutated) under typeMapSyncLock; volatile so readers always observe the latest published snapshot + private static volatile Dictionary typeMap; + private static readonly object typeMapSyncLock = new(); [Flags] internal enum TypeMapEntryFlags @@ -292,21 +294,24 @@ public static void AddTypeMap(Type type, DbType dbType) /// Whether to prefer over . public static void AddTypeMap(Type type, DbType dbType, bool useGetFieldValue) { - // use clone, mutate, replace to avoid threading issues - var snapshot = typeMap; - var flags = TypeMapEntryFlags.None; - if (dbType >= 0) + lock (typeMapSyncLock) { - flags |= TypeMapEntryFlags.SetType; - } - if (useGetFieldValue) - { - flags |= TypeMapEntryFlags.UseGetFieldValue; - } - var value = new TypeMapEntry(dbType, flags); - if (snapshot.TryGetValue(type, out var oldValue) && oldValue.Equals(value)) return; // nothing to do + // use clone, mutate, replace to avoid threading issues + var snapshot = typeMap; + var flags = TypeMapEntryFlags.None; + if (dbType >= 0) + { + flags |= TypeMapEntryFlags.SetType; + } + if (useGetFieldValue) + { + flags |= TypeMapEntryFlags.UseGetFieldValue; + } + var value = new TypeMapEntry(dbType, flags); + if (snapshot.TryGetValue(type, out var oldValue) && oldValue.Equals(value)) return; // nothing to do - SetTypeMap(new Dictionary(snapshot) { [type] = value }); + SetTypeMap(new Dictionary(snapshot) { [type] = value }); + } } private static void SetTypeMap(Dictionary value) @@ -326,15 +331,18 @@ private static void SetTypeMap(Dictionary value) /// The type to remove from the current map. public static void RemoveTypeMap(Type type) { - // use clone, mutate, replace to avoid threading issues - var snapshot = typeMap; + lock (typeMapSyncLock) + { + // use clone, mutate, replace to avoid threading issues + var snapshot = typeMap; - if (!snapshot.ContainsKey(type)) return; // nothing to do + if (!snapshot.ContainsKey(type)) return; // nothing to do - var newCopy = new Dictionary(snapshot); - newCopy.Remove(type); + var newCopy = new Dictionary(snapshot); + newCopy.Remove(type); - SetTypeMap(newCopy); + SetTypeMap(newCopy); + } } /// diff --git a/Directory.Packages.props b/Directory.Packages.props index 2f91a3cca..8f2772419 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -35,6 +35,7 @@ + diff --git a/tests/Dapper.Tests/Dapper.Tests.csproj b/tests/Dapper.Tests/Dapper.Tests.csproj index 93b498618..09947a89a 100644 --- a/tests/Dapper.Tests/Dapper.Tests.csproj +++ b/tests/Dapper.Tests/Dapper.Tests.csproj @@ -31,8 +31,9 @@ - - + + + diff --git a/tests/Dapper.Tests/FakeDbTests.Issue1439.cs b/tests/Dapper.Tests/FakeDbTests.Issue1439.cs new file mode 100644 index 000000000..662216632 --- /dev/null +++ b/tests/Dapper.Tests/FakeDbTests.Issue1439.cs @@ -0,0 +1,109 @@ +#if !NET481 +using System; +using System.Collections.Generic; +using System.Data; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Xunit; + +namespace Dapper.Tests +{ + public sealed class FakeDbIssue1439Tests + { + private sealed class Marker { } + + // distinct closed types: Marker, Marker>, ... + private static Type[] MakeTypes(int count) + { + var types = new Type[count]; + var t = typeof(Marker); + for (int i = 0; i < count; i++) + { + types[i] = t; + t = typeof(Marker<>).MakeGenericType(t); + } + return types; + } + + [Fact] + public void ConcurrentAddTypeMapUpdatesAreNotLost() + { + const int Threads = 8, PerThread = 16, Rounds = 25; + var types = MakeTypes(Threads * PerThread); + try + { + for (int round = 0; round < Rounds; round++) + { + using var barrier = new Barrier(Threads); + var tasks = Enumerable.Range(0, Threads).Select(n => Task.Factory.StartNew(() => + { + barrier.SignalAndWait(); + foreach (var type in types.Skip(n * PerThread).Take(PerThread)) + { + SqlMapper.AddTypeMap(type, DbType.String); + } + }, TaskCreationOptions.LongRunning)).ToArray(); + Task.WaitAll(tasks); + + foreach (var type in types) + { +#pragma warning disable 618 + Assert.Equal(DbType.String, SqlMapper.LookupDbType(type, "value", true, out _)); +#pragma warning restore 618 + } + + // clear again so the next round races from a clean map + foreach (var type in types) SqlMapper.RemoveTypeMap(type); + } + } + finally + { + foreach (var type in types) SqlMapper.RemoveTypeMap(type); + } + } + + [Fact] + public void ConcurrentAddAndRemoveDoNotLoseOtherUpdates() + { + const int PerThread = 64, Rounds = 25; + var keep = MakeTypes(PerThread); + var churn = MakeTypes(PerThread * 2).Skip(PerThread).ToArray(); + try + { + for (int round = 0; round < Rounds; round++) + { + using var barrier = new Barrier(2); + var adder = Task.Factory.StartNew(() => + { + barrier.SignalAndWait(); + foreach (var type in keep) SqlMapper.AddTypeMap(type, DbType.String); + }, TaskCreationOptions.LongRunning); + var churner = Task.Factory.StartNew(() => + { + barrier.SignalAndWait(); + foreach (var type in churn) + { + SqlMapper.AddTypeMap(type, DbType.Int32); + SqlMapper.RemoveTypeMap(type); + } + }, TaskCreationOptions.LongRunning); + Task.WaitAll(adder, churner); + + foreach (var type in keep) + { +#pragma warning disable 618 + Assert.Equal(DbType.String, SqlMapper.LookupDbType(type, "value", true, out _)); +#pragma warning restore 618 + } + foreach (var type in keep) SqlMapper.RemoveTypeMap(type); + } + } + finally + { + foreach (var type in keep.Concat(churn)) SqlMapper.RemoveTypeMap(type); + } + } + } +} +#endif