Skip to content

mulit_ev: remove socket references on forget - #22895

Closed
icing wants to merge 4 commits into
curl:masterfrom
icing:multi_ev_socket_cleanup
Closed

icing wants to merge 4 commits into
curl:masterfrom
icing:multi_ev_socket_cleanup

Conversation

@icing

@icing icing commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

When multi_ev is informed that a socket is about to be closed, it cleans up its entry for it (if it exists), but it did not remove it from any remembered "last pollset"s. When the OS reuses the file descriptor, this may lead to socket identity confusion on detecting changes of use.

Remove the socket from any pollset that is registered at the socket entry of multi_ev when a socket is being "forgotten".

Found-by: @cmeister2

When multi_ev is informed that a socket is about to be closed, it
cleans up its entry for it (if it exists), but it did not remove
it from any remembered "last pollset"s. When the OS reuses the
file descriptor, this may lead to socket identity confusion on
detecting changes of use.

Remove the socket from any pollset that is registered at
the socket entry of multi_ev when a socket is being "forgotten".

Found-by: cmeister2
@icing
icing requested review from bagder and cmeister2 September 10, 2026 08:27
@cmeister2
cmeister2 requested a balanced review from Copilot September 10, 2026 08:33

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@icing

icing commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

A wonderful new world we are making...

@cmeister2 cmeister2 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bagder
bagder requested a balanced review from Copilot September 10, 2026 09:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The descriptor-reuse regression lacks automated test coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details
  • Files reviewed: 3/3 changed files
  • Comments generated: 1
  • Review effort level: Balanced

Comment thread lib/multi_ev.c
@bagder bagder closed this in 2dee5ea Sep 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants