CSHARP-3927: Drivers should check out an implicit session only after checking out a connection

BorisDog · BorisDog · commit aaa21154fb55 · 2022-02-15T09:08:48.000-08:00
diff --git a/src/MongoDB.Driver.Core/Core/Bindings/CoreSession.cs b/src/MongoDB.Driver.Core/Core/Bindings/CoreSession.cs
@@ -39,7 +39,7 @@ public sealed class CoreSession : ICoreSession
         private bool _isCommitTransactionInProgress;
         private readonly IOperationClock _operationClock = new OperationClock();
         private readonly CoreSessionOptions _options;
-        private readonly ICoreServerSession _serverSession;
+        private readonly Lazy<ICoreServerSession> _serverSession;
         private BsonTimestamp _snapshotTime;
 
         // constructors
@@ -49,13 +49,32 @@ public sealed class CoreSession : ICoreSession
         /// <param name="cluster">The cluster.</param>
         /// <param name="serverSession">The server session.</param>
         /// <param name="options">The options.</param>
+        [Obsolete("This constructor is deprecated. Avoid using CoreSession directly.")]
         public CoreSession(
             ICluster cluster,
             ICoreServerSession serverSession,
             CoreSessionOptions options)
+            : this(cluster, options: options)
+        {
+            Ensure.IsNotNull(serverSession, nameof(serverSession));
+            _serverSession = new Lazy<ICoreServerSession>(() => serverSession);
+        }
+
+        internal CoreSession(
+            ICluster cluster,
+            ICoreServerSessionPool serverSessionPool,
+            CoreSessionOptions options)
+             : this(cluster, options)
+        {
+            Ensure.IsNotNull(serverSessionPool, nameof(serverSessionPool));
+            _serverSession = new Lazy<ICoreServerSession>(() => serverSessionPool.AcquireSession());
+        }
+
+        private CoreSession(
+           ICluster cluster,
+           CoreSessionOptions options)
         {
             _cluster = Ensure.IsNotNull(cluster, nameof(cluster));
-            _serverSession = Ensure.IsNotNull(serverSession, nameof(serverSession));
             _options = Ensure.IsNotNull(options, nameof(options));
         }
 
@@ -75,13 +94,13 @@ public CoreSession(
         public CoreTransaction CurrentTransaction => _currentTransaction;
 
         /// <inheritdoc />
-        public BsonDocument Id => _serverSession.Id;
+        public BsonDocument Id => _serverSession.Value.Id;
 
         /// <inheritdoc />
         public bool IsCausallyConsistent => _options.IsCausallyConsistent;
 
         /// <inheritdoc />
-        public bool IsDirty => _serverSession.IsDirty;
+        public bool IsDirty => _serverSession.Value.IsDirty;
 
         /// <inheritdoc />
         public bool IsImplicit => _options.IsImplicit;
@@ -120,7 +139,7 @@ public bool IsInTransaction
         public CoreSessionOptions Options => _options;
 
         /// <inheritdoc />
-        public ICoreServerSession ServerSession => _serverSession;
+        public ICoreServerSession ServerSession => _serverSession.Value;
 
         /// <inheritdoc />
         public BsonTimestamp SnapshotTime => _snapshotTime;
@@ -270,7 +289,7 @@ public void AdvanceOperationTime(BsonTimestamp newOperationTime)
         /// <inheritdoc />
         public long AdvanceTransactionNumber()
         {
-            return _serverSession.AdvanceTransactionNumber();
+            return _serverSession.Value.AdvanceTransactionNumber();
         }
 
         /// <inheritdoc />
@@ -367,15 +386,15 @@ public void Dispose()
                 }
 
                 _currentTransaction?.UnpinAll();
-                _serverSession.Dispose();
+                _serverSession.Value.Dispose();
                 _disposed = true;
             }
         }
 
         /// <inheritdoc />
         public void MarkDirty()
         {
-            _serverSession.MarkDirty();
+            _serverSession.Value.MarkDirty();
         }
 
         /// <inheritdoc />
@@ -406,7 +425,7 @@ public void SetSnapshotTimeIfNeeded(BsonTimestamp snapshotTime)
         /// <inheritdoc />
         public void WasUsed()
         {
-            _serverSession.WasUsed();
+            _serverSession.Value.WasUsed();
         }
 
         // private methods
