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

TCPListener: unsubscribe asio before socket close #1626

Merged
merged 1 commit into from
Mar 3, 2017

Conversation

dipinhora
Copy link
Contributor

This commit changes TCPListener to unsubscribe asio events
prior to closing the socket and file descriptor.

This prevents events from being reported for closed sockets
due to any race conditions related to socket close and
epoll receiving an event between the socket close request
and the actual close occurring.

See: http://stackoverflow.com/questions/8707601/is-it-necessary-to-deregister-a-socket-from-epoll-before-closing-it


In my limited testing, this change results in less failures in the TCP tests with the following output where the server never accepts a connection:

**** FAILED: net/TCPMute
Test timed out without completing
Action never completed: receiver accepted
Action never completed: receiver muted
Action never completed: receiver asks for data
Action never completed: sender sent data
Action never completed: server accept

This failure seems to mostly be due to a combination of fd reuse combined with the lack of unsubscribe before the close which this PR resolves.

This commit changes TCPListener to unsubscribe asio events
prior to closing the socket and file descriptor.

This prevents events from being reported for closed sockets
due to any race conditions related to socket close and
epoll receiving an event between the socket close request
and the actual close occurring.
@jemc jemc added the changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge label Mar 3, 2017
@jemc jemc merged commit 7037e9f into ponylang:master Mar 3, 2017
ponylang-main added a commit that referenced this pull request Mar 3, 2017
@jemc
Copy link
Member

jemc commented Mar 3, 2017

Thanks!

dipinhora added a commit to WallarooLabs/wally that referenced this pull request Sep 28, 2017
Backport ponylang/ponyc#1578 and ponylang/ponyc#1626 to net actors
because the help resolve edge cases around the networking code with
direct impacts to the stability of the network tests.
nisanharamati pushed a commit to WallarooLabs/wally that referenced this pull request Sep 29, 2017
Backport ponylang/ponyc#1578 and ponylang/ponyc#1626 to net actors
because the help resolve edge cases around the networking code with
direct impacts to the stability of the network tests.
dipinhora added a commit to WallarooLabs/wally that referenced this pull request Oct 5, 2017
Backport ponylang/ponyc#1578 and ponylang/ponyc#1626 to net actors
because the help resolve edge cases around the networking code with
direct impacts to the stability of the network tests.
JONBRWN pushed a commit to WallarooLabs/wally that referenced this pull request Oct 12, 2017
Backport ponylang/ponyc#1578 and ponylang/ponyc#1626 to net actors
because the help resolve edge cases around the networking code with
direct impacts to the stability of the network tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - fixed Automatically add "Fixed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants