libcurl.dll memory leak

[southernedge]

I did this

Hello, I would like to contact the developers.
I noticed a memory leak in libcurl.dll (libcurl-x64.dll) a long time ago. This is version independent.
Here is a small MS Visual Studio C test project showing this problem.
Downloading a file occurs in an endless loop. To speed up the manifestation of memory leaks, the download occurs in 64 threads.
Leave the program running for 15-30 minutes and you will see how memory consumption will increase many times over.
If there is my error and I do not release curl resources in a timely manner, then please suggest a solution to this problem.
Thank you very much for your work, attention and time spent!

I expected the following

No response

curl/libcurl version

8.4.0

operating system

Windows any

[bagder]

Please provide a stand-alone source code file that reproduces the problem, ideally without Windows specific code so that we can try it elsewhere.

You say it leaks in any libcurl version and I find it hard to believe that you tried most of them. Which versions did you use?

[southernedge]

For this test I used 8.4.0
The test code itself is _TestCurlLeakMem.cpp module.

[bagder]

If you cannot make a smaller and less Windows-centric recipe (I don't use Windows), then perhaps you can instead give us much more details about this claimed leak. What memory is leaking? Where is it allocated? How much?

(We test for memory leaks extensively, in all versions and all commits all the time.)

[gvanem]

You do not seems to call CInet::curl_global_cleanup() anywhere and LoadDll() loads multiple libcurls.
And this:

CInet::~CInet( void )
{
}

should at least call FreeLibrary().

[southernedge]

If you cannot make a smaller and less Windows-centric recipe (I don't use Windows), then perhaps you can instead give us much more details about this claimed leak. What memory is leaking? Where is it allocated? How much?

(We test for memory leaks extensively, in all versions and all commits all the time.)

I cannot give you an exact answer to your questions. Memory is leaking inside the dll module. The Visual Studio debugger does not show memory leaks inside external dll modules.
I see the memory increase after each file download using Windows Task Manager. The memory grows with each file download and does not return to its begin state.
The test program does not allocate memory at all. It's running idle.
It would be good if you took a quick look at the text of the module to see if there is an error on my part.

The memory leak problem is not noticeable if the command line curl.exe version is running - file downloaded and curl.exe exits.
But, if a dll library is used in an application for multiple downloads, when tens or hundreds of files can be downloaded in a cycle, then the memory leak becomes very noticeable.

[southernedge]

You do not seems to call CInet::curl_global_cleanup() anywhere and LoadDll() loads multiple libcurls.

• libcurl.dll is loaded once at the start of the program.
• In Windows, you can load a dll library any number of times, but physically it will be loaded once.
• As a rule, Windows does not unload the dll library from memory after FreeLibrary, so this function is of little use.

[bagder]

It would be good if you took a quick look at the text of the module to see if there is an error on my part.

It's not an easily read program. I did not spot a problem.

[southernedge]

It would be good if you took a quick look at the text of the module to see if there is an error on my part.

It's not an easily read program. I did not spot a problem.

Step by step it looks like this:

• get the file length
• divide it by the number of threads (64 thread)
• launch 64 threads.
• each thread will read one piece of the file
• expect all threads to be completed
• endless cycle

[southernedge]

static size_t WriteCallback( void *contents, size_t size, size_t nmemb, void *userp )
{
	WriteCallback_param *pParam = (WriteCallback_param*)userp;
	if ( pParam == NULL ) return 0;
	pParam->m_read += size * nmemb;
	if ( pParam->m_szMaxData > 0 && pParam->m_szMaxData < pParam->m_read ) return 0;
	return size * nmemb;
}

This callback function does not save anything inside the program, but there is a gradual increase in the memory.

[southernedge]

Dear Windows users,
could you test this program and confirm or deny the memory leak?

[bagder]

This callback function does not save anything inside the program, but there is a gradual increase in the memory.

If so, then if you just edit the simple.c example and add your callback in that in a single transfer in a single thread, that also leaks memory?

[southernedge]

If so, then if you just edit the simple.c example and add your callback in that in a single transfer in a single thread, that also leaks memory?
Ok, I will do some more testing with your example. However, this will take some time because downloading with one thread slowly leaked memory.
I will report you the results of the test.
Thank you!

[southernedge]

Here is my test code.

Difficult to do a test for downloading a large file. I took a small 1.3MB file for the test.
Test - 1000 downloads.
The initial program memory is ~1.600MB.
The memory after 1000 downloads is ~6.700MB
Growth ~5MB!

I can not explain it.

static size_t WriteCallback( void *contents, size_t size, size_t nmemb, void *userp )
{
	return size * nmemb;
}

UINT WINAPI OnThread_Download( PVOID pData )
{
	CURL *curl;
	CURLcode res;
	
	curl = CInet::curl_easy_init();
	if(curl) {
		CInet::curl_easy_setopt(curl, CURLOPT_URL, "https://download.sysinternals.com/files/DebugView.zip" );
		/* example.com is redirected, so we tell libcurl to follow redirection */
		CInet::curl_easy_setopt(curl, CURLOPT_FOLLOWLOCATION, 1L);
		res = CInet::curl_easy_setopt( curl, CURLOPT_WRITEFUNCTION, WriteCallback );	_ASSERTE( res == CURLE_OK );
		/* Perform the request, res will get the return code */
		res = CInet::curl_easy_perform(curl);
		/* always cleanup */
		CInet::curl_easy_cleanup(curl);
	}
	return 0;	
}

int main(void)
{
	if ( CInet::Init() == FALSE )
		return 0;

	int nDownloads = 0;
	while ( 1 )
	{
		printf( "Start=%d ", nDownloads + 1 );
		HANDLE hThread = ::CreateThread(	NULL, 0, (LPTHREAD_START_ROUTINE)OnThread_Download, nullptr, 0, NULL );
		DWORD dwObject = WaitForSingleObject( hThread, INFINITE );
		CloseHandle( hThread );
		printf( "Downloads=%d\n", ++nDownloads );
		//Sleep( 5000 );
	}
  return 0;
}

I've edited the code-formatting. It looked like shit. Better?

[southernedge]

If I comment out this line, there is no memory leak.
//res = CInet::curl_easy_perform(curl);

[bagder]

I don't believe there is a memory-leak in curl when running this program, but I also cannot exactly explain the growth. (Based on the fact that it is so simple and there are literally many thousands of users using this simple transfers without seeing any leaks, include all the testing we do ourselves.)

If you run it 2000 laps, does it then grow the size by 10MB so it is consistently 5KB per transfer?

Is there no valgrind-like tool on Windows to pinpoint a memory leak?

[bagder]

Which TLS backend does your curl version use and which version of the library?

[nico-abram]

I tried running the simplified test program and could not reproduce, memory stays fairly stable around 1MB. I did not use the libcurl.dll in the google drive folder, I compiled my own from the tip of the master branch.

[southernedge]

Here is my screenshot - 14MB!
I can't solve this riddle yet. The profiler does not tell which library is actively using memory.

[southernedge]

I tried running the simplified test program and could not reproduce, memory stays fairly stable around 1MB. I did not use the libcurl.dll in the google drive folder, I compiled my own from the tip of the master branch. !)

Could you upload your test project for test?

[nico-abram]

Could you upload your test project for test?

Sure @southernedg
TestCurlLeakMem.zip

[southernedge]

Could you upload your test project for test?

Sure @southernedg TestCurlLeakMem.zip

Thanks, but unfortunately I can't load your library.
Microsoft Visual c++ 2015 redistributable package x32 does not help me.
I may have to wait for the official build 8.5.0....

[southernedge]

If you run it 2000 laps, does it then grow the size by 10MB so it is consistently 5KB per transfer?

I set 5KB transfer (curl_easy_setopt( curl, CURLOPT_BUFFERSIZE, 5000 ) ). This did not affect the result in any way.
Start memory: 1500KB
1000laps: 6600KB
2000laps: 12000KB

Is there no valgrind-like tool on Windows to pinpoint a memory leak?

Unfortunately, I don't see a Windows tool that shows memory allocation inside an external library.

[southernedge]

Which TLS backend does your curl version use and which version of the library?

8.4 - memory leak (my current curl dll)
7.45 - no memory leak
7.69 - no memory leak
7.82 - memory leak

[dfandrich]

Unfortunately, I don't see a Windows tool that shows memory allocation inside an external library.

Why not link the application with the static curl library?

[southernedge]

Unfortunately, I don't see a Windows tool that shows memory allocation inside an external library.
Why not link the application with the static curl library?

Because easy to find a dll curl library, but a static library for linking in MS Visual Studio is difficult.
However, I am very happy with the use of the dll libraries. This does not cause any technical problems.

[edmcln]

I tried this and there was no leaks.

D:\curl-8.4.0\winbuild>nmake /f Makefile.vc mode=dll VC=14 ENABLE_UNICODE=yes DEBUG=yes

// curltest.cpp : This file contains the 'main' function. Program execution begins and ends there.
//

#pragma once

#include <stdio.h>
#include <Windows.h>
#include <tchar.h>
#define _CRTDBG_MAP_ALLOC
#include <crtdbg.h>
#include "D:\curl-8.4.0\builds\libcurl-vc14-x64-debug-dll-ipv6-sspi-schannel\include\curl\curl.h"
#pragma comment( lib, "D:\\curl-8.4.0\\builds\\libcurl-vc14-x64-debug-dll-ipv6-sspi-schannel\\lib\\libcurl_debug.lib" )

DWORD WINAPI CheckUrlThread(LPVOID pArguments)
{
	CURL *crl = curl_easy_init();
	if (crl)
	{
		curl_easy_setopt(crl, CURLOPT_URL, "https://github.com/curl/curl/raw/master/.cirrus.yml");
		curl_easy_setopt(crl, CURLOPT_FOLLOWLOCATION, 1L);
		curl_easy_perform(crl);
		curl_easy_cleanup(crl);
	}
	return 0;
}

int _tmain(int argc, _TCHAR* argv[])
{
	_CrtMemState MemInitial;
	_CrtMemState MemCurrent;
	_CrtMemState MemDiff;

	_CrtMemCheckpoint(&MemInitial);

	HANDLE h[8] = { 0 };
	for (size_t t = 0; t < 8; t++)
	{
		h[t] = CreateThread(NULL, 0, &CheckUrlThread, NULL, 0, NULL);
	}

	WaitForMultipleObjects(8, h, TRUE, INFINITE);

	for (size_t t = 0; t < 8; t++)
	{
		if (h[t]) CloseHandle(h[t]);
	}

	_CrtMemCheckpoint(&MemCurrent);
	
	if (_CrtMemDifference(&MemDiff, &MemInitial, &MemCurrent))
	{
		_CrtDumpMemoryLeaks();
	}

	else
		_tprintf(TEXT("\nNo Leaks ...\n"));
	return 0;
}

[jay]

I've created a branch jay:curl:crtdbg that makes it possible to use Windows CRT heap memory tracking to detect leaks in libcurl and the application, with filenames and line numbers recorded for the leaks. Instructions are in winbuild/README_HEAP_DEBUG.md. If you enable Windows CRT heap memory tracking without using that branch then the CRT will not show the filename or line numbers if the leak is from libcurl.

Your _TestCurlLeakMem.cpp sets _CrtSetDbgFlag which overrides the default flags. That means _CRTDBG_ALLOC_MEM_DF which defaults to ON will not actually be on unless it is passed as a flag. Ideally we need a minimal self contained example that can be used to reproduce. In other words a single file that we can compile and link to libcurl.

[southernedge]

I did more research.
To do this, I had to build the curl dll myself and link it with different libraries and build keys.
Here is the result of my last test

Memory leaks -
nmake.exe /f Makefile.vc mode=dll VC=14 ENABLE_UNICODE=yes WITH_ZLIB=static WITH_NGHTTP2=static WITH_SSL=static
No memory leaks -
nmake.exe /f Makefile.vc mode=dll VC=14 ENABLE_UNICODE=yes WITH_ZLIB=static WITH_NGHTTP2=static

Presumably the build with the WITH_SSL key causes a memory leak. I use the latest openssl-3.2.0.

Can you comment on this and what else could I check to understand the problem?

[jay]

Possibly related #5282. In short if you use OpenSSL as a static library then you may have to call OPENSSL_thread_stop:

"Similarly this message will also not be sent if OpenSSL is linked statically, and therefore applications using static linking should also call OPENSSL_thread_stop() on each thread. Additionally if OpenSSL is loaded dynamically via LoadLibrary() and the threads are not destroyed until after FreeLibrary() is called then each thread should call OPENSSL_thread_stop() prior to the FreeLibrary() call."

[southernedge]

Possibly related #5282. In short if you use OpenSSL as a static library then you may have to call OPENSSL_thread_stop:

"Similarly this message will also not be sent if OpenSSL is linked statically, and therefore applications using static linking should also call OPENSSL_thread_stop() on each thread. Additionally if OpenSSL is loaded dynamically via LoadLibrary() and the threads are not destroyed until after FreeLibrary() is called then each thread should call OPENSSL_thread_stop() prior to the FreeLibrary() call."

Thank you for your advise!

In order to follow your advice I had to add the following lines to your (curl) code and build it again.

CURL_EXTERN void curl_ossl_thread_stop();
...
void curl_ssl_thread_stop()
{
  OPENSSL_thread_stop();
}

Yes, it seems to work. I don't see any memory leaks right now.

Now I have the next question. Can you provide this function are declared in the new implementation of curl?
This function has no meaning for сurl.exe, but it is very important for curl.dll. curl.dll is often used for multi-thread downloading in different projects.
Otherwise, I will have to add these lines to a new version of curl every time and build curl myself for my needs.

Thanks.

[jay]

No. In my opinion this is not a curl issue. If you choose to use static OpenSSL then you are responsible for cleanup. I suppose we could add something to the multithreading document that mentions this.

[southernedge]

No. In my opinion this is not a curl issue. If you choose to use static OpenSSL then you are responsible for cleanup. I suppose we could add something to the multithreading document that mentions this.

The user's project uses curl exclusively (it does not directly use OpenSSL). curl is built with and uses OpenSSL support. It is logical for curl to allow the user to free up OpenSSL resources rather than supply OpenSSL dlls with the user project for the sake of one function OPENSSL_thread_stop.

Once again in different words.
I'm faced with a choice:

• independently change the curl code for the sake of one function OPENSSL_thread_stop, in order to free up resources after curl is running.
• or add OpenSSL dlls to my project build to call only one function OPENSSL_thread_stop.

I don't like both solutions, but I don't like option 2 more.

I'm not sure I explained the side project problem clearly, but it's significant.

[bagder]

I suggest you take OpenSSL complaints to the OpenSSL team.

I don't see any good way for curl to add this call, as it would require that the libcurl source code would need get told if the linking is static or shared. Which would be problematic.

[jay]

I don't see any good way for curl to add this call, as it would require that the libcurl source code would need get told if the linking is static or shared. Which would be problematic.

I asked OpenSSL if OPENSSL_thread_stop can be called multiple times. openssl/openssl#22831

If it can then for Windows we could add a OPENSSL_thread_stop call on thread detach, even if OpenSSL is also a DLL and does the same call on thread detach.

In any case if libcurl is a static library and openssl is a static library then there's nothing we can do, it's up to the application or dll that's it's linked to.

[southernedge]

Ok, sad, thank you.

[jay]

I've updated #12408 to call OPENSSL_thread_stop on DLL_THREAD_DETACH and add a warning to libcurl-thread explaining that OPENSSL_thread_stop may be required in some unusual cases.

[bagder]

@jay do you still intend to take this issue somewhere?

[jay]

@jay do you still intend to take this issue somewhere?

Yes. The PR took more time than I expected so I moved on to other things. I still like it though and will try Viktor's suggestions.