CSHARP-6029: Make ICoreServerSessionPool disposable and send endSessions on dispose by sanych-sun · Pull Request #1995 · mongodb/mongo-csharp-driver

sanych-sun · 2026-05-09T00:26:42Z

No description provided.

Copilot

Pull request overview

This PR aims to ensure server-side sessions are explicitly ended when a client/cluster is disposed (via an endSessions command), and adjusts test event capture behavior to avoid capturing disposal-time commands.

Changes:

Added session-pool draining logic that issues endSessions for pooled server sessions during cluster disposal.
Introduced EventCapturer.Stop() and invoked it from the JSON-driven spec test runner.
Added a shared helper on Cluster to select an available server and trigger session-pool cleanup during disposal.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
tests/MongoDB.Driver.Tests/Specifications/Runner/MongoClientJsonDrivenTestRunnerBase.cs	Stops event capture after test operations complete.
tests/MongoDB.Driver.TestHelpers/Core/EventCapturer.cs	Adds a stop flag to disable capturing after disposal begins.
src/MongoDB.Driver/Core/Clusters/SingleServerCluster.cs	Triggers session-pool cleanup before server removal on dispose.
src/MongoDB.Driver/Core/Clusters/MultiServerCluster.cs	Triggers session-pool cleanup during cluster disposal.
src/MongoDB.Driver/Core/Clusters/Cluster.cs	Adds `ReleaseServerSessionPool()` helper to drive cleanup.
src/MongoDB.Driver/Core/Bindings/ICoreServerSessionPool.cs	Extends pool contract with `ReleaseAll(IServer)`.
src/MongoDB.Driver/Core/Bindings/CoreServerSessionPool.cs	Implements `ReleaseAll` to drain pooled sessions and send `endSessions`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+                        }
+
+                        sessionsBatch = _pool.GetRange(0, Math.Min(_pool.Count, 9999));
+                        _pool.RemoveRange(0, sessionsBatch.Count);
+                    }


+        public void ReleaseAll(IServer server)
+        {
+            if (_pool.Count == 0)
+            {
+                return;
+            }
+
+            try
+            {
+                while (true)
+                {
+                    List<ICoreServerSession> sessionsBatch;
+                    lock (_lock)
+                    {
+                        if (_pool.Count == 0)
+                        {
+                            return;
+                        }
+
+                        sessionsBatch = _pool.GetRange(0, Math.Min(_pool.Count, 9999));
+                        _pool.RemoveRange(0, sessionsBatch.Count);
+                    }
+
+                    var endSessionCommand = new BsonDocument("endSessions", new BsonArray(sessionsBatch.Select(s => s.Id)));
+                    foreach (var session in sessionsBatch)
+                    {
+                        session.Dispose();
+                    }
+
+                    var operationContext = OperationContext.NoTimeout;
+                    using var channel = server.GetChannel(operationContext);
+                    channel.Command(
+                        operationContext,
+                        NoCoreSession.Instance,
+                        ReadPreference.PrimaryPreferred,
+                        DatabaseNamespace.Admin,
+                        endSessionCommand,
+                        null,
+                        NoOpElementNameValidator.Instance,
+                        null,
+                        null,
+                        CommandResponseHandling.Return,
+                        BsonDocumentSerializer.Instance,
+                        null);
+                }
+            }
+            catch
+            {
+                // Ignore any errors.
+            }
+        }


+internal interface ICoreServerSessionPool
 {
-    internal interface ICoreServerSessionPool
-    {
-        ICoreServerSession AcquireSession();
-        void ReleaseSession(ICoreServerSession serverSession);
-    }
+    ICoreServerSession AcquireSession();
+    void ReleaseSession(ICoreServerSession serverSession);
+    void ReleaseAll(IServer server);
 }


…ons on dispose

sanych-sun · 2026-05-22T03:15:17Z

-        private readonly object _lock = new object();
-        private readonly List<ICoreServerSession> _pool = new List<ICoreServerSession>();
+        private readonly ILogger<LogCategories.Client> _logger;
+        private readonly ConcurrentStack<ICoreServerSession> _pool = new();


Accordingly to spec pool should be LIFO.

BorisDog · 2026-05-22T16:01:05Z

        public void ReleaseSession(ICoreServerSession session)
        {
-            lock (_lock)
+            ThrowIfDisposed();


Is this be a behavioral breaking change? In the case server is disposed but the session/operation in flight is not, the exception might happen even for a successful operation?

It is, but if MongoClient/Cluster is being disposed - all resources associated with it should be cleaned up. Otherwise we might let cluster "resurrect" and have leaked resources.

Yes the resources should be clean up.
But should the pool throw on the "after-cleanup release attempt"?

If somebody disposed client/cluster and then suddenly started returning sessions to the pool: it means a) they disposed cluster too early b) the returned sessions might not be closed properly (if CloseAndDispose is already done).

