Random memory leak while calling curl_multi_cleanup #7683
Comments
|
I fixed the example and increased parallelism (see below) but it doesn't seem to reproduce for me with git master and OpenSSL/1.1.1l. It does reproduce pretty easily with 7.78.0. I thus conclude that this bug doesn't exist anymore. --- issue7683.c~ 2021-09-07 15:26:57.346871588 +0200
+++ issue7683.c 2021-09-07 15:29:29.956228837 +0200
@@ -8,7 +8,7 @@
/* curl stuff */
#include <curl/curl.h>
-#define HANDLECOUNT 2 /* Number of simultaneous transfers */
+#define HANDLECOUNT 20 /* Number of simultaneous transfers */
/* The maximum number of curl_multi_perform iterations to perform before
* calling curl_multi_cleanup.
@@ -44,7 +44,7 @@
CURLMcode mc = curl_multi_perform(multi_handle, &still_running);
if (iteration++ >= MAX_ITERATIONS)
- goto out;
+ break;
if (still_running)
/* wait for activity, timeout or "nothing" */ |
|
Sorry for the I have also tested the leak using the $ git clone https://github.com/curl/curl.git
$ cd curl
$ autoreconf -fi
$ ./configure --enable-shared --disable-static --with-openssl --prefix=/home/demo/test/curl_memleak
$ make -j9
$ make installThen from the test folder I have run the following script: #!/bin/sh
while LD_LIBRARY_PATH=lib valgrind -q --error-exitcode=1 --leak-check=full ./multi_app > /dev/null; do
echo "Retrying..."
doneThe output was: |
|
I just can't seem to make master leak. Your new valgrind output lacks debug symbols in libcurl but I presume it is the same call. This one: Line 1407 in a2f8ec0 The leak is also clearly visible to be an allocation done within OpenSSL and to me it looks like that memory should be freed just a few lines further down in the libcurl code: Line 1412 in a2f8ec0 |
|
I investigated further more and it is very strange what is happening. I I attached the archive curl-memleak.zip with the source code and small shell scripts used for the Conclusions:
Thank you for your time. |
|
Hello @bagder ! After some investigation, I found the "key" configure option that makes the memory leak to appear. It is In the previous post, on my Gentoo system (that my colleague used) the memory leak did appear. However on the very same machine building libcurl Likewise, installing on Debian and Fedora and Ubuntu the As a conclusion, the memory leak does appear on any Linux machine (in a few iterations using the
I hope this information sheds some light on the issue. |
|
Not really. I always use openssl and nghttp2 in my builds and this is what it says for me right now: build detailscurl 7.79.1-DEV (x86_64-pc-linux-gnu) libcurl/7.79.1-DEV OpenSSL/1.1.1l zlib/1.2.11 brotli/1.0.9 zstd/1.4.8 c-ares/1.17.0 libidn2/2.3.2 libpsl/0.21.0 (+libidn2/2.3.0) libssh2/1.9.0_DEV nghttp2/1.45.0-DEV librtmp/2.3 libgsasl/1.10.0 OpenLDAP/2.4.59 |
|
This is weird. I made a build with Almost identical dependencies as yours: @bagder would you mind testing a vanilla Debian 11 QEMU image or a vanilla Debian Docker image (script), if I create one for you, so you would not waste your time by creating it? Which one would you prefer? Thanks. |
|
I'm not sure I need to be able to reproduce this. As mentioned already I think this rather looks like an OpenSSL memory-leak. I don't see that libcurl does anything wrong but openssl leaks that memory. Don't you agree? |
The memory leak is definitely inside OpenSSL (session cache), however the cause is not clear. The fact is, the issue appeared with curl-7.78. Of course, this can mean any of the following:
What bothers me is that we managed to reproduce on several machines with several different stable distributions. It can happen on the field. We have no machines where the issue does not reproduce and that is confusing me. To answer your question: I agree with you. case 3 is only more a theoretical case, in practice it can be excluded as you wrote in your linked comment. Let's close this issue with the following conclusions:
Thank you very much for your input. |
|
This problem is likely triggered by this change that was introduced in 7.78.0: b249592 which is why it isn't visible in earlier libcurl versions. |
|
One last note about investigation under QEMU 6, using vanilla Debian 11.
Probably it is some kind of timing/threading/synchronization issue. Interesting is the |
|
@bagder I found the "missing component" of the reproduction: CPU speed. My computer is "too fast". If I run So the conditions for memory leak to appear:
My desktop system is an i7-8700 CPU @ 3.20GHz for reference. Single core, or a slower CPU will make the memory leak to disappear. |
All TLS handling done by (lib)curl is in the same single thread no matter if there are threads available... |
Sure, I was just documenting all my investigation results here, just in case when someone will encounter the same issue and wants to dig further (outside the libcurl scope). Soon curl 7.78 will be widespread on common distros so the exposure on "fast CPUs" will be higher, and the issue may reappear in future. I just can't get why From our point of view, you may close the issue. Thank you for your time. |
That correlation is very weak and mostly a guesswork though. I ran my tests on a i7-3770K CPU @ 3.50GHz. I think there's another trigger. |
|
@mkauf any ideas |
|
Thank you for the test program. Valgrind reports these leaks on my system too. It has something to do with the callback function |
All the stack traces showing the leak that I've seen here have showed it involving I don't see how curl is responsible for such memory or even can free such memory! If the memory leak would've been done by libcurl code in the callback, then valgrind would've pinpointed that as the culprit. I still think this smells like an OpenSSL memory-leak. |
|
The problem is this check in So if the SSL session cache does not exist anymore, Probably Maybe this low-risk patch can be considered for tomorrow's curl release: |
Thanks for the patch! It's too late for tomorrow though I think. It's not even a PR yet and it seems like we might want to make that change to cover more/all TLS backends. @slydder101 can you try it out and see if it fixes the leak for you? |
|
Thank you very much for the fix! I confirm the patch fixes the leak on all the systems that we have with Gentoo, Fedora, Debian (those from the previous comments, where the leak appeared). Manjaro has to be confirmed by @slydder101 on his system.
@bagder do you think this leak may happen also with mBedTLS as well? I mean, is there any theoretical chance for it, but in practice we did not encountered it? |
Now that @mkauf has done his awesome research and figured out why it happened, I think its a good time to make sure that the fix makes sure that there's no leak left in any of the TLS backends! |
|
I can also confirm that the patch fixes the leak on Manjaro. Thank you for your effort. |
…cache On connection shutdown, a new TLS session ticket may arrive after the SSL session cache has already been destructed. In this case, the new SSL session cannot be added to the SSL session cache. The callers of Curl_ssl_addsessionid() need to know whether the SSL session has been added to the cache. If it has not been added, the reference counter of the SSL session must not be incremented, or memory used by the SSL session must be freed. This is now possible with the new output parameter "added" of Curl_ssl_addsessionid(). Fixes curl#7683
|
Thank you for the feedback! I have created a pull request: #7752 |
Random memory leak is detected when calling
curl_multi_cleanupbefore the associated easy handles are closed.Steps to reproduce.
Build the following sample multi_app application:
Run the application with
valgrindusing the following shell script:Depending on your computer's speed the leak will appear after a few tries:
Environment
curl 7.78openSSL 1.1.1.kandopenSSL 1.1.1.lNotes
The memory leak appeared starting from
curl 7.78with the following commitb249592d29ae0a2b3e8e07fdbc01f33b5a5b8420.
For know I have managed to reproduce it only with
openSSLbackend and not withmbedTLS.To reproduce the issue there needs to be at least two simultaneous connections
and the cleanup operations need to be close in time to exit.
The text was updated successfully, but these errors were encountered: