CSHARP-5798: Implement test to track if any UnobservedTaskExceptions were raised while test run #1836

sanych-sun · 2025-12-08T21:40:43Z

Fix for the corner case on net6 on MacOS, when disposal of socket while connecting makes Connect to complete without any errors, but the socket is being Disposed and any next operation will fail with ObjectDisposedException.
It might be related to this issue: dotnet/runtime#75889

…were raised while test run

sanych-sun · 2025-12-08T21:41:33Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

                }
            }
+
+            static void EnsureConnected(Socket socket)


Implementation of the method is inspired by the recommented by Microsoft way to check if the socket is alive:
https://learn.microsoft.com/en-us/dotnet/api/system.net.sockets.socket.connected?view=net-10.0

Copilot

Pull request overview

This PR implements a fix for a corner case on .NET 6 macOS where socket disposal during connection can cause Connect to complete without errors, leaving the socket in a disposed state that causes subsequent operations to fail. The fix introduces verification that the socket is truly connected and prevents unnecessary disposal when the connection succeeds.

Key changes:

Added EnsureConnected method to verify socket connectivity after Connect completes
Introduced ConnectOperationState to track connection success and prevent disposal of successfully connected sockets
Refactored cancellation callback to check connection state before disposing socket

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

Copilot · 2025-12-08T21:41:33Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

