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

adjust_pollset improvements #12640

Closed
wants to merge 2 commits into from
Closed

Conversation

icing
Copy link
Contributor

@icing icing commented Jan 5, 2024

There is the possibility in the existing implementation that for HTTP/2 the pollset adjustment could still lead to cpu busy loops. If HTTP/2 flow control blocks sending, the h2 filter would replace a POLLOUT in the pollset adjustment with a POLLIN. But the underlying cf-socket filter would still add a POLLOUT. A writeable socket would then lead to sends that are blocked in the HTTP/2 filters.

Fix:

  • let multi_getsock() initialize the pollset in what the transfer state requires in regards to SEND/RECV
  • change connection filters adjust_pollset() implementation to react on the presence of POLLIN/-OUT in the pollset and no longer check CURL_WANT_SEND/CURL_WANT_RECV
  • cf-socket will no longer add POLLIN on its own
  • http2 and http/3 filters will only do adjustments if the passed pollset wants to POLLIN/OUT for the transfer on the socket. This is similar to the HTTP/2 proxy filter and works in stacked filters.

- let `multi_getsock()` initialize the pollset in what the
  transfer state requires in regards to SEND/RECV
- change connection filters `adjust_pollset()` implementation
  to react on the presence of POLLIN/-OUT in the pollset and
  no longer check CURL_WANT_SEND/CURL_WANT_RECV
- cf-socket will no longer add POLLIN on its own
- http2 and http/3 filters will only do adjustments if the
  passed pollset wants to POLLIN/OUT for the transfer on
  the socket. This is similar to the HTTP/2 proxy filter
  and works in stacked filters.
@bagder bagder closed this in a0f9480 Jan 7, 2024
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.

None yet

2 participants