try to improve Npgsql design:

- make NpgSqlHealthCheckOptions.DataSource internal, so the users can't make mistakes by assigning it to a new data source for every health check invocation
- make sure not more than a single DataSource is created
- when the user provides ConnectionString, try to resolve DataSource just once. If it's present and the connection string is the same, reuse it

src/HealthChecks.NpgSql/DependencyInjection/NpgSqlHealthCheckBuilderExtensions.cs

@@ -37,7 +37,13 @@ public static IHealthChecksBuilder AddNpgSql(
         IEnumerable<string>? tags = default,
         TimeSpan? timeout = default)
     {
-        return builder.AddNpgSql(_ => connectionString, healthQuery, configure, name, failureStatus, tags, timeout);
+        Guard.ThrowIfNull(connectionString, throwOnEmptyString: true);
+
+        return builder.AddNpgSql(new NpgSqlHealthCheckOptions(connectionString)
+        {
+            CommandText = healthQuery,
+            Configure = configure
+        }, name, failureStatus, tags, timeout);
     }
 
     /// <summary>
@@ -65,14 +71,36 @@ public static IHealthChecksBuilder AddNpgSql(
         IEnumerable<string>? tags = default,
         TimeSpan? timeout = default)
     {
-        return builder.AddNpgSql(sp => new NpgsqlDataSourceBuilder(connectionStringFactory(sp)).Build(), healthQuery, configure, name, failureStatus, tags, timeout);
+        // This instance is captured in lambda closure, so it can be reused (perf)
+        NpgSqlHealthCheckOptions options = new()
+        {
+            CommandText = healthQuery,
+            Configure = configure,
+        };
+
+        return builder.Add(new HealthCheckRegistration(
+            name ?? NAME,
+            sp =>
+            {
+                options.ConnectionString ??= Guard.ThrowIfNull(connectionStringFactory.Invoke(sp), throwOnEmptyString: true, paramName: nameof(connectionStringFactory));
+
+                ResolveDataSourceIfPossible(options, sp);
+
+                return new NpgSqlHealthCheck(options);
+            },
+            failureStatus,
+            tags,
+            timeout));
     }
 
     /// <summary>
     /// Add a health check for Postgres databases.
     /// </summary>
     /// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param>
-    /// <param name="dbDataSourceFactory">A factory to build the NpgsqlDataSource to use.</param>
+    /// <param name="dbDataSourceFactory">
+    /// An optional factory to obtain <see cref="NpgsqlDataSource" /> instance.
+    /// When not provided, <see cref="NpgsqlDataSource" /> is simply resolved from <see cref="IServiceProvider"/>.
+    /// </param>
     /// <param name="healthQuery">The query to be used in check.</param>
     /// <param name="configure">An optional action to allow additional Npgsql specific configuration.</param>
     /// <param name="name">The health check name. Optional. If <c>null</c> the type name 'npgsql' will be used for the name.</param>