+                bool originalBlockingState = socket.Blocking;
+                socket.Blocking = false;
+
+                try
+                {
+                    // Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.
+                    socket.Send(__ensureConnectedBuffer, 0, 0);
+                }
+                finally
+                {
+                    // Restore original blocking state
+                    socket.Blocking = originalBlockingState;


The socket's blocking state is not restored if accessing socket.Blocking throws an ObjectDisposedException. If the socket is already disposed when entering EnsureConnected, the getter for originalBlockingState will throw, but the finally block won't execute to restore the state (though this is moot since the socket is disposed). More critically, if the socket gets disposed between line 229 and the try block, the finally block will attempt to set socket.Blocking on a disposed socket. Consider wrapping the entire method body in a try-catch to handle ObjectDisposedException gracefully.

Suggested change

bool originalBlockingState = socket.Blocking;

socket.Blocking = false;

try

{

// Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.

socket.Send(__ensureConnectedBuffer, 0, 0);

}

finally

{

// Restore original blocking state

socket.Blocking = originalBlockingState;

try

{

bool originalBlockingState = socket.Blocking;

socket.Blocking = false;

try

{

// Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.

socket.Send(__ensureConnectedBuffer, 0, 0);

}

finally

{

// Restore original blocking state

socket.Blocking = originalBlockingState;

}

}

catch (ObjectDisposedException)

{

// Socket is already disposed, nothing to do.

It's OK for the method to throw ObjectDisposedExcpetion, we have try-catch in the outer code.

Copilot · 2025-12-08T21:41:34Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

+                    // Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.
+                    socket.Send(__ensureConnectedBuffer, 0, 0);


Sending 0 bytes may not reliably detect a disposed socket on all platforms. Consider documenting why this specific approach was chosen for the macOS .NET 6 issue, or explore alternative detection methods such as checking socket.Connected property or using socket.Poll() with a zero timeout. The current implementation's effectiveness should be validated across different platforms.

Suggested change

// Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.

socket.Send(__ensureConnectedBuffer, 0, 0);

// Use Poll and Connected to ensure the socket is actually connected.

// This is a cross-platform way to check for a disposed or disconnected socket.

if (!socket.Connected || !socket.Poll(0, SelectMode.SelectWrite))

{

throw new SocketException((int)SocketError.NotConnected);

}

Disagree. Sending the 0-bytes is done as per Microsoft's suggestion. Also socket.Connected does not makes any checks, but returns results based on the last operation results. Problem with socket.Poll() - we need to provide a small-timeout, what value should we use?

That's OK. If ensuring the socket is alive takes too long - we want to interrupt the operation by disposing the socket.

Copilot · 2025-12-08T21:41:34Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

+            using var cancellationSubscription = combinedCancellationTokenSource.Token.Register(state =>
+            {
+                var operationState = (ConnectOperationState)state;
+                if (operationState.IsSucceeded)
+                {
+                    return;
+                }
+                DisposeSocket(operationState.Socket);
+            }, callbackState);


There's a potential race condition between setting callbackState.IsSucceeded = true on line 199 and checking operationState.IsSucceeded on line 176. If cancellation occurs between EnsureConnected completing and IsSucceeded being set, the socket could be disposed even though the connection succeeded. Consider setting IsSucceeded = true before calling EnsureConnected, or use a lock/interlocked operation to ensure thread-safe state updates.

We cannot be sure if the connection was really established until we done the aliveness check, so we cannot set the value before calling EnsureConnected.

…were raised while test run

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

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

src/MongoDB.Driver/Core/Misc/OperationCallbackState.cs

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

BorisDog · 2025-12-08T23:22:27Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

+
+            static void EnsureConnected(Socket socket)
+            {
+                bool originalBlockingState = socket.Blocking;


BorisDog · 2025-12-09T01:42:48Z

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs

 #else
                socket.Connect(endPoint);
 #endif
+                if (!callbackState.TryChangeStatusFromInProgress(OperationCallbackState<Socket>.OperationStatus.Done))


Maybe add a unittest for this with mock socket?

BorisDog

LGTM

CSHARP-5798: Implement test to track if any UnobservedTaskExceptions …

6b9eb50

…were raised while test run

sanych-sun requested a review from BorisDog December 8, 2025 21:40

sanych-sun requested a review from a team as a code owner December 8, 2025 21:40

Copilot AI review requested due to automatic review settings December 8, 2025 21:40

sanych-sun added the chore Non–user-facing code changes (tests, build scripts, etc.). label Dec 8, 2025

sanych-sun commented Dec 8, 2025

View reviewed changes

Copilot AI reviewed Dec 8, 2025

View reviewed changes

sanych-sun added 2 commits December 8, 2025 14:13

pr

83e8b52

CSHARP-5798: Implement test to track if any UnobservedTaskExceptions …

1c3449e

…were raised while test run

sanych-sun requested a review from Copilot December 9, 2025 00:21

Copilot AI reviewed Dec 9, 2025

View reviewed changes

src/MongoDB.Driver/Core/Misc/OperationCallbackState.cs Outdated Show resolved Hide resolved

src/MongoDB.Driver/Core/Connections/TcpStreamFactory.cs Outdated Show resolved Hide resolved

PR

b8ab963

BorisDog reviewed Dec 9, 2025

View reviewed changes

BorisDog approved these changes Dec 9, 2025

View reviewed changes

pr

823a0f7

BorisDog approved these changes Dec 9, 2025

View reviewed changes

sanych-sun merged commit e590d18 into mongodb:main Dec 9, 2025
22 of 23 checks passed

sanych-sun deleted the csharp5798 branch December 9, 2025 02:43

		// Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.
		socket.Send(__ensureConnectedBuffer, 0, 0);

-                    // Try to use the socket to ensure it's connected. On MacOS with net6.0 sometimes Connect is completed successfully even after the socket disposal.
-                    socket.Send(__ensureConnectedBuffer, 0, 0);
+                    // Use Poll and Connected to ensure the socket is actually connected.
+                    // This is a cross-platform way to check for a disposed or disconnected socket.
+                    if (!socket.Connected || !socket.Poll(0, SelectMode.SelectWrite))
+                    {
+                        throw new SocketException((int)SocketError.NotConnected);
+                    }

CSHARP-5798: Implement test to track if any UnobservedTaskExceptions were raised while test run #1836

CSHARP-5798: Implement test to track if any UnobservedTaskExceptions were raised while test run #1836

Uh oh!

Conversation

sanych-sun commented Dec 8, 2025

Uh oh!

sanych-sun Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sanych-sun Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sanych-sun Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sanych-sun Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

sanych-sun Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

BorisDog Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

BorisDog Dec 9, 2025

Choose a reason for hiding this comment

Uh oh!

BorisDog left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants