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 socket watching read and write conflict. #4430

Draft
wants to merge 5 commits into
base: 25.lts.1+
Choose a base branch
from

Conversation

jellefoks
Copy link
Member

@jellefoks jellefoks commented Nov 14, 2024

The TCPSocketStarboard and UDPSocketStarboard classes used one single socket watcher for watching both reading and writing without tracking which interests were being tracked.

A single watcher was used because the underlying platform code does not allow watching the same descriptor twice, but the existing logic for determining when to stop watching was fragile for moments when a socket was being watched for both reading and writing. This tracks the interests explicitly to make sure the watcher is registered with the correct interests, and registered when no interests are left.

Before this change, if a socket needed a write interest at the same time as a read interest, both interests would remain active and with their possible callbacks until they both were satisfied.

This may fix the 3rd top crasher, and should improve performance by avoiding spurious callbacks for obsolete interests.

Note: Socket watching is done when a socket either is not ready for sending or has no data to receive. Socket watching serves to get a callback when that condition changes. The socket watching is registered with an interest, indicating whether the registrant is interested in reading or writing from/to the socket.

b/361151407

@jellefoks jellefoks marked this pull request as draft November 14, 2024 01:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant