Fix leak when adding the same resolver multiple times (#4165)

Fixes #4166

(cherry picked from commit 0b6fb33)

src/Npgsql.NetTopologySuite/NpgsqlNetTopologySuiteExtensions.cs

@@ -27,8 +27,9 @@ public static INpgsqlTypeMapper UseNetTopologySuite(
             Ordinates handleOrdinates = Ordinates.None,
             bool geographyAsDefault = false)
         {
-            mapper.AddTypeResolverFactory(new NetTopologySuiteTypeHandlerResolverFactory(coordinateSequenceFactory, precisionModel,
-                handleOrdinates, geographyAsDefault));
+            mapper.AddTypeResolverFactory(
+                new NetTopologySuiteTypeHandlerResolverFactory(
+                    coordinateSequenceFactory, precisionModel, handleOrdinates, geographyAsDefault));
             return mapper;
         }
     }

src/Npgsql/TypeMapping/ConnectorTypeMapper.cs

@@ -34,7 +34,7 @@ internal NpgsqlDatabaseInfo DatabaseInfo
             }
         }
 
-        volatile TypeHandlerResolver[] _resolvers;
+        volatile List<TypeHandlerResolver> _resolvers;
         internal NpgsqlTypeHandler UnrecognizedTypeHandler { get; }
 
         readonly ConcurrentDictionary<uint, NpgsqlTypeHandler> _handlersByOID = new();
@@ -59,7 +59,7 @@ internal ConnectorTypeMapper(NpgsqlConnector connector) : base(GlobalTypeMapper.
         {
             Connector = connector;
             UnrecognizedTypeHandler = new UnknownTypeHandler(Connector);
-            _resolvers = Array.Empty<TypeHandlerResolver>();
+            _resolvers = new List<TypeHandlerResolver>();
         }
 
         #endregion Constructors
@@ -583,11 +583,22 @@ public override void AddTypeResolverFactory(TypeHandlerResolverFactory resolverF
         {
             lock (this)
             {
-                var oldResolvers = _resolvers;
-                var newResolvers = new TypeHandlerResolver[oldResolvers.Length + 1];
-                Array.Copy(oldResolvers, 0, newResolvers, 1, oldResolvers.Length);
-                newResolvers[0] = resolverFactory.Create(Connector);
-                _resolvers = newResolvers;
+                // Since EFCore.PG plugins (and possibly other users) repeatedly call NpgsqlConnection.GlobalTypeMapped.UseNodaTime,
+                // we replace an existing resolver of the same CLR type.
+
+                var newResolver = resolverFactory.Create(Connector);
+                var newResolverType = newResolver.GetType();
+
+                if (_resolvers[0].GetType() == newResolverType)
+                    _resolvers[0] = newResolver;
+                else
+                {
+                    for (var i = 0; i < _resolvers.Count; i++)
+                        if (_resolvers[i].GetType() == newResolverType)
+                            _resolvers.RemoveAt(i);
+
+                    _resolvers.Insert(0, newResolver);
+                }
 
                 _handlersByOID.Clear();
                 _handlersByNpgsqlDbType.Clear();
@@ -598,7 +609,6 @@ public override void AddTypeResolverFactory(TypeHandlerResolverFactory resolverF
             }
         }
 
-        [MemberNotNull(nameof(_resolvers))]
         public override void Reset()
         {
             lock (this)
@@ -612,10 +622,9 @@ public override void Reset()
                     _handlersByClrType.Clear();
                     _handlersByDataTypeName.Clear();
 
-                    var newResolvers = new TypeHandlerResolver[globalMapper.ResolverFactories.Count];
-                    for (var i = 0; i < newResolvers.Length; i++)
-                        newResolvers[i] = globalMapper.ResolverFactories[i].Create(Connector);
-                    _resolvers = newResolvers;
+                    _resolvers.Clear();
+                    for (var i = 0; i < globalMapper.ResolverFactories.Count; i++)
+                        _resolvers.Add(globalMapper.ResolverFactories[i].Create(Connector));
 
                     _userTypeMappings.Clear();
 

src/Npgsql/TypeMapping/GlobalTypeMapper.cs

@@ -168,7 +168,21 @@ public override void AddTypeResolverFactory(TypeHandlerResolverFactory resolverF
             Lock.EnterWriteLock();
             try
             {
-                ResolverFactories.Insert(0, resolverFactory);
+                // Since EFCore.PG plugins (and possibly other users) repeatedly call NpgsqlConnection.GlobalTypeMapped.UseNodaTime,
+                // we replace an existing resolver of the same CLR type.
+                var type = resolverFactory.GetType();
+
+                if (ResolverFactories[0].GetType() == type)
+                    ResolverFactories[0] = resolverFactory;
+                else
+                {
+                    for (var i = 0; i < ResolverFactories.Count; i++)
+                        if (ResolverFactories[i].GetType() == type)
+                            ResolverFactories.RemoveAt(i);
+
+                    ResolverFactories.Insert(0, resolverFactory);
+                }
+
                 RecordChange();
             }
             finally

test/Npgsql.Tests/TypeMapperTests.cs

@@ -51,59 +51,62 @@ public void Global_mapping()
             }
         }
 
-         [Test]
-         public void Local_mapping()
-         {
-             var myFactory = new MyInt32TypeHandlerResolverFactory();
-             using var _ = CreateTempPool(ConnectionString, out var connectionString);
-
-             using (var conn = OpenConnection(connectionString))
-             using (var cmd = new NpgsqlCommand("SELECT @p", conn))
-             {
-                 conn.TypeMapper.AddTypeResolverFactory(myFactory);
-                 cmd.Parameters.AddWithValue("p", 8);
-                 cmd.ExecuteScalar();
-                 Assert.That(myFactory.Reads, Is.EqualTo(1));
-                 Assert.That(myFactory.Writes, Is.EqualTo(1));
-             }
-
-             // Make sure reopening (same physical connection) reverts the mapping
-             using (var conn = OpenConnection(connectionString))
-             using (var cmd = new NpgsqlCommand("SELECT @p", conn))
-             {
-                 cmd.Parameters.AddWithValue("p", 8);
-                 cmd.ExecuteScalar();
-                 Assert.That(myFactory.Reads, Is.EqualTo(1));
-                 Assert.That(myFactory.Writes, Is.EqualTo(1));
-             }
-         }
-
-         [Test]
-         public void Global_reset()
-         {
-             var myFactory = new MyInt32TypeHandlerResolverFactory();
-             NpgsqlConnection.GlobalTypeMapper.AddTypeResolverFactory(myFactory);
-             using var _ = CreateTempPool(ConnectionString, out var connectionString);
-
-             using (OpenConnection(connectionString)) {}
-             // We now have a connector in the pool with our custom mapping
-
-             NpgsqlConnection.GlobalTypeMapper.Reset();
-             using (var conn = OpenConnection(connectionString))
-             {
-                 // Should be the pooled connector from before, but it should have picked up the reset
-                 conn.ExecuteScalar("SELECT 1");
-                 Assert.That(myFactory.Reads, Is.Zero);
-
-                 // Now create a second *physical* connection to make sure it picks up the new mapping as well
-                 using (var conn2 = OpenConnection(connectionString))
-                 {
-                     conn2.ExecuteScalar("SELECT 1");
-                     Assert.That(myFactory.Reads, Is.Zero);
-                 }
-                 NpgsqlConnection.ClearPool(conn);
-             }
-         }
+        [Test]
+        public void Local_mapping()
+        {
+            var myFactory = new MyInt32TypeHandlerResolverFactory();
+            using var _ = CreateTempPool(ConnectionString, out var connectionString);
+
+            using (var conn = OpenConnection(connectionString))
+            using (var cmd = new NpgsqlCommand("SELECT @p", conn))
+            {
+                conn.TypeMapper.AddTypeResolverFactory(myFactory);
+                cmd.Parameters.AddWithValue("p", 8);
+                cmd.ExecuteScalar();
+                Assert.That(myFactory.Reads, Is.EqualTo(1));
+                Assert.That(myFactory.Writes, Is.EqualTo(1));
+            }
+
+            // Make sure reopening (same physical connection) reverts the mapping
+            using (var conn = OpenConnection(connectionString))
+            using (var cmd = new NpgsqlCommand("SELECT @p", conn))
+            {
+                cmd.Parameters.AddWithValue("p", 8);
+                cmd.ExecuteScalar();
+                Assert.That(myFactory.Reads, Is.EqualTo(1));
+                Assert.That(myFactory.Writes, Is.EqualTo(1));
+            }
+        }
+
+        [Test]
+        public void Global_reset()
+        {
+            var myFactory = new MyInt32TypeHandlerResolverFactory();
+            NpgsqlConnection.GlobalTypeMapper.AddTypeResolverFactory(myFactory);
+            using var _ = CreateTempPool(ConnectionString, out var connectionString);
+
+            using (OpenConnection(connectionString))
+            {
+            }
+            // We now have a connector in the pool with our custom mapping
+
+            NpgsqlConnection.GlobalTypeMapper.Reset();
+            using (var conn = OpenConnection(connectionString))
+            {
+                // Should be the pooled connector from before, but it should have picked up the reset
+                conn.ExecuteScalar("SELECT 1");
+                Assert.That(myFactory.Reads, Is.Zero);
+
+                // Now create a second *physical* connection to make sure it picks up the new mapping as well
+                using (var conn2 = OpenConnection(connectionString))
+                {
+                    conn2.ExecuteScalar("SELECT 1");
+                    Assert.That(myFactory.Reads, Is.Zero);
+                }
+
+                NpgsqlConnection.ClearPool(conn);
+            }
+        }
 
         [Test]
         public async Task String_to_citext()