diff --git a/src/MongoDB.Driver.Core/Core/Clusters/Cluster.cs b/src/MongoDB.Driver.Core/Core/Clusters/Cluster.cs
@@ -329,8 +329,7 @@ public async Task<IServer> SelectServerAsync(IServerSelector selector, Cancellat
         public ICoreSessionHandle StartSession(CoreSessionOptions options)
         {
             options = options ?? new CoreSessionOptions();
-            var serverSession = AcquireServerSession();
-            var session = new CoreSession(this, serverSession, options);
+            var session = new CoreSession(this, _serverSessionPool, options);
             return new CoreSessionHandle(session);
         }
 
diff --git a/src/MongoDB.Driver.Core/Core/Clusters/LoadBalancedCluster.cs b/src/MongoDB.Driver.Core/Core/Clusters/LoadBalancedCluster.cs
@@ -230,8 +230,7 @@ public ICoreSessionHandle StartSession(CoreSessionOptions options = null)
             ThrowIfDisposed();
 
             options = options ?? new CoreSessionOptions();
-            var serverSession = AcquireServerSession();
-            var session = new CoreSession(this, serverSession, options);
+            var session = new CoreSession(this, _serverSessionPool, options);
             return new CoreSessionHandle(session);
         }
 
diff --git a/src/MongoDB.Driver/ClientSessionHandle.cs b/src/MongoDB.Driver/ClientSessionHandle.cs
@@ -19,7 +19,6 @@
 using MongoDB.Bson;
 using MongoDB.Driver.Core.Bindings;
 using MongoDB.Driver.Core.Misc;
-using MongoDB.Driver.Support;
 
 namespace MongoDB.Driver
 {
@@ -35,7 +34,7 @@ internal sealed class ClientSessionHandle : IClientSessionHandle
         private readonly ICoreSessionHandle _coreSession;
         private bool _disposed;
         private readonly ClientSessionOptions _options;
-        private readonly IServerSession _serverSession;
+        private IServerSession _serverSession;
 
         // constructors
         /// <summary>
@@ -54,7 +53,6 @@ internal ClientSessionHandle(IMongoClient client, ClientSessionOptions options,
             _client = client;
             _options = options;
             _coreSession = coreSession;
-            _serverSession = new ServerSession(coreSession.ServerSession);
             _clock = clock;
         }
 
@@ -78,7 +76,18 @@ internal ClientSessionHandle(IMongoClient client, ClientSessionOptions options,
         public ClientSessionOptions Options => _options;
 
         /// <inheritdoc />
-        public IServerSession ServerSession => _serverSession;
+        public IServerSession ServerSession
+        {
+            get
+            {
+                if (_serverSession == null)
+                {
+                    _serverSession = new ServerSession(_coreSession.ServerSession);
+                }
+
+                return _serverSession;
+            }
+        }
 
         /// <inheritdoc />
         public ICoreSessionHandle WrappedCoreSession => _coreSession;
@@ -126,7 +135,7 @@ public void Dispose()
             if (!_disposed)
             {
                 _coreSession.Dispose();
-                _serverSession.Dispose();
+                _serverSession?.Dispose();
                 _disposed = true;
             }
         }
diff --git a/tests/MongoDB.Driver.Core.Tests/Core/Bindings/CoreSessionTests.cs b/tests/MongoDB.Driver.Core.Tests/Core/Bindings/CoreSessionTests.cs
@@ -40,7 +40,9 @@ public void constructor_should_initialize_instance()
             var serverSession = Mock.Of<ICoreServerSession>();
             var options = new CoreSessionOptions();
 
+#pragma warning disable CS0618 // Type or member is obsolete
             var result = new CoreSession(cluster, serverSession, options);
+#pragma warning restore CS0618 // Type or member is obsolete
 
             result.Cluster.Should().BeSameAs(cluster);
             result.CurrentTransaction.Should().BeNull();
@@ -440,7 +442,9 @@ private CoreSession CreateSubject(
             cluster = cluster ?? CreateMockReplicaSetCluster();
             serverSession = serverSession ?? Mock.Of<ICoreServerSession>();
             options = options ?? new CoreSessionOptions();
+#pragma warning disable CS0618 // Type or member is obsolete
             return new CoreSession(cluster, serverSession, options);
+#pragma warning restore CS0618 // Type or member is obsolete
         }
 
         private CoreSession CreateSubject(ClusterDescription clusterDescription)
diff --git a/tests/MongoDB.Driver.Tests/ClientSessionHandleTests.cs b/tests/MongoDB.Driver.Tests/ClientSessionHandleTests.cs
@@ -252,7 +252,9 @@ public void Fork_should_return_expected_result()
             var cluster = Mock.Of<ICluster>();
             var coreServerSession = new CoreServerSession();
             var options = new ClientSessionOptions();
+#pragma warning disable CS0618 // Type or member is obsolete
             var coreSession = new CoreSession(cluster, coreServerSession, options.ToCore());
+#pragma warning restore CS0618 // Type or member is obsolete
             var coreSessionHandle = new CoreSessionHandle(coreSession);
             var subject = CreateSubject(coreSession: coreSessionHandle);
             coreSessionHandle.ReferenceCount().Should().Be(1);
diff --git a/tests/MongoDB.Driver.Tests/MockOperationExecutor.cs b/tests/MongoDB.Driver.Tests/MockOperationExecutor.cs
@@ -181,7 +181,9 @@ public IClientSessionHandle StartImplicitSession(CancellationToken cancellationT
             var cluster = Mock.Of<ICluster>();
             var options = new ClientSessionOptions();
             var coreServerSession = new CoreServerSession();
+#pragma warning disable CS0618 // Type or member is obsolete
             var coreSession = new CoreSession(cluster, coreServerSession, options.ToCore(isImplicit: true));
+#pragma warning restore CS0618 // Type or member is obsolete
             var coreSessionHandle = new CoreSessionHandle(coreSession);
             return new ClientSessionHandle(_client, options, coreSessionHandle);
         }
diff --git a/tests/MongoDB.Driver.Tests/MongoClientTests.cs b/tests/MongoDB.Driver.Tests/MongoClientTests.cs
@@ -425,7 +425,9 @@ private IClientSessionHandle CreateClientSession()
             var options = new ClientSessionOptions();
             var cluster = Mock.Of<ICluster>();
             var coreServerSession = new CoreServerSession();
+#pragma warning disable CS0618 // Type or member is obsolete
             var coreSession = new CoreSession(cluster, coreServerSession, options.ToCore());
+#pragma warning restore CS0618 // Type or member is obsolete
             var coreSessionHandle = new CoreSessionHandle(coreSession);
             return new ClientSessionHandle(client, options, coreSessionHandle);
         }
diff --git a/tests/MongoDB.Driver.Tests/MongoCollectionImplTests.cs b/tests/MongoDB.Driver.Tests/MongoCollectionImplTests.cs
@@ -3755,7 +3755,9 @@ private IClientSessionHandle CreateSession(bool usingSession)
                 var cluster = Mock.Of<ICluster>();
                 var options = new ClientSessionOptions();
                 var coreServerSession = new CoreServerSession();
+#pragma warning disable CS0618 // Type or member is obsolete
                 var coreSession = new CoreSession(cluster, coreServerSession, options.ToCore());
+#pragma warning restore CS0618 // Type or member is obsolete
                 var coreSessionHandle = new CoreSessionHandle(coreSession);
                 return new ClientSessionHandle(client, options, coreSessionHandle);
             }
diff --git a/tests/MongoDB.Driver.Tests/MongoDatabaseImplTests.cs b/tests/MongoDB.Driver.Tests/MongoDatabaseImplTests.cs
@@ -1452,7 +1452,9 @@ private IClientSessionHandle CreateSession(bool usingSession)
                 var cluster = Mock.Of<ICluster>();
                 var options = new ClientSessionOptions();
                 var coreServerSession = new CoreServerSession();
+#pragma warning disable CS0618 // Type or member is obsolete
                 var coreSession = new CoreSession(cluster, coreServerSession, options.ToCore());
+#pragma warning restore CS0618 // Type or member is obsolete
                 var coreSessionHandle = new CoreSessionHandle(coreSession);
                 return new ClientSessionHandle(_client, options, coreSessionHandle);
             }