@@ -85,17 +113,15 @@ public static IHealthChecksBuilder AddNpgSql(
     /// <returns>The specified <paramref name="builder"/>.</returns>
     public static IHealthChecksBuilder AddNpgSql(
         this IHealthChecksBuilder builder,
-        Func<IServiceProvider, NpgsqlDataSource> dbDataSourceFactory,
+        Func<IServiceProvider, NpgsqlDataSource>? dbDataSourceFactory = null,
         string healthQuery = HEALTH_QUERY,
         Action<NpgsqlConnection>? configure = null,
         string? name = default,
         HealthStatus? failureStatus = default,
         IEnumerable<string>? tags = default,
         TimeSpan? timeout = default)
     {
-        Guard.ThrowIfNull(dbDataSourceFactory);
-
-        NpgsqlDataSource? dataSource = null;
+        // This instance is captured in lambda closure, so it can be reused (perf)
         NpgSqlHealthCheckOptions options = new()
         {
             CommandText = healthQuery,
@@ -106,47 +132,15 @@ public static IHealthChecksBuilder AddNpgSql(
             name ?? NAME,
             sp =>
             {
-                // The Data Source needs to be created only once,
-                // as each instance has it's own connection pool.
-                // See https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/issues/1993 for more details.
-
-                // Perform an atomic read of the current value.
-                NpgsqlDataSource? existingDataSource = Volatile.Read(ref dataSource);
-                if (existingDataSource is null)
+                if (options.DataSource is null)
                 {
-                    // Create a new Data Source
-                    NpgsqlDataSource fromFactory = dbDataSourceFactory(sp);
-                    // Try to resolve the Data Source from DI.
-                    NpgsqlDataSource? fromDI = sp.GetService<NpgsqlDataSource>();
-
-                    if (fromDI is not null && fromDI.ConnectionString.Equals(fromFactory.ConnectionString))
+                    // this lock will most likely be obtained once, with no contention so the cost should be low and acceptable
+                    lock (options)
                     {
-                        // If they are using the same ConnectionString, we can reuse the instance from DI.
-                        // So there is only ONE NpgsqlDataSource per the whole app and ONE connection pool.
-
-                        if (!ReferenceEquals(fromDI, fromFactory))
-                        {
-                            // Dispose it, as long as it's not the same instance.
-                            fromFactory.Dispose();
-                        }
-                        Interlocked.Exchange(ref dataSource, fromDI);
-                        options.DataSource = fromDI;
-                    }
-                    else
-                    {
-                        // Perform an atomic exchange, but only if the value is still null.
-                        existingDataSource = Interlocked.CompareExchange(ref dataSource, fromFactory, null);
-                        if (existingDataSource is not null)
-                        {
-                            // Some other thread has created the data source in the meantime,
-                            // we dispose our own copy, and use the existing instance.
-                            fromFactory.Dispose();
-                            options.DataSource = existingDataSource;
-                        }
-                        else
-                        {
-                            options.DataSource = fromFactory;
-                        }
+                        // The Data Source needs to be created only once,
+                        // as each instance has it's own connection pool.
+                        // See https://github.com/Xabaril/AspNetCore.Diagnostics.HealthChecks/issues/1993 for more details.
+                        options.DataSource ??= dbDataSourceFactory?.Invoke(sp) ?? sp.GetRequiredService<NpgsqlDataSource>();
                     }
                 }
 
@@ -161,7 +155,7 @@ public static IHealthChecksBuilder AddNpgSql(
     /// Add a health check for Postgres databases.
     /// </summary>
     /// <param name="builder">The <see cref="IHealthChecksBuilder"/>.</param>
-    /// <param name="options">Options for health check.</param>
+    /// <param name="options">Options for health check. It's mandatory to provide <see cref="NpgSqlHealthCheckOptions.ConnectionString"/>.</param>
     /// <param name="name">The health check name. Optional. If <c>null</c> the type name 'npgsql' will be used for the name.</param>
     /// <param name="failureStatus">
     /// The <see cref="HealthStatus"/> that should be reported when the health check fails. Optional. If <c>null</c> then
@@ -179,12 +173,32 @@ public static IHealthChecksBuilder AddNpgSql(
         TimeSpan? timeout = default)
     {
         Guard.ThrowIfNull(options);
+        Guard.ThrowIfNull(options.ConnectionString, throwOnEmptyString: true, paramName: "ConnectionString");
 
         return builder.Add(new HealthCheckRegistration(
             name ?? NAME,
-            _ => new NpgSqlHealthCheck(options),
+            sp =>
+            {
+                ResolveDataSourceIfPossible(options, sp);
+
+                return new NpgSqlHealthCheck(options);
+            },
             failureStatus,
             tags,
             timeout));
     }
+
+    private static void ResolveDataSourceIfPossible(NpgSqlHealthCheckOptions options, IServiceProvider sp)
+    {
+        if (options.DataSource is null && !options.TriedToResolveFromDI)
+        {
+            NpgsqlDataSource? fromDi = sp.GetService<NpgsqlDataSource>();
+            if (fromDi?.ConnectionString == options.ConnectionString)
+            {
+                // When it's possible, we reuse the DataSource registered in the DI
+                options.DataSource = fromDi;
+            }
+            options.TriedToResolveFromDI = true; // save the answer, so we don't do it more than once
+        }
+    }
 }

src/HealthChecks.NpgSql/NpgSqlHealthCheck.cs

@@ -1,4 +1,6 @@
+using System.Diagnostics;
 using Microsoft.Extensions.Diagnostics.HealthChecks;
+using Npgsql;
 
 namespace HealthChecks.NpgSql;
 
@@ -11,7 +13,7 @@ public class NpgSqlHealthCheck : IHealthCheck
 
     public NpgSqlHealthCheck(NpgSqlHealthCheckOptions options)
     {
-        Guard.ThrowIfNull(options.DataSource);
+        Debug.Assert(options.ConnectionString is not null || options.DataSource is not null);
         Guard.ThrowIfNull(options.CommandText, true);
         _options = options;
     }
@@ -21,7 +23,9 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context
     {
         try
         {
-            await using var connection = _options.DataSource!.CreateConnection();
+            await using var connection = _options.DataSource is not null
+                ? _options.DataSource.CreateConnection()
+                : new NpgsqlConnection(_options.ConnectionString);
 
             _options.Configure?.Invoke(connection);
             await connection.OpenAsync(cancellationToken).ConfigureAwait(false);
@@ -33,7 +37,6 @@ public async Task<HealthCheckResult> CheckHealthAsync(HealthCheckContext context
             return _options.HealthCheckResultBuilder == null
                 ? HealthCheckResult.Healthy()
                 : _options.HealthCheckResultBuilder(result);
-
         }
         catch (Exception ex)
         {

src/HealthChecks.NpgSql/NpgSqlHealthCheckOptions.cs

@@ -9,20 +9,34 @@ namespace HealthChecks.NpgSql;
 /// </summary>
 public class NpgSqlHealthCheckOptions
 {
+    internal NpgSqlHealthCheckOptions()
+    {
+        // this ctor is internal on purpose:
+        // those who want to use DataSource need to use the extension methods
+        // that take care of creating the right thing 
+    }
+
     /// <summary>
-    /// The Postgres connection string to be used.
-    /// Use <see cref="DataSource"/> property for advanced configuration.
+    /// Creates an instance of <see cref="NpgSqlHealthCheckOptions"/>.
     /// </summary>
-    public string? ConnectionString
+    /// <param name="connectionString">The PostgreSQL connection string to be used.</param>
+    public NpgSqlHealthCheckOptions(string connectionString)
     {
-        get => DataSource?.ConnectionString;
-        set => DataSource = value is not null ? NpgsqlDataSource.Create(value) : null;
+        ConnectionString = Guard.ThrowIfNull(connectionString, throwOnEmptyString: true);
     }
 
+    /// <summary>
+    /// The Postgres connection string to be used.
+    /// Use <see cref="DataSource"/> property for advanced configuration.
+    /// </summary>
+    public string? ConnectionString { get; set; }
+
     /// <summary>
     /// The Postgres data source to be used.
     /// </summary>
-    public NpgsqlDataSource? DataSource { get; set; }
+    internal NpgsqlDataSource? DataSource { get; set; }
+
+    internal bool TriedToResolveFromDI { get; set; }
 
     /// <summary>
     /// The query to be executed.

test/HealthChecks.Npgsql.Tests/DependencyInjection/RegistrationTests.cs

@@ -96,7 +96,7 @@ public void factory_is_called_only_once()
     [InlineData(false)]
     public void factory_reuses_pre_registered_datasource_when_possible(bool sameConnectionString)
     {
-        const string connectionString = "Server=localhost";
+        const string connectionString = "Host=localhost";
         ServiceCollection services = new();
 
         services.AddSingleton<NpgsqlDataSource>(serviceProvider =>
@@ -122,10 +122,13 @@ public void factory_reuses_pre_registered_datasource_when_possible(bool sameConn
         for (int i = 0; i < 10; i++)
         {
             var healthCheck = (NpgSqlHealthCheck)registration.Factory(serviceProvider);
-            var fieldInfo = typeof(NpgSqlHealthCheck).GetField("_options", BindingFlags.Instance | BindingFlags.NonPublic);
-            var npgSqlHealthCheckOptions = (NpgSqlHealthCheckOptions)fieldInfo!.GetValue(healthCheck)!;
+            var optionsField = typeof(NpgSqlHealthCheck).GetField("_options", BindingFlags.Instance | BindingFlags.NonPublic);
+            var npgSqlHealthCheckOptions = (NpgSqlHealthCheckOptions)optionsField!.GetValue(healthCheck)!;
 
-            Assert.Equal(sameConnectionString, ReferenceEquals(serviceProvider.GetRequiredService<NpgsqlDataSource>(), npgSqlHealthCheckOptions.DataSource));
+            var dataSourceProperty = typeof(NpgSqlHealthCheckOptions).GetProperty("DataSource", BindingFlags.Instance | BindingFlags.NonPublic);
+            var dataSource = (NpgsqlDataSource)dataSourceProperty!.GetValue(npgSqlHealthCheckOptions)!;
+
+            Assert.Equal(sameConnectionString, ReferenceEquals(serviceProvider.GetRequiredService<NpgsqlDataSource>(), dataSource));
         }
 
         factoryCalls.ShouldBe(1);

test/HealthChecks.Npgsql.Tests/HealthChecks.NpgSql.approved.txt

@@ -7,11 +7,10 @@ namespace HealthChecks.NpgSql
     }
     public class NpgSqlHealthCheckOptions
     {
-        public NpgSqlHealthCheckOptions() { }
+        public NpgSqlHealthCheckOptions(string connectionString) { }
         public string CommandText { get; set; }
         public System.Action<Npgsql.NpgsqlConnection>? Configure { get; set; }
         public string? ConnectionString { get; set; }
-        public Npgsql.NpgsqlDataSource? DataSource { get; set; }
         public System.Func<object?, Microsoft.Extensions.Diagnostics.HealthChecks.HealthCheckResult>? HealthCheckResultBuilder { get; set; }
     }
 }
@@ -20,8 +19,8 @@ namespace Microsoft.Extensions.DependencyInjection
     public static class NpgSqlHealthCheckBuilderExtensions
     {
         public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddNpgSql(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, HealthChecks.NpgSql.NpgSqlHealthCheckOptions options, string? name = null, Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus? failureStatus = default, System.Collections.Generic.IEnumerable<string>? tags = null, System.TimeSpan? timeout = default) { }
-        public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddNpgSql(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, System.Func<System.IServiceProvider, Npgsql.NpgsqlDataSource> dbDataSourceFactory, string healthQuery = "SELECT 1;", System.Action<Npgsql.NpgsqlConnection>? configure = null, string? name = null, Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus? failureStatus = default, System.Collections.Generic.IEnumerable<string>? tags = null, System.TimeSpan? timeout = default) { }
         public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddNpgSql(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, System.Func<System.IServiceProvider, string> connectionStringFactory, string healthQuery = "SELECT 1;", System.Action<Npgsql.NpgsqlConnection>? configure = null, string? name = null, Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus? failureStatus = default, System.Collections.Generic.IEnumerable<string>? tags = null, System.TimeSpan? timeout = default) { }
+        public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddNpgSql(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, System.Func<System.IServiceProvider, Npgsql.NpgsqlDataSource>? dbDataSourceFactory = null, string healthQuery = "SELECT 1;", System.Action<Npgsql.NpgsqlConnection>? configure = null, string? name = null, Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus? failureStatus = default, System.Collections.Generic.IEnumerable<string>? tags = null, System.TimeSpan? timeout = default) { }
         public static Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder AddNpgSql(this Microsoft.Extensions.DependencyInjection.IHealthChecksBuilder builder, string connectionString, string healthQuery = "SELECT 1;", System.Action<Npgsql.NpgsqlConnection>? configure = null, string? name = null, Microsoft.Extensions.Diagnostics.HealthChecks.HealthStatus? failureStatus = default, System.Collections.Generic.IEnumerable<string>? tags = null, System.TimeSpan? timeout = default) { }
     }
 }