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

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
Labels

Comments

@foutrelis
Copy link

@foutrelis foutrelis commented Mar 23, 2022

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 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

@foutrelis foutrelis commented Mar 23, 2022

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 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
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
@foutrelis
Copy link
Author

@foutrelis foutrelis commented Mar 31, 2022

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 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 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
As it could otherwise stall all streams on the connection

Reported-by: Evangelos Foutras
Fixes #8626
@bagder bagder linked a pull request Apr 1, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants