From 730bb9eddf24897fb88b17d7f1641826bc137f65 Mon Sep 17 00:00:00 2001 From: Sfurti-yb Date: Tue, 25 Nov 2025 16:43:43 +0530 Subject: [PATCH 1/2] Set up a lock before refresh mechanism --- src/Npgsql/NpgsqlConnection.cs | 41 ++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 17 deletions(-) diff --git a/src/Npgsql/NpgsqlConnection.cs b/src/Npgsql/NpgsqlConnection.cs index 6f7d1c08f0..205b93a4e3 100644 --- a/src/Npgsql/NpgsqlConnection.cs +++ b/src/Npgsql/NpgsqlConnection.cs @@ -168,8 +168,20 @@ internal static NpgsqlConnection FromDataSource(NpgsqlDataSource dataSource) /// A task representing the asynchronous operation. public override Task OpenAsync(CancellationToken cancellationToken) => Open(async: true, cancellationToken); + static readonly object SetupDataSourceLock = new(); void SetupDataSource() { + // A pool already corresponds to this version of string but also needs Refresh + + lock (SetupDataSourceLock) + { + if (PoolManager.Pools.TryGetValue(_connectionString, out _dataSource) && _dataSource.NeedsRefresh()) + { + _dataSource.Refresh(); + Settings = _dataSource.Settings; // Great, we already have a pool + return; + } + } // Fast path: a pool already corresponds to this exact version of the connection string. if (PoolManager.Pools.TryGetValue(_connectionString, out _dataSource) && !_dataSource.NeedsRefresh()) { @@ -177,14 +189,6 @@ void SetupDataSource() return; } - // A pool already corresponds to this version of string but also needs Refresh - - if (PoolManager.Pools.TryGetValue(_connectionString, out _dataSource) && _dataSource.NeedsRefresh()) - { - _dataSource.Refresh(); - Settings = _dataSource.Settings; // Great, we already have a pool - return; - } // Connection string hasn't been seen before. Check for empty and parse (slow one-time path). if (_connectionString == string.Empty) @@ -204,6 +208,18 @@ void SetupDataSource() // Note that we remove TargetSessionAttributes to make all connection strings that are otherwise identical point to the same pool. var canonical = settings.ConnectionStringForMultipleHosts; + lock (SetupDataSourceLock) + { + if (PoolManager.Pools.TryGetValue(canonical, out _dataSource) && _dataSource.NeedsRefresh()) + { + _dataSource.Refresh(); + // The pool was found, but only under the canonical key - we're using a different version + // for the first time. Map it via our own key for next time. + _dataSource = PoolManager.Pools.GetOrAdd(_connectionString, _dataSource); + return; + } + } + if (PoolManager.Pools.TryGetValue(canonical, out _dataSource) && !_dataSource.NeedsRefresh()) { // If this is a multi-host data source and the user specified a TargetSessionAttributes, create a wrapper in front of the @@ -217,15 +233,6 @@ void SetupDataSource() return; } - if (PoolManager.Pools.TryGetValue(canonical, out _dataSource) && _dataSource.NeedsRefresh()) - { - _dataSource.Refresh(); - // The pool was found, but only under the canonical key - we're using a different version - // for the first time. Map it via our own key for next time. - _dataSource = PoolManager.Pools.GetOrAdd(_connectionString, _dataSource); - return; - } - // Really unseen, need to create a new pool // The canonical pool is the 'base' pool so we need to set that up first. If someone beats us to it use what they put. // The connection string pool can either be added here or above, if it's added above we should just use that. From 8a0cb72533b2ff25b6fedf4979a6f6a86f31daa2 Mon Sep 17 00:00:00 2001 From: Sfurti-yb Date: Thu, 4 Dec 2025 09:45:43 +0530 Subject: [PATCH 2/2] Optimised the code changes --- src/Npgsql/NpgsqlConnection.cs | 37 +++++++++++++--------------------- 1 file changed, 14 insertions(+), 23 deletions(-) diff --git a/src/Npgsql/NpgsqlConnection.cs b/src/Npgsql/NpgsqlConnection.cs index 205b93a4e3..4cf806db81 100644 --- a/src/Npgsql/NpgsqlConnection.cs +++ b/src/Npgsql/NpgsqlConnection.cs @@ -175,19 +175,16 @@ void SetupDataSource() lock (SetupDataSourceLock) { - if (PoolManager.Pools.TryGetValue(_connectionString, out _dataSource) && _dataSource.NeedsRefresh()) + if (PoolManager.Pools.TryGetValue(_connectionString, out _dataSource)) { - _dataSource.Refresh(); + if (_dataSource.NeedsRefresh()) + { + _dataSource.Refresh(); + } Settings = _dataSource.Settings; // Great, we already have a pool return; } } - // Fast path: a pool already corresponds to this exact version of the connection string. - if (PoolManager.Pools.TryGetValue(_connectionString, out _dataSource) && !_dataSource.NeedsRefresh()) - { - Settings = _dataSource.Settings; // Great, we already have a pool - return; - } // Connection string hasn't been seen before. Check for empty and parse (slow one-time path). @@ -210,9 +207,16 @@ void SetupDataSource() lock (SetupDataSourceLock) { - if (PoolManager.Pools.TryGetValue(canonical, out _dataSource) && _dataSource.NeedsRefresh()) + if (PoolManager.Pools.TryGetValue(canonical, out _dataSource)) { - _dataSource.Refresh(); + if (_dataSource.NeedsRefresh()) + { + _dataSource.Refresh(); + } + // If this is a multi-host data source and the user specified a TargetSessionAttributes, create a wrapper in front of the + // MultiHostDataSource with that TargetSessionAttributes. + if (_dataSource is NpgsqlMultiHostDataSource multiHostDataSource && settings.TargetSessionAttributesParsed.HasValue) + _dataSource = multiHostDataSource.WithTargetSession(settings.TargetSessionAttributesParsed.Value); // The pool was found, but only under the canonical key - we're using a different version // for the first time. Map it via our own key for next time. _dataSource = PoolManager.Pools.GetOrAdd(_connectionString, _dataSource); @@ -220,19 +224,6 @@ void SetupDataSource() } } - if (PoolManager.Pools.TryGetValue(canonical, out _dataSource) && !_dataSource.NeedsRefresh()) - { - // If this is a multi-host data source and the user specified a TargetSessionAttributes, create a wrapper in front of the - // MultiHostDataSource with that TargetSessionAttributes. - if (_dataSource is NpgsqlMultiHostDataSource multiHostDataSource && settings.TargetSessionAttributesParsed.HasValue) - _dataSource = multiHostDataSource.WithTargetSession(settings.TargetSessionAttributesParsed.Value); - - // The pool was found, but only under the canonical key - we're using a different version - // for the first time. Map it via our own key for next time. - _dataSource = PoolManager.Pools.GetOrAdd(_connectionString, _dataSource); - return; - } - // Really unseen, need to create a new pool // The canonical pool is the 'base' pool so we need to set that up first. If someone beats us to it use what they put. // The connection string pool can either be added here or above, if it's added above we should just use that.