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

Jump to bottom

cURL multi transfers not terminating with HTTP2 + future time condition #8626

Closed
foutrelis opened this issue Mar 23, 2022 · 6 comments
Closed

cURL multi transfers not terminating with HTTP2 + future time condition #8626

foutrelis opened this issue Mar 23, 2022 · 6 comments
Assignees
@bagder
Labels
HTTP/2

Comments

@foutrelis
Copy link

I did this

Used 10-at-a-time.c.gz (modified curl example) to simulate part of what pacman from Arch Linux is doing (when parallel downloads are enabled).

I am seeing 100% CPU usage and dload_progress_cb being called repeatedly, with curl_multi_perform reporting a couple active transfers.

I expected the following

All 10 downloads to succeed, which they do if I leave CURLOPT_TIMECONDITION unset.

Changing the time condition to be in the past (e.g. time(NULL) - 604800) also allows curl to complete all transfers.

curl/libcurl version

$ curl -V
curl 7.82.0 (x86_64-pc-linux-gnu) libcurl/7.82.0 OpenSSL/1.1.1n zlib/1.2.11 brotli/1.0.9 zstd/1.5.2 libidn2/2.3.2 libpsl/0.21.1 (+libidn2/2.3.0) libssh2/1.10.0 nghttp2/1.47.0
Release-Date: 2022-03-05
Protocols: dict file ftp ftps gopher gophers http https imap imaps mqtt pop3 pop3s rtsp scp sftp smb smbs smtp smtps telnet tftp 
Features: alt-svc AsynchDNS brotli GSS-API HSTS HTTP2 HTTPS-proxy IDN IPv6 Kerberos Largefile libz NTLM NTLM_WB PSL SPNEGO SSL TLS-SRP UnixSockets zstd

operating system

$ uname -a
Linux notbad 5.16.16-arch1-1 #1 SMP PREEMPT Mon, 21 Mar 2022 22:59:40 +0000 x86_64 GNU/Linux

$ pacman -Q curl libnghttp2 openssl
curl 7.82.0-1
libnghttp2 1.47.0-1
openssl 1.1.1.n-1
@bagder bagder added the HTTP/2 label Mar 23, 2022
@bagder
Copy link
Member

bagder commented Mar 23, 2022

You mean something like the equivalent of this command line?

curl 'https://curl.se/index.html#[1-10]' -z "Wed, 26 Jan 2026 09:09:39 GMT" -v -o /dev/null -Z

@foutrelis
Copy link
Author

Indeed, something to this effect:

curl 'https://pkgbuild.com/~foutrelis/curl-parallel-test/{testing,core,extra,community-testing,community}.db{,.sig}' \
     -z "Wed, 26 Jan 2026 09:09:39 GMT" -o /dev/null -Z --parallel-max 5

However, I am unable to repro the hang @ 100% CPU with straight curl, only with the example I've attached (which mimics how pacman does parallel downloads). Note that the attached 10-at-a-time.c.gz doesn't always hang but it's easy to repro for me after a few tries.

@bagder bagder self-assigned this Mar 31, 2022
@bagder
Copy link
Member

bagder commented Mar 31, 2022

Thanks, I can reproduce the issue with the provided recipe.

bagder added a commit that referenced this issue Mar 31, 2022
@bagder
http: stop the transfer better for a met time condition
9fd0270 
When getting a 200 response but a "soft check" of the time says the
transfer should abort, it needs to:

1. properly switch off reading
2. close the stream, not the connection - in the case of h2

Reported-by: Evangelos Foutras
Fixes #8626
@bagder bagder mentioned this issue Mar 31, 2022
Closed
@foutrelis
Copy link
Author

foutrelis commented Mar 31, 2022 •
edited

Applying 9fd0270 on top of 7.82.0 does NOT seem to fix the issue for me. I'll check with a local master build next.

Edit: Same behavior with master + 9fd0270 (no change from the original issue).

@bagder
Copy link
Member

bagder commented Mar 31, 2022

curious, because the problem doesn't reproduce for me anymore with this fix. I've run the example many times now

@bagder
Copy link
Member

bagder commented Mar 31, 2022

Update: nah, I was mistaken. I had edits in the code that made it succeed. The problem is still there with #8662 present.

bagder added a commit that referenced this issue Apr 1, 2022
@bagder
http2: handle DONE called for the paused stream
6175c91 
As it could otherwise stall all streams on the connection

Reported-by: Evangelos Foutras
Fixes #8626
@bagder bagder mentioned this issue Apr 1, 2022
Closed
@bagder bagder linked a pull request Apr 1, 2022 that will close this issue
http2: fix handling of client-initiated "done" #8664
Closed
@bagder bagder closed this as completed in b5a9680 Apr 1, 2022
@CvH CvH mentioned this issue Apr 15, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bagder bagder

Labels
HTTP/2
Milestone
No milestone
2 participants
@foutrelis @bagder