Mark AddTypeHandlerImpl as obsolete and prevent lost updates via AddTypeHandler (#2129)

* - mark AddTypeHandlerImpl as obsolete
- syncronize type-handler mutates, to prevent lost writes

* CI: revert pgsql uddate

* suppress Impl's use of clone: false

* avoid additional local snapshot, now that we're synchronized

* explicitly request postgresql96

* more pgsql poking

* pg path fix

Author: mgravell

Dapper/SqlMapper.cs

@@ -266,11 +266,14 @@ static SqlMapper()
         [MemberNotNull(nameof(typeHandlers))]
         private static void ResetTypeHandlers(bool clone)
         {
-            typeHandlers = [];
-            AddTypeHandlerImpl(typeof(DataTable), new DataTableHandler(), clone);
-            AddTypeHandlerImpl(typeof(XmlDocument), new XmlDocumentHandler(), clone);
-            AddTypeHandlerImpl(typeof(XDocument), new XDocumentHandler(), clone);
-            AddTypeHandlerImpl(typeof(XElement), new XElementHandler(), clone);
+            lock (typeHandlersSyncLock)
+            {
+                typeHandlers = [];
+                AddTypeHandlerCore(typeof(DataTable), new DataTableHandler(), clone);
+                AddTypeHandlerCore(typeof(XmlDocument), new XmlDocumentHandler(), clone);
+                AddTypeHandlerCore(typeof(XDocument), new XDocumentHandler(), clone);
+                AddTypeHandlerCore(typeof(XElement), new XElementHandler(), clone);
+            }
         }
 
         /// <summary>
@@ -339,7 +342,7 @@ public static void RemoveTypeMap(Type type)
         /// </summary>
         /// <param name="type">The type to handle.</param>
         /// <param name="handler">The handler to process the <paramref name="type"/>.</param>
-        public static void AddTypeHandler(Type type, ITypeHandler handler) => AddTypeHandlerImpl(type, handler, true);
+        public static void AddTypeHandler(Type type, ITypeHandler handler) => AddTypeHandlerCore(type, handler, true);
         /// <summary>
         /// Determine if the specified type will be processed by a custom handler.
         /// </summary>
@@ -353,7 +356,16 @@ public static void RemoveTypeMap(Type type)
         /// <param name="type">The type to handle.</param>
         /// <param name="handler">The handler to process the <paramref name="type"/>.</param>
         /// <param name="clone">Whether to clone the current type handler map.</param>
+        [Obsolete("Please use " + nameof(AddTypeHandler), error: true)]
+        [Browsable(false), EditorBrowsable(EditorBrowsableState.Never)]
         public static void AddTypeHandlerImpl(Type type, ITypeHandler? handler, bool clone)
+        {
+            // this method was accidentally made public; we'll mark it as illegal, but
+            // preserve existing usage in compiled code; sorry about this!
+            AddTypeHandlerCore(type, handler, true); // do not allow suppress clone
+        }
+
+        private static void AddTypeHandlerCore(Type type, ITypeHandler? handler, bool clone)
         {
             if (type is null) throw new ArgumentNullException(nameof(type));
 
@@ -373,39 +385,45 @@ public static void AddTypeHandlerImpl(Type type, ITypeHandler? handler, bool clo
                 }
             }
 
-            var snapshot = typeHandlers;
-            if (snapshot.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do
+            // synchronize between callers mutating type-handlers; note that regular query
+            // code may still be accessing the field, so we still use snapshot/mutate/swap;
+            // the synchronize is just to prevent lost writes
+            lock (typeHandlersSyncLock)
+            {
+                if (typeHandlers.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do
 
-            var newCopy = clone ? new Dictionary<Type, ITypeHandler>(snapshot) : snapshot;
+                var newCopy = clone ? new Dictionary<Type, ITypeHandler>(typeHandlers) : typeHandlers;
 
 #pragma warning disable 618
-            typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
-            if (secondary is not null)
-            {
-                typeof(TypeHandlerCache<>).MakeGenericType(secondary).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
-            }
+                typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
+                if (secondary is not null)
+                {
+                    typeof(TypeHandlerCache<>).MakeGenericType(secondary).GetMethod(nameof(TypeHandlerCache<int>.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]);
+                }
 #pragma warning restore 618
-            if (handler is null)
-            {
-                newCopy.Remove(type);
-                if (secondary is not null) newCopy.Remove(secondary);
-            }
-            else
-            {
-                newCopy[type] = handler;
-                if (secondary is not null) newCopy[secondary] = handler;
+                if (handler is null)
+                {
+                    newCopy.Remove(type);
+                    if (secondary is not null) newCopy.Remove(secondary);
+                }
+                else
+                {
+                    newCopy[type] = handler;
+                    if (secondary is not null) newCopy[secondary] = handler;
+                }
+                typeHandlers = newCopy;
             }
-            typeHandlers = newCopy;
         }
 
         /// <summary>
         /// Configure the specified type to be processed by a custom handler.
         /// </summary>
         /// <typeparam name="T">The type to handle.</typeparam>
         /// <param name="handler">The handler for the type <typeparamref name="T"/>.</param>
-        public static void AddTypeHandler<T>(TypeHandler<T> handler) => AddTypeHandlerImpl(typeof(T), handler, true);
+        public static void AddTypeHandler<T>(TypeHandler<T> handler) => AddTypeHandlerCore(typeof(T), handler, true);
 
         private static Dictionary<Type, ITypeHandler> typeHandlers;
+        private static readonly object typeHandlersSyncLock = new();
 
         internal const string LinqBinary = "System.Data.Linq.Binary";
 
@@ -479,7 +497,7 @@ public static void SetDbType(IDataParameter parameter, object value)
                         {
                             handler = (ITypeHandler)Activator.CreateInstance(
                                 typeof(SqlDataRecordHandler<>).MakeGenericType(argTypes))!;
-                            AddTypeHandlerImpl(type, handler, true);
+                            AddTypeHandlerCore(type, handler, true);
                             return DbType.Object;
                         }
                         catch

appveyor.yml

@@ -9,10 +9,12 @@ skip_commits:
 # install:
 #   - choco install dotnet-sdk --version 8.0.100
 
+stack: postgresql 15
+
 environment:
   Appveyor: true
   # Postgres
-  POSTGRES_PATH: C:\Program Files\PostgreSQL\13
+  POSTGRES_PATH: C:\Program Files\PostgreSQL\15
   PGUSER: postgres
   PGPASSWORD: Password12!
   POSTGRES_ENV_POSTGRES_USER: postgres
@@ -32,7 +34,7 @@ environment:
 
 services:
   # - mysql
-  - postgresql13
+  - postgresql15
 
 init:
   - git config --global core.autocrlf input