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

HTTP2 requests through multi interface might not properly finish #12971

Closed
5533asdg opened this issue Feb 22, 2024 · 4 comments
Closed

HTTP2 requests through multi interface might not properly finish #12971

5533asdg opened this issue Feb 22, 2024 · 4 comments

Comments

@5533asdg
Copy link

I did this

The issue is observed after upgrading curl from 8.5.0 to 8.6.0.
Specifically, the behavior change is seen from d7b6ce6.

When issuing queries, we might not get the state change from PERFORMING to DONE.

The currently seen behavior on failure is*:

  1. HTTPS/2 request is sent through curl_multi_add_handle(...).
  2. The request is completed and drain_steam(...) (https://github.com/curl/curl/blob/master/lib/http2.c#L212) is called.
    • data->state.select_bits is set to CURL_CSELECT_IN.
    • Curl_expire(data, 0, EXPIRE_RUN_NOW); is called.
  3. Client's epoll returns the socket to be writable but not readable.
  4. curl_multi_socket_action(..., CURL_CSELECT_OUT, ...) is called by the client.
    • data->state.select_bits is set to CURL_CSELECT_OUT (overriding libcurl operation on step 2).
  5. CURLMOPT_TIMERFUNCTION is called as a result of step 2.
  6. readwrite_data(...) is not called because data->state.select_bits == CURL_CSELECT_OUT and k->keepon == KEEP_RECV.

The currently seen behavior on success is*:

  1. HTTPS/2 request is sent through curl_multi_add_handle(...).
  2. The request is completed and drain_steam(...) (https://github.com/curl/curl/blob/master/lib/http2.c#L212) is called.
    • data->state.select_bits is set to CURL_CSELECT_IN.
    • Curl_expire(data, 0, EXPIRE_RUN_NOW); is called.
  3. CURLMOPT_TIMERFUNCTION is called as a result of step 2.
  4. readwrite_data(...) is called because data->state.select_bits == CURL_CSELECT_IN and k->keepon == KEEP_RECV.
    • Curl gets EndOfSteam (EOS) / nread == 0 from readwrite_data(...).
  5. Curl updated the state from PERFORMING to DONE.

*The difference is whether curl_multi_socket_action(..., CURL_CSELECT_OUT, ...) or CURLMOPT_TIMERFUNCTION is called first.

The previous behavior is (< 8.6.0):

  1. HTTPS/2 request is sent through curl_multi_add_handle(...).
  2. The request is completed and drain_steam(...) (https://github.com/curl/curl/blob/master/lib/http2.c#L212) is called.
    • data->state.select_bits is set to CURL_CSELECT_IN.
    • Curl_expire(data, 0, EXPIRE_RUN_NOW); is called.
  3. Curl updated the state from PERFORMING to DONE within the same multi_socket(...) calls as step 2.
    • As such, it is not possible for the client to call curl_multi_socket_action(..., CURL_CSELECT_OUT, ...).

From my understanding, the behavior change is the result of 2944e46.

I expected the following

Curl to always finish the request even if curl_multi_socket_action(..., CURL_CSELECT_OUT, ...) is called.

curl/libcurl version

WARNING: this libcurl is Debug-enabled, do not use in production

curl 8.6.0-DEV (x86_64-cros-linux-gnu) libcurl/8.6.0-DEV OpenSSL/3.2.0 zlib/1.2.13 c-ares/1.19.1 libpsl/0.21.1 nghttp2/1.57.0
Release-Date: [unreleased]
Protocols: dict file http https ipfs ipns mqtt
Features: alt-svc AsynchDNS Debug GSS-API HSTS HTTP2 HTTPS-proxy IPv6 Kerberos Largefile libz NTLM PSL SPNEGO SSL threadsafe TLS-SRP TrackMemory UnixSockets

operating system

ChromeOS

$ uname -a
Linux localhost 5.15.147-21486-g7e7afc4ef5b4 #1 SMP PREEMPT Tue, 16 Jan 2024 22:08:30 +0000 x86_64 Intel(R) Core(TM) i3-10110U CPU @ 2.10GHz GenuineIntel GNU/Linux

@5533asdg
Copy link
Author

As an additional note, the same behavior is observed even if the curl_multi_socket_action(..., CURL_CSELECT_OUT, ...) calls are replaced with curl_multi_socket_action(..., 0, ...).
Curl is also not seeing the socket as readable, resulting in the same failing behavior.

icing added a commit to icing/curl that referenced this issue Feb 22, 2024
- refs curl#12971
- add the event bitmask to data->state.select_bits instead
  of overwriting them. They are cleared again on use.
@icing
Copy link
Contributor

icing commented Feb 22, 2024

Could you verify if #12972 fixes the issue for you?

@5533asdg
Copy link
Author

Yep, I can confirm that it fixes the issue. Thanks!

@icing
Copy link
Contributor

icing commented Feb 22, 2024

Thanks for verifying.

@bagder bagder closed this as completed in f274fc5 Feb 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants