Make type map updates thread-safe

Dapper/SqlMapper.cs

@@ -168,7 +168,9 @@ where pair.Value > 1
                    select Tuple.Create(pair.Key, pair.Value);
         }
 
-        private static Dictionary<Type, TypeMapEntry> typeMap;
+        // replaced wholesale (never mutated) under typeMapSyncLock; volatile so readers always observe the latest published snapshot
+        private static volatile Dictionary<Type, TypeMapEntry> typeMap;
+        private static readonly object typeMapSyncLock = new();
 
         [Flags]
         internal enum TypeMapEntryFlags
@@ -292,21 +294,24 @@ public static void AddTypeMap(Type type, DbType dbType)
         /// <param name="useGetFieldValue">Whether to prefer <see cref="DbDataReader.GetFieldValue{T}(int)"/> over <see cref="DbDataReader.GetValue(int)"/>.</param>
         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<Type, TypeMapEntry>(snapshot) { [type] = value });
+                SetTypeMap(new Dictionary<Type, TypeMapEntry>(snapshot) { [type] = value });
+            }
         }
 
         private static void SetTypeMap(Dictionary<Type, TypeMapEntry> value)
@@ -326,15 +331,18 @@ private static void SetTypeMap(Dictionary<Type, TypeMapEntry> value)
         /// <param name="type">The type to remove from the current map.</param>
         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<Type, TypeMapEntry>(snapshot);
-            newCopy.Remove(type);
+                var newCopy = new Dictionary<Type, TypeMapEntry>(snapshot);
+                newCopy.Remove(type);
 
-            SetTypeMap(newCopy);
+                SetTypeMap(newCopy);
+            }
         }
 
         /// <summary>

Directory.Packages.props

@@ -35,6 +35,7 @@
     <PackageVersion Include="Norm.net" Version="5.4.0" />
     <PackageVersion Include="Npgsql" Version="10.0.2" />
     <PackageVersion Include="PetaPoco" Version="5.1.306" />
+    <PackageVersion Include="pengdows.crud.fakeDb" Version="2.0.5" />
     <PackageVersion Include="RepoDb.SqlServer" Version="1.13.1" />
     <PackageVersion Include="ServiceStack.OrmLite.SqlServer" Version="10.0.6" />
     <PackageVersion Include="Snowflake.Data" Version="5.5.0" />

tests/Dapper.Tests/Dapper.Tests.csproj

@@ -31,8 +31,9 @@
     <PackageReference Include="log4net" />
   </ItemGroup>
 
-  <ItemGroup Condition="'$(TargetFramework)' != 'net481'">
-    <PackageReference Include="Npgsql" />
+  <ItemGroup Condition="'$(TargetFramework)' != 'net481'">
+    <PackageReference Include="pengdows.crud.fakeDb" />
+    <PackageReference Include="Npgsql" />
     <PackageReference Include="Snowflake.Data" />
   </ItemGroup>
 

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<T> { }
+
+        // distinct closed types: Marker<int>, Marker<Marker<int>>, ...
+        private static Type[] MakeTypes(int count)
+        {
+            var types = new Type[count];
+            var t = typeof(Marker<int>);
+            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