diff --git a/tests/MongoDB.Driver.Tests/Specifications/sessions/SessionsProseTests.cs b/tests/MongoDB.Driver.Tests/Specifications/sessions/SessionsProseTests.cs
@@ -14,8 +14,14 @@
 */
 
 using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Threading.Tasks;
 using FluentAssertions;
-using MongoDB.Driver.Core.Misc;
+using MongoDB.Bson;
+using MongoDB.Bson.TestHelpers;
+using MongoDB.Driver.Core;
+using MongoDB.Driver.Core.Events;
 using MongoDB.Driver.Core.TestHelpers.XunitExtensions;
 using Xunit;
 
@@ -40,5 +46,57 @@ public void Snapshot_and_causal_consistent_session_is_not_allowed()
             var exception = Record.Exception(() => mongoClient.StartSession(sessionOptions));
             exception.Should().BeOfType<NotSupportedException>();
         }
+
+        [SkippableFact]
+        public async Task Ensure_server_session_are_allocated_only_on_connection_checkout()
+        {
+            var eventCapturer = new EventCapturer()
+               .Capture<ConnectionPoolCheckedOutConnectionEvent>()
+               .Capture<CommandStartedEvent>();
+
+            using var client = DriverTestConfiguration.CreateDisposableClient(
+                (MongoClientSettings settings) =>
+                {
+                    settings.MaxConnectionPoolSize = 1;
+                    settings.ClusterConfigurator = c => c.Subscribe(eventCapturer);
+                },
+                logger: null);
+
+            var database = client.GetDatabase("test");
+
+            var collection = database.GetCollection<BsonDocument>("inventory");
+            database.DropCollection("inventory");
+
+            collection.InsertOne(new BsonDocument("x", 0));
+
+            var serverSessionPool = (CoreServerSessionPool)Reflector.GetFieldValue(client.Cluster, "_serverSessionPool");
+            var serverSessionsList = (List<ICoreServerSession>)Reflector.GetFieldValue(serverSessionPool, "_pool");
+
+            var serverSession = serverSessionsList.Single();
+            eventCapturer.Clear();
+
+            var eventsTask = eventCapturer.NotifyWhen(events =>
+            {
+                var connectionCheckedOutEvent = events.OfType<ConnectionPoolCheckedOutConnectionEvent>().FirstOrDefault();
+                var commandStartedEvent = events.OfType<CommandStartedEvent>().FirstOrDefault();
+
+                if (commandStartedEvent.ConnectionId != null)
+                {
+                    serverSessionsList.Count.Should().Be(0);
+                    commandStartedEvent.Command["lsid"].Should().Be(serverSession.Id);
+
+                    return true;
+                }
+                else if (connectionCheckedOutEvent.ConnectionId != null)
+                {
+                    serverSessionsList.Single().Should().Be(serverSession);
+                }
+
+                return false;
+            });
+
+            collection.InsertOne(new BsonDocument("x", 1));
+            await TasksUtils.WithTimeout(eventsTask, 1000);
+        }
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -329,8 +329,7 @@ public async Task<IServer> SelectServerAsync(IServerSelector selector, Cancellat`
`329`	`329`	`public ICoreSessionHandle StartSession(CoreSessionOptions options)`
`330`	`330`	`{`
`331`	`331`	`options = options ?? new CoreSessionOptions();`
`332`		`- var serverSession = AcquireServerSession();`
`333`		`- var session = new CoreSession(this, serverSession, options);`
	`332`	`+ var session = new CoreSession(this, _serverSessionPool, options);`
`334`	`333`	`return new CoreSessionHandle(session);`
`335`	`334`	`}`
`336`	`335`
Original file line number	Diff line number	Diff line change
`@@ -230,8 +230,7 @@ public ICoreSessionHandle StartSession(CoreSessionOptions options = null)`
`230`	`230`	`ThrowIfDisposed();`
`231`	`231`
`232`	`232`	`options = options ?? new CoreSessionOptions();`
`233`		`- var serverSession = AcquireServerSession();`
`234`		`- var session = new CoreSession(this, serverSession, options);`
	`233`	`+ var session = new CoreSession(this, _serverSessionPool, options);`
`235`	`234`	`return new CoreSessionHandle(session);`
`236`	`235`	`}`
`237`	`236`