As it was discussed offline we will log an error for now to prevent any possible breaking change in the minor release.

BorisDog · 2026-05-22T16:34:03Z

+                }
+                else
+                {
+                    Interlocked.Increment(ref _sessionsAcquired);


nit: consider having a single place for incrementing for readability (single return)

BorisDog · 2026-05-22T16:39:28Z

+            {
+                _logger?.LogDebug(
+                    "Closed server session pool for {clusterId} in {milliseconds}ms.",
+                    _cluster.ClusterId, (Stopwatch.GetTimestamp() - timestamp) / (double)Stopwatch.Frequency * 1000);


Add how many session where disposed (out of initial)?

BorisDog · 2026-05-22T17:30:47Z

            }
+            catch(Exception ex)
+            {
+                _logger?.LogError(ex, "Error closing server session pool for {clusterId}: {exception}", _cluster.ClusterId, ex);


Do we need to pass ex as an message formatting argument as well?

It's there, the last parameter.

Yes, but do we need it twice?

Oh, I see. Will remove.

I still think passing ex.Message is redundant. Just ex as a first parameter is enough, as logger sink will handle all the formatting needed.

Ok. Will remove.

BorisDog · 2026-05-22T18:00:18Z

        public void ReleaseSession(ICoreServerSession session)
        {
-            lock (_lock)
+            ThrowIfDisposed();


Yes the resources should be clean up.
But should the pool throw on the "after-cleanup release attempt"?

BorisDog

LGTM + minor comments.

BorisDog · 2026-05-25T18:04:47Z

            }
+            catch(Exception ex)
+            {
+                _logger?.LogError(ex, "Error closing server session pool for {clusterId}: {exception}", _cluster.ClusterId, ex);


I still think passing ex.Message is redundant. Just ex as a first parameter is enough, as logger sink will handle all the formatting needed.

BorisDog · 2026-05-25T18:06:23Z

        {
-            lock (_lock)
+            if (_isDisposed)
+            {


Dispose the session object?

Copilot AI review requested due to automatic review settings May 9, 2026 00:26

sanych-sun requested a review from a team as a code owner May 9, 2026 00:26

sanych-sun added the bug Fixes issues or unintended behavior. label May 9, 2026

sanych-sun requested review from BorisDog and ajcvickers and removed request for ajcvickers May 9, 2026 00:26

sanych-sun marked this pull request as draft May 9, 2026 00:26

Copilot started reviewing on behalf of sanych-sun May 9, 2026 00:27 View session

Copilot AI reviewed May 9, 2026

View reviewed changes

sanych-sun added 2 commits May 19, 2026 16:39

CSHARP-6029: Make ICoreServerSessionPool disposable and send endSessi…

a42ca19

…ons on dispose

pr

b1b8b07

sanych-sun force-pushed the CSHARP-6029 branch from 7d4ebf7 to b1b8b07 Compare May 20, 2026 00:37

sanych-sun added 2 commits May 21, 2026 18:49

pr

f2bb7f7

pr

88900f0

sanych-sun marked this pull request as ready for review May 22, 2026 03:10

sanych-sun commented May 22, 2026

View reviewed changes

BorisDog requested changes May 22, 2026

View reviewed changes

pr

3eeced0

sanych-sun requested a review from BorisDog May 22, 2026 18:06

BorisDog requested changes May 22, 2026

View reviewed changes

BorisDog reviewed May 22, 2026

View reviewed changes

Comment thread tests/MongoDB.Driver.Tests/Core/Bindings/CoreServerSessionPoolTests.cs Outdated

pr

dcadd7d

sanych-sun requested a review from BorisDog May 22, 2026 21:03

sanych-sun added 3 commits May 25, 2026 10:06

pr

d23b3f6

pr

51d4499

pr

5ddcc9a

BorisDog approved these changes May 25, 2026

View reviewed changes

pr

dc009ab

sanych-sun merged commit 528a7ad into mongodb:main May 25, 2026
33 checks passed

sanych-sun deleted the CSHARP-6029 branch May 25, 2026 19:34

Conversation

sanych-sun commented May 9, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

BorisDog May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

BorisDog May 22, 2026 •

edited

Loading