From e6d00c238875a43db1859dee68a675e0573c6d92 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:02:54 +0000 Subject: [PATCH 1/7] - mark AddTypeHandlerImpl as obsolete - syncronize type-handler mutates, to prevent lost writes --- Dapper/SqlMapper.cs | 71 ++++++++++++++++++++++++++++----------------- 1 file changed, 45 insertions(+), 26 deletions(-) diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs index fa5ce52df..656d00fa5 100644 --- a/Dapper/SqlMapper.cs +++ b/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); + } } /// @@ -339,7 +342,7 @@ public static void RemoveTypeMap(Type type) /// /// The type to handle. /// The handler to process the . - public static void AddTypeHandler(Type type, ITypeHandler handler) => AddTypeHandlerImpl(type, handler, true); + public static void AddTypeHandler(Type type, ITypeHandler handler) => AddTypeHandlerCore(type, handler, true); /// /// Determine if the specified type will be processed by a custom handler. /// @@ -353,7 +356,16 @@ public static void RemoveTypeMap(Type type) /// The type to handle. /// The handler to process the . /// Whether to clone the current type handler map. + [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, clone); + } + + private static void AddTypeHandlerCore(Type type, ITypeHandler? handler, bool clone) { if (type is null) throw new ArgumentNullException(nameof(type)); @@ -373,29 +385,35 @@ 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) + { + var snapshot = typeHandlers; + if (snapshot.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do - var newCopy = clone ? new Dictionary(snapshot) : snapshot; + var newCopy = clone ? new Dictionary(snapshot) : snapshot; #pragma warning disable 618 - typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]); - if (secondary is not null) - { - typeof(TypeHandlerCache<>).MakeGenericType(secondary).GetMethod(nameof(TypeHandlerCache.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]); - } + typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]); + if (secondary is not null) + { + typeof(TypeHandlerCache<>).MakeGenericType(secondary).GetMethod(nameof(TypeHandlerCache.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; } /// @@ -403,9 +421,10 @@ public static void AddTypeHandlerImpl(Type type, ITypeHandler? handler, bool clo /// /// The type to handle. /// The handler for the type . - public static void AddTypeHandler(TypeHandler handler) => AddTypeHandlerImpl(typeof(T), handler, true); + public static void AddTypeHandler(TypeHandler handler) => AddTypeHandlerCore(typeof(T), handler, true); private static Dictionary typeHandlers; + private static readonly object typeHandlersSyncLock = new(); internal const string LinqBinary = "System.Data.Linq.Binary"; @@ -479,7 +498,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 From 91702ff6d92e9944cd494f870982d67cef5da482 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:18:44 +0000 Subject: [PATCH 2/7] CI: revert pgsql uddate --- appveyor.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index c277902bd..08c5eb159 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -12,7 +12,7 @@ skip_commits: environment: Appveyor: true # Postgres - POSTGRES_PATH: C:\Program Files\PostgreSQL\13 + POSTGRES_PATH: C:\Program Files\PostgreSQL\9.6 PGUSER: postgres PGPASSWORD: Password12! POSTGRES_ENV_POSTGRES_USER: postgres @@ -32,7 +32,7 @@ environment: services: # - mysql - - postgresql13 + - postgresql init: - git config --global core.autocrlf input From cc188be05aff15d817166a3c2b8b37d2706cb75e Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:21:27 +0000 Subject: [PATCH 3/7] suppress Impl's use of clone: false --- Dapper/SqlMapper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs index 656d00fa5..76f00118b 100644 --- a/Dapper/SqlMapper.cs +++ b/Dapper/SqlMapper.cs @@ -362,7 +362,7 @@ public static void AddTypeHandlerImpl(Type type, ITypeHandler? handler, bool clo { // 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, clone); + AddTypeHandlerCore(type, handler, true); // do not allow suppress clone } private static void AddTypeHandlerCore(Type type, ITypeHandler? handler, bool clone) From ea273e0297a2af78c3802ed5629651181db7d6d8 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:26:35 +0000 Subject: [PATCH 4/7] avoid additional local snapshot, now that we're synchronized --- Dapper/SqlMapper.cs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Dapper/SqlMapper.cs b/Dapper/SqlMapper.cs index 76f00118b..d23e949a5 100644 --- a/Dapper/SqlMapper.cs +++ b/Dapper/SqlMapper.cs @@ -390,10 +390,9 @@ private static void AddTypeHandlerCore(Type type, ITypeHandler? handler, bool cl // the synchronize is just to prevent lost writes lock (typeHandlersSyncLock) { - var snapshot = typeHandlers; - if (snapshot.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do + if (typeHandlers.TryGetValue(type, out var oldValue) && handler == oldValue) return; // nothing to do - var newCopy = clone ? new Dictionary(snapshot) : snapshot; + var newCopy = clone ? new Dictionary(typeHandlers) : typeHandlers; #pragma warning disable 618 typeof(TypeHandlerCache<>).MakeGenericType(type).GetMethod(nameof(TypeHandlerCache.SetHandler), BindingFlags.Static | BindingFlags.NonPublic)!.Invoke(null, [handler]); From d5156145400192d9c8b84afb0def36eac2368368 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:30:54 +0000 Subject: [PATCH 5/7] explicitly request postgresql96 --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 08c5eb159..a53dfb064 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -32,7 +32,7 @@ environment: services: # - mysql - - postgresql + - postgresql96 init: - git config --global core.autocrlf input From 6c49d5d2d1279983385fc85bbc99455c4ab28c60 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:37:29 +0000 Subject: [PATCH 6/7] more pgsql poking --- appveyor.yml | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/appveyor.yml b/appveyor.yml index a53dfb064..c26a6758c 100644 --- a/appveyor.yml +++ b/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\9.6 + POSTGRES_PATH: C:\Program Files\PostgreSQL\15.8 PGUSER: postgres PGPASSWORD: Password12! POSTGRES_ENV_POSTGRES_USER: postgres @@ -32,7 +34,7 @@ environment: services: # - mysql - - postgresql96 + - postgresql15 init: - git config --global core.autocrlf input From 5c29a4bad08ac6cb173d409f6500e235643f37e4 Mon Sep 17 00:00:00 2001 From: Marc Gravell Date: Thu, 31 Oct 2024 06:42:33 +0000 Subject: [PATCH 7/7] pg path fix --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index c26a6758c..a8a7fb3b9 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -14,7 +14,7 @@ stack: postgresql 15 environment: Appveyor: true # Postgres - POSTGRES_PATH: C:\Program Files\PostgreSQL\15.8 + POSTGRES_PATH: C:\Program Files\PostgreSQL\15 PGUSER: postgres PGPASSWORD: Password12! POSTGRES_ENV_POSTGRES_USER: postgres