Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix "double socket close" issue with Windows version of TCPConnection #4437

Merged
merged 1 commit into from
Sep 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions .release-notes/4437.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
## Fix "double socket close" issue with Windows version of TCPConnection

When fixing a number of "smaller" Windows TCP networking issues a couple years back, in addition to fixing those issues, we introduced a new bug. That bug lingered for two years. It lingered in large part because it would only become apparent in a low resource environment.

When we recently switched our Windows CI from CirrusCI to GitHub Actions, we went from a high-resource environment to a low-resource environment and started getting a ton of "random" Windows TCP test failures.

The problem that was fairly easy to recreate in a test environment would be fairly unlikely in most applications but existed nontheless. The scenario in our test environment was like this:

- Test 1 runs and completes but hasn't done test teardown yet
- Test 2 starts up
- Test 1 runs the "buggy" line of code and closes the socket it has been using with OS, but doesn't reset its own internal record of the file descriptor for the socket.
- Test 2 is gets a socket from the OS with the file descriptor for the socket just closed by Test 1
- Test 1 still has a "valid" file descriptor and as part of full shutdown, closes the socket associated with "its" file descriptor. When Test 1 does this, test 2's socket closes and the test fails to complete successfully.

The problem would appear "in the wild" if a Windows application was quickly closing and opening TCP sockets in a manner similiar to the Pony standard library TCP tests.
6 changes: 1 addition & 5 deletions packages/net/tcp_connection.pony
Original file line number Diff line number Diff line change
Expand Up @@ -1059,11 +1059,7 @@ actor TCPConnection is AsioEventNotify
_shutdown = true

if _connected then
ifdef windows then
@pony_os_socket_close(_fd)
else
@pony_os_socket_shutdown(_fd)
end
@pony_os_socket_shutdown(_fd)
else
_shutdown_peer = true
end
Expand Down
Loading