fix: make PooledSocket.DisposeSocket thread-safe

[XiLiuRoy]

Problem

PooledSocket.Connect() registers a CancellationTokenSource callback (Cancel()) to enforce the connection timeout. When the timeout fires, Cancel() calls DisposeSocket() on a thread-pool thread. The same code path calls DisposeSocket() again on the caller thread when the post-connect check finds the socket is not connected — with no protection against both running concurrently.

This creates a race with two failure modes:

1. Double-dispose — both threads see _socket != null, both call _socket.Dispose(), the second call throws ObjectDisposedException.
2. NullReferenceException — Cancel() sets _socket = null between the null-check and _socket.Dispose() on the caller thread.

These races are low-probability under normal conditions but reproducible under connection-timeout pressure (e.g. connecting to a slow or firewalled host) and more likely on ARM / weakly-ordered architectures.

Fix

Rewrite DisposeSocket() to use Interlocked.Exchange(ref _socket, null):

private protected void DisposeSocket()
{
    _isSocketDisposed = true;
    var socket = Interlocked.Exchange(ref _socket, null);
    socket?.Dispose();
}

Interlocked.Exchange atomically captures the socket reference and nulls the field in one operation. Exactly one concurrent caller gets the non-null reference and disposes it; all others receive null and no-op. This makes the method safe to call concurrently from any number of threads.

The visibility is changed from private to private protected so the method can be exercised directly in tests (via a thin subclass in the test project) without reflection. InternalsVisibleTo is scoped to the test assembly only, using the full public key to respect the existing strong-name signing.

Testing

Added PooledSocketRaceReproTests which calls DisposeSocket() from 64 concurrent tasks and asserts no exceptions are raised.

Made with Cursor :)

[cnblogs-dudu]

Thanks for your contribution!