CSHARP-6011: Remove Session from IBinding by moving it to OperationContext#1985
Open
sanych-sun wants to merge 8 commits into
Open
CSHARP-6011: Remove Session from IBinding by moving it to OperationContext#1985sanych-sun wants to merge 8 commits into
sanych-sun wants to merge 8 commits into
Conversation
sanych-sun
added
the
improvement
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements CSHARP-6011 by removing Session from IBinding/channel source APIs and threading the session through MongoDB.Driver.OperationContext instead, updating both production code and the test suite accordingly.
Changes:
- Refactors core binding/channel abstractions to no longer carry
ICoreSessionHandle, usingoperationContext.Sessionwhere session state is required. - Updates retryable read/write executors and numerous operations (read, write, cursor-based) to use the new
OperationContext-centric session flow. - Adjusts a broad set of unit/integration/spec tests and test helpers to construct
OperationContextwith an explicit session handle.
Reviewed changes
Copilot reviewed 167 out of 167 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/MongoDB.Driver.Tests/Specifications/server-selection/InWindowTestRunner.cs | Updates server selection spec runner to pass OperationContext instead of OperationContext.NoTimeout. |
| tests/MongoDB.Driver.Tests/Specifications/server-discovery-and-monitoring/ServerDiscoveryAndMonitoringProseTests.cs | Updates SDAM prose tests to create OperationContext with a session. |
| tests/MongoDB.Driver.Tests/Specifications/mongodb-handshake/prose-tests/MongoDbHandshakeProseTests.cs | Updates handshake prose tests to use OperationContext instance. |
| tests/MongoDB.Driver.Tests/Specifications/connection-monitoring-and-pooling/ConnectionMonitoringAndPoolingTestRunner.cs | Updates CMAP spec runner to use OperationContext instance. |
| tests/MongoDB.Driver.Tests/Specifications/client-side-encryption/prose-tests/ClientEncryptionProseTests.cs | Updates CSFLE prose tests to execute operations with OperationContext. |
| tests/MongoDB.Driver.Tests/Specifications/client-backpressure/prose-tests/ClientBackpressureProseTests.cs | Updates client backpressure prose tests for new OperationContext constructor/session requirements. |
| tests/MongoDB.Driver.Tests/OperationExecutorTests.cs | Updates OperationExecutor tests to construct OperationContext with a session handle. |
| tests/MongoDB.Driver.Tests/OperationContextTests.cs | Updates OperationContext tests for new Session property/constructor; adds disposal behavior assertion. |
| tests/MongoDB.Driver.Tests/OperationContextExtensionsTests.cs | Updates extension tests to construct OperationContext with session + clock. |
| tests/MongoDB.Driver.Tests/Core/WireProtocol/CommandWriteProtocolTests.cs | Updates command protocol tests to use non-static OperationContext and updated mocking. |
| tests/MongoDB.Driver.Tests/Core/Servers/LoadBalancedServerTests.cs | Updates load-balanced server tests to pass OperationContext into channel acquisition. |
| tests/MongoDB.Driver.Tests/Core/Operations/WriteConcernHelperTests.cs | Updates tests for new WriteConcernHelper.GetEffectiveWriteConcern signature and session type changes. |
| tests/MongoDB.Driver.Tests/Core/Operations/WriteCommandOperationTests.cs | Updates write command operation tests to provide OperationContext and remove binding session expectations. |
| tests/MongoDB.Driver.Tests/Core/Operations/RetryableWriteOperationExecutorTests.cs | Updates retryable write executor tests for new APIs requiring OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/RetryableReadOperationExecutorTests.cs | Updates retryable read executor tests for new APIs requiring OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/RenameCollectionOperationTests.cs | Updates rename collection operation tests for new CreateCommand signature. |
| tests/MongoDB.Driver.Tests/Core/Operations/ReadCommandOperationTests.cs | Updates read command operation tests to provide OperationContext and remove binding session expectations. |
| tests/MongoDB.Driver.Tests/Core/Operations/QueryHelperTests.cs | Removes tests for deleted QueryHelper.CalculateFirstBatchSize. |
| tests/MongoDB.Driver.Tests/Core/Operations/MapReduceOutputToCollectionOperationTests.cs | Updates tests for new CreateCommand/execution signatures requiring OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/MapReduceOperationTests.cs | Updates MapReduce tests for new OperationContext usage. |
| tests/MongoDB.Driver.Tests/Core/Operations/ListIndexesUsingCommandOperationTests.cs | Updates list indexes (command) tests to pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/ListIndexesOperationTests.cs | Updates list indexes tests to pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/ListDatabasesOperationTests.cs | Updates list databases tests to pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/FindOneAndDeleteOperationTests.cs | Updates findAndModify-family tests for new CreateCommand signature using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/EstimatedDocumentCountOperationTests.cs | Updates estimated count tests for new CreateCommand signature using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/EndTransactionOperationTests.cs | Updates reflector test to invoke CreateCommand with OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/DropIndexOperationTests.cs | Updates drop index tests for new CreateCommand signature using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/DropDatabaseOperationTests.cs | Updates drop database tests to thread OperationContext and session explicitly. |
| tests/MongoDB.Driver.Tests/Core/Operations/DropCollectionOperationTests.cs | Updates drop collection tests for new CreateCommand signature using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/DistinctOperationTests.cs | Updates distinct tests for new CreateCommand signature using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/CursorTests.cs | Updates cursor construction tests for new AsyncCursor ctor requiring a session argument. |
| tests/MongoDB.Driver.Tests/Core/Operations/CreateViewOperationTests.cs | Updates create view tests to use OperationContext and updated helper signature. |
| tests/MongoDB.Driver.Tests/Core/Operations/CreateIndexesOperationTests.cs | Updates create indexes tests for new CreateCommand signature using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/CountDocumentsOperationTests.cs | Updates count documents tests to execute using OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/CompositeWriteOperationTests.cs | Updates composite write operation tests to provide an OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Operations/ChangeStreamOperationTests.cs | Updates change stream tests to provide OperationContext for execute paths. |
| tests/MongoDB.Driver.Tests/Core/Operations/AsyncCursorSourceEnumeratorTests.cs | Updates cursor source enumerator tests for new AsyncCursor ctor requiring session. |
| tests/MongoDB.Driver.Tests/Core/Operations/AsyncCursorEnumeratorTests.cs | Updates cursor enumerator tests for new AsyncCursor ctor requiring session. |
| tests/MongoDB.Driver.Tests/Core/LoadBalancingIntegrationTests.cs | Updates integration tests to use OperationContext in retryable contexts and operations. |
| tests/MongoDB.Driver.Tests/Core/Jira/CSharp3302Tests.cs | Updates Jira regression tests to use OperationContext instances. |
| tests/MongoDB.Driver.Tests/Core/Jira/CSharp3173Tests.cs | Updates Jira regression tests to use OperationContext instances. |
| tests/MongoDB.Driver.Tests/Core/IAsyncCursorSourceExtensionsTests.cs | Updates extension tests for new AsyncCursor ctor requiring session. |
| tests/MongoDB.Driver.Tests/Core/IAsyncCursorExtensionsTests.cs | Updates extension tests for new AsyncCursor ctor requiring session. |
| tests/MongoDB.Driver.Tests/Core/Connections/ConnectionInitializerTests.cs | Updates connection initializer tests to pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Connections/BinaryConnection_CommandEventTests.cs | Updates binary connection tests to pass OperationContext to open/send/receive calls. |
| tests/MongoDB.Driver.Tests/Core/ConnectionPools/MaintenanceHelperTests.cs | Updates maintenance tests to use OperationContext for pool checkout. |
| tests/MongoDB.Driver.Tests/Core/Clusters/LoadBalancedClusterTests.cs | Updates LB cluster tests to pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Bindings/ServerChannelSourceTests.cs | Updates tests for ServerChannelSource API removal of session parameter. |
| tests/MongoDB.Driver.Tests/Core/Bindings/ReadBindingHandleTests.cs | Updates binding handle tests to remove session delegation and pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Bindings/ChannelSourceHandleTests.cs | Updates channel source handle tests to remove session delegation and pass OperationContext. |
| tests/MongoDB.Driver.Tests/Core/Bindings/ChannelChannelSourceTests.cs | Updates tests for ChannelChannelSource API removal of session parameter. |
| tests/MongoDB.Driver.Tests/ClusterTests.cs | Updates cluster load-balancing prose test logic for server selection and OperationContext. |
| tests/MongoDB.Driver.Tests/AuthenticationTests.cs | Updates auth tests to use OperationContext for server/channel selection. |
| tests/MongoDB.Driver.Tests/Authentication/ScramSha1AuthenticatorTests.cs | Updates SCRAM tests to pass OperationContext through authenticator APIs. |
| tests/MongoDB.Driver.Tests/Authentication/PlainAuthenticatorTests.cs | Updates PLAIN auth tests to pass OperationContext through authenticator APIs. |
| tests/MongoDB.Driver.Tests/Authentication/MongoDBX509AuthenticatorTests.cs | Updates X509 auth tests to pass OperationContext through authenticator APIs. |
| tests/MongoDB.Driver.Tests/Authentication/MongoAWSAuthenticatorTests.cs | Updates AWS auth tests to pass OperationContext through authenticator APIs. |
| tests/MongoDB.Driver.Tests/Authentication/AuthenticationHelperTests.cs | Updates helper tests to pass OperationContext to AuthenticationHelper. |
| tests/MongoDB.Driver.TestHelpers/DriverTestConfiguration.cs | Updates helper to build bindings without embedding sessions; uses OperationContext for channel acquisition. |
| tests/MongoDB.Driver.TestHelpers/Core/MockClusterableServerFactory.cs | Updates test helper; attempts to adjust connection pool acquire call signature. |
| tests/MongoDB.Driver.TestHelpers/Core/FailPoint.cs | Updates fail point helper to use OperationContext and session-less bindings. |
| tests/MongoDB.Driver.TestHelpers/Core/CoreTestConfiguration.cs | Updates core test configuration helpers to create OperationContext and session-less bindings. |
| tests/MongoDB.Bson.TestHelpers/Reflector.cs | Adds a 4-argument Invoke overload used by updated reflection-based tests. |
| src/MongoDB.Driver/TransactionExecutor.cs | Constructs OperationContext with the wrapped core session for transaction execution. |
| src/MongoDB.Driver/OperationExecutor.cs | Removes binding session forking; uses session-less bindings and operation-context session flow. |
| src/MongoDB.Driver/OperationContext.cs | Adds required Session to OperationContext; removes NoTimeout; updates fork/metadata logic. |
| src/MongoDB.Driver/MongoDB.Driver.csproj | Excludes deleted DelayedEvaluationWriteConcernSerializer from compilation. |
| src/MongoDB.Driver/MongoDatabase.cs | Creates OperationContext using session + object initializer for tracing metadata. |
| src/MongoDB.Driver/MongoCollectionImpl.cs | Creates OperationContext using session + object initializer for tracing metadata. |
| src/MongoDB.Driver/MongoClient.cs | Creates OperationContext using session + object initializer for tracing metadata. |
| src/MongoDB.Driver/GridFS/GridFSSeekableDownloadStream.cs | Updates internal CSOT TODO contexts to include a session handle. |
| src/MongoDB.Driver/GridFS/GridFSForwardOnlyUploadStream.cs | Updates internal CSOT TODO contexts to include a session handle. |
| src/MongoDB.Driver/GridFS/GridFSForwardOnlyDownloadStream.cs | Updates internal CSOT TODO contexts to include a session handle. |
| src/MongoDB.Driver/Core/Servers/ServerMonitor.cs | Updates monitor heartbeats to use OperationContext.Session via explicit session handle. |
| src/MongoDB.Driver/Core/Servers/ServerChannel.cs | Removes session parameter from Command/CommandAsync; uses operationContext.Session. |
| src/MongoDB.Driver/Core/Servers/RoundTripTimeMonitor.cs | Updates RTT monitor loop to use OperationContext with a session handle. |
| src/MongoDB.Driver/Core/Operations/WriteConcernHelper.cs | Changes effective write concern to derive transaction state from operationContext.Session. |
| src/MongoDB.Driver/Core/Operations/WriteCommandOperation.cs | Removes binding session usage from protocol execution. |
| src/MongoDB.Driver/Core/Operations/UpdateSearchIndexOperation.cs | Removes channel binding session usage; uses OperationContext for session state. |
| src/MongoDB.Driver/Core/Operations/RetryableWriteOperationExecutor.cs | Moves retry/session checks from binding session to operationContext.Session; updates end-transaction retry hook signature. |
| src/MongoDB.Driver/Core/Operations/RetryableWriteContext.cs | Pins channels using operationContext.Session instead of binding session. |
| src/MongoDB.Driver/Core/Operations/RetryableWriteCommandOperationBase.cs | Removes session parameter from command creation flow; uses OperationContext only. |
| src/MongoDB.Driver/Core/Operations/RetryableUpdateCommandOperation.cs | Uses new WriteConcernHelper signature; removes explicit session param. |
| src/MongoDB.Driver/Core/Operations/RetryableReadOperationExecutor.cs | Moves transaction checks to operationContext.Session. |
| src/MongoDB.Driver/Core/Operations/RetryableReadContext.cs | Pins channels using operationContext.Session instead of binding session. |
| src/MongoDB.Driver/Core/Operations/RetryableInsertCommandOperation.cs | Uses new WriteConcernHelper signature; removes explicit session param. |
| src/MongoDB.Driver/Core/Operations/RetryableDeleteCommandOperation.cs | Uses new WriteConcernHelper signature; removes explicit session param. |
| src/MongoDB.Driver/Core/Operations/RenameCollectionOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/ReadCommandOperation.cs | Removes session param from command creator/protocol execution. |
| src/MongoDB.Driver/Core/Operations/QueryHelper.cs | Removes CalculateFirstBatchSize method (and corresponding tests). |
| src/MongoDB.Driver/Core/Operations/MapReduceOutputToCollectionOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/MapReduceOperationBase.cs | Removes session param from command creation. |
| src/MongoDB.Driver/Core/Operations/MapReduceOperation.cs | Removes session param from channel binding; uses operationContext.Session for read concern decisions. |
| src/MongoDB.Driver/Core/Operations/ListIndexesUsingCommandOperation.cs | Updates cursor creation to take OperationContext and supply session for cursor continuation. |
| src/MongoDB.Driver/Core/Operations/ListIndexesOperation.cs | Updates headers/usings in operation implementation. |
| src/MongoDB.Driver/Core/Operations/ListDatabasesOperation.cs | Updates headers/usings in operation implementation. |
| src/MongoDB.Driver/Core/Operations/ListCollectionsOperation.cs | Updates cursor creation to take OperationContext and supply session for cursor continuation. |
| src/MongoDB.Driver/Core/Operations/ICommandCreator.cs | Removes session parameter from command creator interface. |
| src/MongoDB.Driver/Core/Operations/FindOperation.cs | Removes session parameter from command creation; uses operationContext.Session for read concern/snapshot time and cursor session. |
| src/MongoDB.Driver/Core/Operations/FindOneAndUpdateOperation.cs | Removes explicit session param from command creation; uses new write concern helper signature. |
| src/MongoDB.Driver/Core/Operations/FindOneAndReplaceOperation.cs | Removes explicit session param from command creation; uses new write concern helper signature. |
| src/MongoDB.Driver/Core/Operations/FindOneAndDeleteOperation.cs | Removes explicit session param from command creation; uses new write concern helper signature. |
| src/MongoDB.Driver/Core/Operations/FindAndModifyOperationBase.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/EndTransactionOperation.cs | Updates retry hook to accept OperationContext; removes binding session usage for unpin logic. |
| src/MongoDB.Driver/Core/Operations/DropSearchIndexOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/DropIndexOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/DropDatabaseOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/DropCollectionOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/DistinctOperation.cs | Uses operationContext.Session for snapshot time and read concern; updates command creator signature. |
| src/MongoDB.Driver/Core/Operations/DelayedEvaluationWriteConcernSerializer.cs | Deletes serializer implementation file (and removes from project). |
| src/MongoDB.Driver/Core/Operations/CreateViewOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/CreateSearchIndexesOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/CreateIndexesOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/CreateCollectionOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/CountOperation.cs | Removes session param from command creation; uses operationContext.Session for read concern decisions. |
| src/MongoDB.Driver/Core/Operations/CommandOperationBase.cs | Removes explicit session argument from protocol execution; relies on OperationContext. |
| src/MongoDB.Driver/Core/Operations/ClientBulkWriteOperation.cs | Updates retryable write command base integration and cursor creation to include session via OperationContext. |
| src/MongoDB.Driver/Core/Operations/ChangeStreamOperation.cs | Passes session handle into ChangeStreamCursor and reads operation time from operationContext.Session. |
| src/MongoDB.Driver/Core/Operations/ChangeStreamCursor.cs | Stores and disposes an explicit session handle; creates resume OperationContext with that session. |
| src/MongoDB.Driver/Core/Operations/AggregateToCollectionOperation.cs | Removes session param from command creation and per-attempt channel binding. |
| src/MongoDB.Driver/Core/Operations/AggregateOperation.cs | Uses operationContext.Session for snapshot time and cursor session; updates command creator signature. |
| src/MongoDB.Driver/Core/Misc/Feature.cs | Uses OperationContext + session-less bindings for feature checks. |
| src/MongoDB.Driver/Core/ConnectionPools/ExclusiveConnectionPool.Helpers.cs | Updates internal connection creation helper to construct OperationContext with a session handle. |
| src/MongoDB.Driver/Core/Clusters/IClusterExtensions.cs | Adjusts pinning helpers to accept ICoreSession rather than ICoreSessionHandle. |
| src/MongoDB.Driver/Core/ChannelPinningHelper.cs | Removes session storage from bindings/channel sources; pins using provided session directly. |
| src/MongoDB.Driver/Core/Bindings/WritableServerBinding.cs | Removes embedded session; selects/pins servers using operationContext.Session. |
| src/MongoDB.Driver/Core/Bindings/SingleServerReadWriteBinding.cs | Removes embedded session; channel source creation no longer forks/owns sessions. |
| src/MongoDB.Driver/Core/Bindings/SingleServerReadBinding.cs | Removes embedded session; channel source creation no longer forks/owns sessions. |
| src/MongoDB.Driver/Core/Bindings/ServerChannelSource.cs | Removes embedded session; IChannelSource.Session removed. |
| src/MongoDB.Driver/Core/Bindings/ReadWriteBindingHandle.cs | Removes session delegation via binding handle. |
| src/MongoDB.Driver/Core/Bindings/ReadPreferenceBinding.cs | Removes embedded session; selects/pins servers using operationContext.Session. |
| src/MongoDB.Driver/Core/Bindings/ReadBindingHandle.cs | Removes session delegation via binding handle. |
| src/MongoDB.Driver/Core/Bindings/IChannelSource.cs | Removes Session from channel source interface. |
| src/MongoDB.Driver/Core/Bindings/IChannel.cs | Removes explicit session parameter from channel command APIs. |
| src/MongoDB.Driver/Core/Bindings/IBinding.cs | Removes Session from IBinding. |
| src/MongoDB.Driver/Core/Bindings/EndTransactionReadWriteBinding.cs | Removes embedded session and rebuilds inner binding lazily from operationContext.Session. |
| src/MongoDB.Driver/Core/Bindings/CoreSession.cs | Creates transaction commit/abort OperationContext with non-disposing session handle; updates end-transaction binding creation. |
| src/MongoDB.Driver/Core/Bindings/ChannelSourceHandle.cs | Removes session delegation via channel source handle. |
| src/MongoDB.Driver/Core/Bindings/ChannelReadWriteBinding.cs | Removes embedded session; channel source creation no longer forks/owns sessions. |
| src/MongoDB.Driver/Core/Bindings/ChannelReadBinding.cs | Removes embedded session; channel source creation no longer forks/owns sessions. |
| src/MongoDB.Driver/Core/Bindings/ChannelChannelSource.cs | Removes embedded session; IChannelSource.Session removed. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
188
to
192
| var helloResult = new HelloResult(new BsonDocument { { "compressors", new BsonArray() }, { "maxWireVersion", maxWireVersion } }); | ||
| var mockConnection = Mock.Get(server._connectionPool().AcquireConnection(OperationContext.NoTimeout)); | ||
|
|
||
| var mockConnection = Mock.Get(server._connectionPool().AcquireConnection(It.IsAny<OperationContext>())); | ||
| mockConnection.SetupGet(c => c.Description) | ||
| .Returns(new ConnectionDescription(new ConnectionId(description.ServerId, 0), helloResult)); |
sanych-sun
commented
May 5, 2026
| return CreateCursorFromCursorResult(operationContext, channelSource, channel, result); | ||
| } | ||
|
|
||
| return new SingleBatchAsyncCursor<TResult>(result.Results); |
Member
Author
There was a problem hiding this comment.
Instead of creating AsyncCursor without channelSource we can simplify the execution by creating SingleBatchAsyncCursor which exists for this very reason - to create a cursor from inlined response. I can separate this to another PR if needed.
papafe
reviewed
May 6, 2026
| DatabaseName = DatabaseName, | ||
| CollectionName = CollectionName, | ||
| IsTracingEnabled = IsTracingEnabled | ||
| IsTracingEnabled = IsTracingEnabled, |
| <Compile Include="..\MongoDB.Shared\DictionaryComparer.cs" Link="Shared\DictionaryComparer.cs" /> | ||
| <Compile Include="..\MongoDB.Shared\SequenceComparer.cs" Link="Shared\SequenceComparer.cs" /> | ||
| <Compile Include="..\MongoDB.Shared\Hasher.cs" Link="Shared\Hasher.cs" /> | ||
| <Compile Remove="Core\Operations\DelayedEvaluationWriteConcernSerializer.cs" /> |
| return _innerBinding.GetWriteChannelSourceAsync(operationContext, deprioritizedServers, mayUseSecondary); | ||
| } | ||
|
|
||
| private void EnsureInnerBinding(OperationContext operationContext) |
Contributor
There was a problem hiding this comment.
Should we worry about thread safety for this?
Member
Author
There was a problem hiding this comment.
I do not think we have any code path that uses Bindings in a concurrent way. RebuildInnerBinding (the previous place where it was assigned before) was also non-thread safe.
| mockConnection.SetupGet(c => c.Description).Returns(description); | ||
| mockConnection.SetupGet(c => c.Settings).Returns(settings); | ||
|
|
||
| using var operationContext = new OperationContext(NoCoreSession.NewHandle()); |
Contributor
There was a problem hiding this comment.
nit. Given we use this in quite a lot of places, would it make sense to move it to a static factory...?
adelinowona
reviewed
May 29, 2026
Contributor
adelinowona
left a comment
adelinowona
left a comment
There was a problem hiding this comment.
CoreSession.cs — ExecuteEndTransactionOnPrimary[Async] lines 562–563 and 575–576
By the time ExecuteEndTransactionOnPrimary is called, operationContext.Session is already a NonDisposingCoreSessionHandle wrapping this — the callers (CommitTransaction, AbortTransaction, and their async variants) create one four lines before calling this method. The second new NonDisposingCoreSessionHandle(this) here is a redundant wrapper around the same object and can be eliminated by passing operationContext.Session directly to CreateEndTransactionBinding.
| var helloResult = new HelloResult(new BsonDocument { { "compressors", new BsonArray() }, { "maxWireVersion", maxWireVersion } }); | ||
| var mockConnection = Mock.Get(server._connectionPool().AcquireConnection(OperationContext.NoTimeout)); | ||
|
|
||
| var mockConnection = Mock.Get(server._connectionPool().AcquireConnection(It.IsAny<OperationContext>())); |
Contributor
There was a problem hiding this comment.
It.IsAny<OperationContext>() is being used here as a concrete method argument, not inside a Moq Setup/Verify call. Outside that context, It.IsAny<T>() returns default(T), so this passes null to AcquireConnection. It works by accident because the mock was set up with It.IsAny<OperationContext>() which also matches null — but it's the wrong tool here. This was presumably meant to replace the deleted OperationContext.NoTimeout static. Should be something like new OperationContext(NoCoreSession.NewHandle()).
| if (!channel.Connection.IsExpired) | ||
| { | ||
| ExecuteKillCursorsCommand(channel, cancellationToken); | ||
| ExecuteKillCursorsCommand(operationContext, channel); |
Contributor
There was a problem hiding this comment.
The 10-second timeout applied via operationContext.WithTimeout(TimeSpan.FromSeconds(10)) scopes the channel acquisition only — ExecuteKillCursorsCommand here receives the original uncapped operationContext. The kill command itself runs without any timeout guard. The timeout-scoped context should be captured in a local variable and passed through to ExecuteKillCursorsCommand as well (same issue on the async path at line 480).
| return _innerBinding.GetWriteChannelSourceAsync(operationContext, deprioritizedServers, mayUseSecondary); | ||
| } | ||
|
|
||
| private void EnsureInnerBinding(OperationContext operationContext) |
Contributor
There was a problem hiding this comment.
EnsureInnerBinding doesn't call ThrowIfDisposed() before doing its work. All 12 public Get*ChannelSource* methods delegate here without checking disposed state first, so after Dispose() any of those calls silently rebuilds _innerBinding. Every other binding class guards its public methods — and RebuildInnerBinding (line 43) does correctly call ThrowIfDisposed — which makes the gap here inconsistent. ThrowIfDisposed() should be the first line of this method.
| return new ReadBindingHandle(binding); | ||
| } | ||
|
|
||
| private static IReadWriteBindingHandle CreateReadWriteBinding(ICoreSessionHandle session) |
Contributor
There was a problem hiding this comment.
The session parameter is no longer used in the body — it was needed when bindings took a session argument, but WritableServerBinding no longer does. Dead parameter, should be removed.
| @@ -262,15 +267,15 @@ private IAsyncCursor<RawBsonDocument> Resume(CancellationToken cancellationToken | |||
| { | |||
| ReconfigureOperationResumeValues(); | |||
| // TODO: CSOT implement proper way to obtain the operationContext | |||
Contributor
There was a problem hiding this comment.
The session wiring is now correct — this was fixed as part of this PR. The only remaining open item is threading a CSOT timeout through, not the entire operationContext construction. Worth narrowing the comment to something like // TODO CSHARP-XXXX: pass CSOT timeout to Resume so it's clear what's actually still missing.
adelinowona
reviewed
May 29, 2026
| Ensure.IsNotNull((object)id, nameof(id)); | ||
| // TODO: CSOT implement proper way to obtain the operationContext | ||
| var operationContext = new OperationContext(null, cancellationToken); | ||
| using var operationContext = new OperationContext(NoCoreSession.NewHandle(), null, cancellationToken); |
Contributor
There was a problem hiding this comment.
NoCoreSession.NewHandle() is created inline and never disposed. OperationContext.Dispose() intentionally does not dispose the session, so the CoreSessionHandle (and its ReferenceCountedCoreSession) returned by NewHandle() is silently abandoned on every GridFS call.
Copilot already flagged this in Feature.cs and it was fixed there. The same fix is needed for all occurrences in this file:
using var session = NoCoreSession.NewHandle();
using var operationContext = new OperationContext(session, null, cancellationToken);
adelinowona
reviewed
May 29, 2026
|
|
||
| var helloOk = false; | ||
| using var operationContext = new OperationContext(null, _cancellationToken); | ||
| using var operationContext = new OperationContext(NoCoreSession.NewHandle(), null, _cancellationToken); |
Contributor
There was a problem hiding this comment.
Same inline NoCoreSession.NewHandle() leak as flagged in Feature.cs and GridFSBucket.cs. OperationContext.Dispose() does not dispose the session, so the handle is abandoned on every RTT measurement iteration. Fix:
using var session = NoCoreSession.NewHandle();
using var operationContext = new OperationContext(session, null, _cancellationToken);
adelinowona
reviewed
May 29, 2026
| private void Heartbeat(CancellationToken cancellationToken) | ||
| { | ||
| using var operationContext = new OperationContext(null, cancellationToken); | ||
| using var operationContext = new OperationContext(NoCoreSession.NewHandle(), null, cancellationToken); |
Contributor
There was a problem hiding this comment.
Same inline NoCoreSession.NewHandle() leak as flagged in Feature.cs and GridFSBucket.cs. OperationContext.Dispose() does not dispose the session, so the CoreSessionHandle (and its ReferenceCountedCoreSession) is abandoned on every heartbeat. Fix:
using var session = NoCoreSession.NewHandle();
using var operationContext = new OperationContext(session, null, cancellationToken);
adelinowona
reviewed
May 29, 2026
| } | ||
|
|
||
| var batch = GetNextBatch(cancellationToken); | ||
| using var operationContext = new OperationContext(_session, cancellationToken: cancellationToken); |
Contributor
There was a problem hiding this comment.
The old GetNextBatch had a // TODO: CSOT implement proper way to obtain the operationContext comment documenting that cursor get-more operations have no CSOT timeout. This refactor moved OperationContext creation here but did not carry the TODO forward, so the known limitation is now undocumented. ChangeStreamCursor.Resume still keeps the equivalent TODO, creating inconsistency. Same issue applies to line 513 (MoveNextAsync).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a pre-requisite for the operations refactoring. It moves Session from IBinding to OperationContext. (Bindings most likely will be removed in scope of operations refactoring)