-
-
Notifications
You must be signed in to change notification settings - Fork 6.7k
Memory leak when cancelling paused http 2 transfer #10971
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
Comments
This is the code that is not executed for this case and thus the leak happens: Lines 722 to 727 in b32b7bb
The question is why not. Lines 704 to 710 in b32b7bb
... which then makes it return before the free, if the connection is still in use! We should probably try to move the free calls to before that check and see if that doesn't fix it. @lemourin, is it hard to reproduce for you or would you be able to try out a patch/PR to check? (Only h2 and h3 would do that caching as it is due to how the stream windows work, for h1 there is no such caching done.) |
Before checking for more users of the connection and possibly bailing out. Fixes #10971 Reported-by: Paweł Wegner
#10972 is an attempt to fix this. Any chance you can test this on your case? |
I tested with my test program, looks good to me. Thanks for the quick fix! |
I did more testing today with my real app coro-cloudstorage and I noticed that the downloads are now getting stuck. I can provide more context on what the app does if necessary; or try to write a smaller repro. Without #10972 it leaks, with #10972 the connections get stuck. EDIT: changed the test program a bit, it now resumes some transfers; with the memory leak fix one transfer gets stuck and returns "Stream error in the HTTP/2 framing layer" after ~5 minutes |
Before checking for more users of the connection and possibly bailing out. Fixes curl#10971 Reported-by: Paweł Wegner Closes curl#10972
I did this
I wrote the following program which at the high level does the following:
Essentially it tries to cancel requests after they were paused.
This results in the following memory leak when running with address sanitizer:
Interestingly enough, the memory leak doesn't occur when forcing http version 1.1. Possibly the issue could be specific to http 2.
I expected the following
No memory leaks.
curl/libcurl version
libcurl/7.88.1-DEV OpenSSL/3.0.8 zlib/1.2.13 c-ares/1.19.0 nghttp2/1.51.0
operating system
Linux razer 6.1.23-1-MANJARO #1 SMP PREEMPT_DYNAMIC Thu Apr 6 19:47:04 UTC 2023 x86_64 GNU/Linux
Happens on Windows as well.
The text was updated successfully, but these errors were encountered: