Skip to content

Commit 0163c17

Browse files
BrarBrar
committed
WIP: Feedback and fixes for failing tests.
1 parent f450288 commit 0163c17

4 files changed

Lines changed: 85 additions & 59 deletions

File tree

‎src/Npgsql/MultiplexingDataSource.cs‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,15 @@ static void CompleteWrite(NpgsqlConnector connector, ref MultiplexingStats stats
349349
// ReSharper disable once FunctionNeverReturns
350350
}
351351

352+
protected override void Dispose(bool disposing)
353+
{
354+
// Force doing the startup checks again when disposed
355+
// in order to reset the Connection's DataSource
356+
if (disposing)
357+
StartupCheckPerformed = false;
358+
base.Dispose(disposing);
359+
}
360+
352361
struct MultiplexingStats
353362
{
354363
internal Stopwatch Stopwatch;

‎src/Npgsql/NpgsqlConnection.cs‎

Lines changed: 57 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -314,26 +314,7 @@ async Task OpenAsync(bool async, CancellationToken cancellationToken)
314314
enlistToTransaction = null;
315315
}
316316
else
317-
while (true)
318-
{
319-
try
320-
{
321-
connector = await _dataSource.Get(this, timeout, async, cancellationToken);
322-
break;
323-
}
324-
// When using PoolManager (legacy code path setting NpgsqlConnection.ConnectionString), clearing a pool
325-
// removes the pool (NpgsqlDataSource) from the PoolManager and disposes it.
326-
// This leads to an ObjectDisposedException when calling NpgsqlDataSource.Get() here.
327-
// If this happens we reinitialize the pool and add it to the PoolManager by calling SetupDataSource() again.
328-
// When not using PoolManager (modern code path creating the NpgsqlConnection from a NpgsqlDataSource;
329-
// _retryDisposedDataSource is false) we bubble up an eventual ObjectDisposedException as usual.
330-
catch (ObjectDisposedException)
331-
{
332-
if (!_retryDisposedDataSource)
333-
throw;
334-
SetupDataSource();
335-
}
336-
}
317+
connector = await GetConnector(timeout, async, cancellationToken);
337318

338319
Debug.Assert(connector.Connection is null,
339320
$"Connection for opened connector '{Connector?.Id.ToString() ?? "???"}' is bound to another connection");
@@ -386,6 +367,29 @@ async Task PerformMultiplexingStartupCheck(bool async, CancellationToken cancell
386367
}
387368
}
388369
}
370+
async Task<NpgsqlConnector> GetConnector(NpgsqlTimeout timeout, bool async, CancellationToken cancellationToken)
371+
{
372+
while (true)
373+
{
374+
try
375+
{
376+
Debug.Assert(_dataSource != null);
377+
return await _dataSource.Get(this, timeout, async, cancellationToken);
378+
}
379+
// When using PoolManager (legacy code path setting NpgsqlConnection.ConnectionString), clearing a pool
380+
// removes the pool (NpgsqlDataSource) from the PoolManager and disposes it.
381+
// This leads to an ObjectDisposedException when calling NpgsqlDataSource.Get() here.
382+
// If this happens we reinitialize the pool and add it to the PoolManager by calling SetupDataSource() again.
383+
// When not using PoolManager (modern code path creating the NpgsqlConnection from a NpgsqlDataSource;
384+
// _retryDisposedDataSource is false) we bubble up an eventual ObjectDisposedException as usual.
385+
catch (ObjectDisposedException)
386+
{
387+
if (!_retryDisposedDataSource)
388+
throw;
389+
SetupDataSource();
390+
}
391+
}
392+
}
389393

390394
#endregion Open / Init
391395

@@ -1653,7 +1657,7 @@ async ValueTask<NpgsqlConnector> StartBindingScopeAsync()
16531657
Debug.Assert(Settings.Multiplexing);
16541658
Debug.Assert(_dataSource != null);
16551659

1656-
var connector = await _dataSource.Get(this, timeout, async, cancellationToken);
1660+
var connector = await GetConnector(timeout, async, cancellationToken);
16571661
Connector = connector;
16581662
connector.Connection = this;
16591663
ConnectorBindingScope = scope;
@@ -1905,32 +1909,49 @@ public void ReloadTypes()
19051909
CheckReady();
19061910

19071911
using var scope = StartTemporaryBindingScope(out var connector);
1908-
1909-
_dataSource!.Bootstrap(
1910-
connector,
1911-
NpgsqlTimeout.Infinite,
1912-
forceReload: true,
1913-
async: false,
1914-
CancellationToken.None)
1915-
.GetAwaiter().GetResult();
1912+
BootstrapDataSource(connector, async: false).GetAwaiter().GetResult();
19161913
}
19171914

