OctopusDeploy / Halibut

| Public | A secure communication stack for .NET using JSON-RPC over SSL.
Other
12 stars 44 forks source link

Create TcpClient.CloseImmediately extension method #476

Closed acodrington closed 1 year ago

acodrington commented 1 year ago

Background

This PR introduces a CloseImmediately extension method for TcpClient, which ensures that no blocking I/O is performed upon trying to close the client.

After some investigation, we were able to clearly show via WireShark that while TcpClient.Close() returns almost immediately, the OS has not actually closed the socket and is still trying to send data. This can cause stability issues on the build server when running tests, as sockets are being held open at the OS level longer than expected.

The new CloseImmediately extension method first closes the internal Socket manually, explicitly passing a 0 timeout, then calls the regular TcpClient.Close() method. This ensures that the client immediately kills the connection, regardless of pending operations, freeing up the socket as expected. This behaviour was verified via WireShark, as well as by a new automated test which has been added.

A new integration fixture TrustFixture has also been added, which exercises the only non-obsolete code path which will make use of this change.

Results

Fixes [sc-57237]

Related to https://github.com/OctopusDeploy/Issues/issues/8266

Before

TcpClient client = ...

// Setup code

Logger.Information("Writer has blocked, closing...");
var stopWatch = Stopwatch.StartNew();
client.Close();
stopWatch.Stop();
Logger.Information($"Close completed, duration: {stopWatch.Elapsed.TotalMilliseconds}ms");

20230825-152618_rider64_0BgbNgwFCJ

20230825-151934_Wireshark_HH3NRZBxP6

After

TcpClient client = ...

// Setup code

Logger.Information("Writer has blocked, closing...");
var stopWatch = Stopwatch.StartNew();
client.CloseImmediately();
stopWatch.Stop();
Logger.Information($"Close completed, duration: {stopWatch.Elapsed.TotalMilliseconds}ms");

20230825-163036_rider64_yOp1paJHsW

20230825-162414_Wireshark_bYOYUA2bBd

shortcut-integration[bot] commented 1 year ago

This pull request has been linked to Shortcut Story #57237: SecureListener.Disconnect has no async version.