Skip to content

Commit deccaaf

Browse files
mdaiglemdaigle
committed
Doc comment cleanup. Add unit tests for ConnectionPoolSlots.
1 parent 1f0e170 commit deccaaf

File tree

5 files changed

+663
-41
lines changed

5 files changed

+663
-41
lines changed

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPool.cs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -532,13 +532,13 @@ private void PrepareConnection(DbConnection owningObject, DbConnectionInternal c
532532
//TODO: pass through transaction
533533
connection.ActivateConnection(null);
534534
}
535-
catch
535+
catch (Exception e)
536536
{
537537
// At this point, the connection is "out of the pool" (the call to postpop). If we hit a transient
538538
// error anywhere along the way when enlisting the connection in the transaction, we need to get
539539
// the connection back into the pool so that it isn't leaked.
540540
ReturnInternalConnection(connection, owningObject);
541-
throw;
541+
throw new Exception("Failed to activate connection", e);
542542
}
543543
}
544544

‎src/Microsoft.Data.SqlClient/src/Microsoft/Data/SqlClient/ConnectionPool/ConnectionPoolSlots.cs‎

Lines changed: 30 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -5,47 +5,21 @@
55
using System;
66
using System.Diagnostics;
77
using System.Threading;
8-
using System.Threading.Channels;
9-
using System.Threading.Tasks;
108
using Microsoft.Data.ProviderBase;
119

1210
#nullable enable
1311

1412
namespace Microsoft.Data.SqlClient.ConnectionPool
1513
{
1614
/// <summary>
17-
/// A thread-safe collection with a fixed capacity that allows reservations.
18-
/// A reservation *must* be made before adding a connection to the collection.
19-
/// Exceptions *must* be handled by the caller when trying to add connections
20-
/// and the caller *must* release the reservation.
15+
/// A thread-safe collection with a fixed capacity. Avoids wasted work by reserving a slot before adding an item.
2116
/// </summary>
22-
/// <example>
23-
/// <code>
24-
/// ConnectionPoolSlots slots = new ConnectionPoolSlots(100);
25-
///
26-
/// if (slots.TryReserve())
27-
/// {
28-
/// try {
29-
/// var connection = OpenConnection();
30-
/// slots.Add(connection);
31-
/// }
32-
/// catch (InvalidOperationException ex)
33-
/// {
34-
/// slots.ReleaseReservation();
35-
/// throw;
36-
/// }
37-
/// }
38-
///
39-
/// if (slots.TryRemove())
40-
/// {
41-
/// slots.ReleaseReservation();
42-
/// }
43-
///
44-
/// </code>
45-
/// </example>
4617
internal sealed class ConnectionPoolSlots
4718
{
48-
19+
/// <summary>
20+
/// Represents a reservation that manages a resource and ensures cleanup when no longer needed.
21+
/// </summary>
22+
/// <typeparam name="T">The type of the resource being managed by the reservation.</typeparam>
4923
private sealed class Reservation<T> : IDisposable
5024
{
5125
private Action<T> cleanupCallback;
@@ -91,29 +65,46 @@ internal void Keep()
9165
/// Constructs a ConnectionPoolSlots instance with the given fixed capacity.
9266
/// </summary>
9367
/// <param name="fixedCapacity">The fixed capacity of the collection.</param>
68+
/// <exception cref="ArgumentOutOfRangeException">
69+
/// Thrown when fixedCapacity is greater than Int32.MaxValue or equal to zero.
70+
/// </exception>
9471
internal ConnectionPoolSlots(uint fixedCapacity)
9572
{
73+
if (fixedCapacity > int.MaxValue)
74+
{
75+
throw new ArgumentOutOfRangeException(nameof(fixedCapacity), "Capacity must be less than or equal to Int32.MaxValue.");
76+
}
77+
78+
if (fixedCapacity == 0)
79+
{
80+
throw new ArgumentOutOfRangeException(nameof(fixedCapacity), "Capacity must be greater than zero.");
81+
}
82+
9683
_capacity = fixedCapacity;
9784
_reservations = 0;
9885
_connections = new DbConnectionInternal?[fixedCapacity];
9986
}
10087

10188
/// <summary>
102-
/// Gets the total number of reservations.
89+
/// Gets the total number of reservations currently held.
10390
/// </summary>
10491
internal int ReservationCount => _reservations;
10592

10693
/// <summary>
107-
/// Adds a connection to the collection. Can only be called after a reservation has been made.
94+
/// Adds a connection to the collection.
10895
/// </summary>
109-
/// <param name="createCallback">The connection to add to the collection.</param>
110-
/// <param name="cleanupCallback">Callback to clean up resources if an exception occurs.</param>
96+
/// <param name="createCallback">Callback that provides the connection to add to the collection. This callback
97+
/// *must not* call any other ConnectionPoolSlots methods.</param>
98+
/// <param name="cleanupCallback">Callback to clean up resources if an exception occurs. This callback *must
99+
/// not* call any other ConnectionPoolSlots methods.</param>
111100
/// <param name="createState">State made available to the create callback.</param>
112101
/// <param name="cleanupState">State made available to the cleanup callback.</param>
113-
/// <exception cref="InvalidOperationException">
114-
/// Thrown when unable to find an empty slot.
115-
/// This can occur if a reservation is not taken before adding a connection.
102+
/// <exception cref="Exception">
103+
/// Throws when createCallback throws an exception.
104+
/// Throws when a reservation is successfully made, but an empty slot cannot be found. This condition is
105+
/// unexpected and indicates a bug.
116106
/// </exception>
107+
/// <returns>Returns the new connection, or null if there was not available space.</returns>
117108
internal DbConnectionInternal? Add<T, S>(
118109
CreateCallback<DbConnectionInternal?, T> createCallback,
119110
CleanupCallback<S> cleanupCallback,
@@ -186,7 +177,7 @@ internal bool TryRemove(DbConnectionInternal connection)
186177
/// <summary>
187178
/// Attempts to reserve a spot in the collection.
188179
/// </summary>
189-
/// <returns>True if a reservation was successfully obtained.</returns>
180+
/// <returns>A Reservation if successful, otherwise returns null.</returns>
190181
private Reservation<ConnectionPoolSlots>? TryReserve()
191182
{
192183
for (var expected = _reservations; expected < _capacity; expected = _reservations)

‎src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft.Data.SqlClient.UnitTests.csproj‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,4 +48,7 @@
4848
<TargetPath>xunit.runner.json</TargetPath>
4949
</ContentWithTargetPath>
5050
</ItemGroup>
51+
<ItemGroup>
52+
<Folder Include="microsoft.data.sqlclient\tests\unittests\microsoft\data\sqlclient\connectionpool\" />
53+
</ItemGroup>
5154
</Project>

‎src/Microsoft.Data.SqlClient/tests/UnitTests/Microsoft/Data/SqlClient/ConnectionPool/ChannelDbConnectionPoolTest.cs‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -911,5 +911,123 @@ protected override void Deactivate()
911911
#endregion
912912
}
913913
#endregion
914+
915+
[Fact]
916+
public void Constructor_WithZeroMaxPoolSize_ThrowsArgumentOutOfRangeException()
917+
{
918+
// Arrange
919+
var poolGroupOptions = new DbConnectionPoolGroupOptions(
920+
poolByIdentity: false,
921+
minPoolSize: 0,
922+
maxPoolSize: 0, // This should cause an exception
923+
creationTimeout: 15,
924+
loadBalanceTimeout: 0,
925+
hasTransactionAffinity: true
926+
);
927+
var dbConnectionPoolGroup = new DbConnectionPoolGroup(
928+
new DbConnectionOptions("DataSource=localhost;", null),
929+
new DbConnectionPoolKey("TestDataSource"),
930+
poolGroupOptions
931+
);
932+
933+
// Act & Assert
934+
var exception = Assert.Throws<ArgumentOutOfRangeException>(() =>
935+
new ChannelDbConnectionPool(
936+
SuccessfulConnectionFactory,
937+
dbConnectionPoolGroup,
938+
DbConnectionPoolIdentity.NoIdentity,
939+
new DbConnectionPoolProviderInfo()
940+
));
941+
942+
Assert.Equal("fixedCapacity", exception.ParamName);
943+
Assert.Contains("Capacity must be greater than zero", exception.Message);
944+
}
945+
946+
[Fact]
947+
public void Constructor_WithMaxIntMaxPoolSize_DoesNotThrow()
948+
{
949+
// Arrange - Test that Int32.MaxValue is accepted as a valid pool size
950+
var poolGroupOptions = new DbConnectionPoolGroupOptions(
951+
poolByIdentity: false,
952+
minPoolSize: 0,
953+
maxPoolSize: int.MaxValue,
954+
creationTimeout: 15,
955+
loadBalanceTimeout: 0,
956+
hasTransactionAffinity: true
957+
);
958+
var dbConnectionPoolGroup = new DbConnectionPoolGroup(
959+
new DbConnectionOptions("DataSource=localhost;", null),
960+
new DbConnectionPoolKey("TestDataSource"),
961+
poolGroupOptions
962+
);
963+
964+
// Act & Assert - This should not throw
965+
var pool = new ChannelDbConnectionPool(
966+
SuccessfulConnectionFactory,
967+
dbConnectionPoolGroup,
968+
DbConnectionPoolIdentity.NoIdentity,
969+
new DbConnectionPoolProviderInfo()
970+
);
971+
972+
Assert.NotNull(pool);
973+
Assert.Equal(0, pool.Count);
974+
}
975+
976+
[Fact]
977+
public void Constructor_WithValidSmallPoolSizes_WorksCorrectly()
978+
{
979+
// Arrange - Test various small pool sizes that should work correctly
980+
981+
// Test with pool size of 1
982+
var poolGroupOptions1 = new DbConnectionPoolGroupOptions(
983+
poolByIdentity: false,
984+
minPoolSize: 0,
985+
maxPoolSize: 1,
986+
creationTimeout: 15,
987+
loadBalanceTimeout: 0,
988+
hasTransactionAffinity: true
989+
);
990+
var dbConnectionPoolGroup1 = new DbConnectionPoolGroup(
991+
new DbConnectionOptions("DataSource=localhost;", null),
992+
new DbConnectionPoolKey("TestDataSource"),
993+
poolGroupOptions1
994+
);
995+
996+
// Act & Assert - Pool size of 1 should work
997+
var pool1 = new ChannelDbConnectionPool(
998+
SuccessfulConnectionFactory,
999+
dbConnectionPoolGroup1,
1000+
DbConnectionPoolIdentity.NoIdentity,
1001+
new DbConnectionPoolProviderInfo()
1002+
);
1003+
1004+
Assert.NotNull(pool1);
1005+
Assert.Equal(0, pool1.Count);
1006+
1007+
// Test with pool size of 2
1008+
var poolGroupOptions2 = new DbConnectionPoolGroupOptions(
1009+
poolByIdentity: false,
1010+
minPoolSize: 0,
1011+
maxPoolSize: 2,
1012+
creationTimeout: 15,
1013+
loadBalanceTimeout: 0,
1014+
hasTransactionAffinity: true
1015+
);
1016+
var dbConnectionPoolGroup2 = new DbConnectionPoolGroup(
1017+
new DbConnectionOptions("DataSource=localhost;", null),
1018+
new DbConnectionPoolKey("TestDataSource"),
1019+
poolGroupOptions2
1020+
);
1021+
1022+
var pool2 = new ChannelDbConnectionPool(
1023+
SuccessfulConnectionFactory,
1024+
dbConnectionPoolGroup2,
1025+
DbConnectionPoolIdentity.NoIdentity,
1026+
new DbConnectionPoolProviderInfo()
1027+
);
1028+
1029+
Assert.NotNull(pool2);
1030+
Assert.Equal(0, pool2.Count);
1031+
}
9141032
}
9151033
}

0 commit comments

Comments
 (0)