19181915
/// <summary>
19191916
/// Flushes the type cache for this connection's connection string and reloads the types for this connection only.
19201917
/// Type changes will appear for other connections only after they are re-opened from the pool.
19211918
/// </summary>
1922-
public async Task ReloadTypesAsync()
1919+
public Task ReloadTypesAsync()
19231920
{
1921+
using var _ = NoSynchronizationContextScope.Enter();
19241922
CheckReady();
19251923

19261924
using var scope = StartTemporaryBindingScope(out var connector);
1925+
return BootstrapDataSource(connector, async: true);
1926+
}
19271927

1928-
await _dataSource!.Bootstrap(
1929-
connector,
1930-
NpgsqlTimeout.Infinite,
1931-
forceReload: true,
1932-
async: true,
1933-
CancellationToken.None);
1928+
async Task BootstrapDataSource(NpgsqlConnector connector, bool async)
1929+
{
1930+
while (true)
1931+
{
1932+
try
1933+
{
1934+
await _dataSource!.Bootstrap(
1935+
connector,
1936+
NpgsqlTimeout.Infinite,
1937+
forceReload: true,
1938+
async,
1939+
CancellationToken.None);
1940+
break;
1941+
}
1942+
// When using PoolManager (legacy code path setting NpgsqlConnection.ConnectionString), clearing a pool
1943+
// removes the pool (NpgsqlDataSource) from the PoolManager and disposes it.
1944+
// This leads to an ObjectDisposedException when calling NpgsqlDataSource.Get() here.
1945+
// If this happens we reinitialize the pool and add it to the PoolManager by calling SetupDataSource() again.
1946+
// When not using PoolManager (modern code path creating the NpgsqlConnection from a NpgsqlDataSource;
1947+
// _retryDisposedDataSource is false) we bubble up an eventual ObjectDisposedException as usual.
1948+
catch (ObjectDisposedException)
1949+
{
1950+
if (!_retryDisposedDataSource)
1951+
throw;
1952+
SetupDataSource();
1953+
}
1954+
}
19341955
}
19351956

19361957
/// <summary>

‎src/Npgsql/NpgsqlDataSource.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ internal async Task Bootstrap(
202202
bool async,
203203
CancellationToken cancellationToken)
204204
{
205+
CheckDisposed();
205206
if (_isBootstrapped && !forceReload)
206207
return;
207208

‎src/Npgsql/PoolManager.cs‎

Lines changed: 18 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ namespace Npgsql;
1515
/// </remarks>
1616
static class PoolManager
1717
{
18+
static readonly object LockObject = new();
1819
static ConcurrentDictionary<string, NpgsqlDataSource> _pools = new();
1920

2021
// Keep the IEnumerable API for testing purposes but remove the rest to avoid careless (unlocked) access to the
@@ -23,41 +24,35 @@ static class PoolManager
2324

2425
internal static void Clear(string connString)
2526
{
26-
// ReSharper disable once InconsistentlySynchronizedField
27-
var pools = _pools;
28-
if (!pools.TryRemove(connString, out var pool))
29-
return;
27+
lock (LockObject)
28+
{
29+
var pools = _pools;
30+
if (!pools.TryRemove(connString, out var pool))
31+
return;
3032

31-
pool.Dispose();
33+
pool.Dispose();
3234

33-
// The same NpgsqlDataSource instance may be referenced via different connection strings
34-
// in our dictionary, so we have to make sure that we remove all occurrences
35-
#if NET5_0_OR_GREATER
36-
foreach (var pair in pools)
37-
if (ReferenceEquals(pair.Value, pool))
38-
pools.TryRemove(pair);
39-
#else
40-
// Make sure nobody adds back a new pool with the same connection string
41-
// as one we've already removed.
42-
lock (pools)
43-
{
35+
// The same NpgsqlDataSource instance may be referenced via different connection strings
36+
// in our dictionary, so we have to make sure that we remove all occurrences
4437
foreach (var pair in pools)
4538
if (ReferenceEquals(pair.Value, pool))
39+
#if NET5_0_OR_GREATER
40+
pools.TryRemove(pair);
41+
#else
4642
pools.TryRemove(pair.Key, out _);
47-
}
4843
#endif
44+
}
4945
}
5046

5147
internal static void ClearAll()
5248
{
53-
ConcurrentDictionary<string, NpgsqlDataSource> pools;
54-
lock (_pools)
49+
lock (LockObject)
5550
{
56-
pools = _pools;
51+
var pools = _pools;
5752
_pools = new();
53+
foreach (var pool in pools.Values)
54+
pool.Dispose();
5855
}
59-
foreach (var pool in pools.Values)
60-
pool.Dispose();
6156
}
6257

6358
static PoolManager()
@@ -71,7 +66,7 @@ static PoolManager()
7166
[MethodImpl(MethodImplOptions.AggressiveInlining)]
7267
public static NpgsqlDataSource GetOrAddPool(string connectionString, NpgsqlDataSource dataSource)
7368
{
74-
lock (_pools)
69+
lock (LockObject)
7570
{
7671
return _pools.GetOrAdd(connectionString, dataSource);
7772
}

0 commit comments

Comments
 (0)