mbedtls not safe for multithreaded

[xydone]

Hey!
Currently, as far as my understanding goes, with the macro MBEDTLS_THREADING_C not enabled by default, mbedtls will not run in a thread-safe manner. ^[1][2] This in turn causes issues when making multiple requests at once to inconsistently crash at runtime with hard to debug C-land errors, such as double free or corruption (fasttop), segfaults and malloc_consolidate(): unaligned fastbin chunk detected among others.

The way to resolve it would be to enable said macro, alongside MBEDTLS_THREADING_PTHREAD^[2] or, potentially, MBEDTLS_THREADING_ALT^[4] in the case that pthread is, for some reason, not desired.

The fix is a one liner, but I wanted to raise it as an issue first before making a PR for it, due to the following questions:

• Should multithreading be on by default? I believe the best way of introducing it on by default, but with the ability to turn it off with a build option as there are benefits to not enabling it, such as shipping less code and less dependencies.
• Would pthreads break Windows compatibility? This is something I am unable to answer due to not having a Windows machine at hand and not really being knowledgeable on the matter. That being said, we already are linking against pthreads^[5] so I guess this won't be a big issue.

[1] - https://mbed-tls.readthedocs.io/projects/api/en/development/api/file/crypto__config_8h/#c.MBEDTLS_THREADING_C
[2] - https://mbed-tls.readthedocs.io/projects/api/en/development/api/file/crypto__config_8h/#c.MBEDTLS_THREADING_PTHREAD
[3] - https://curl.se/libcurl/c/threadsafe.html#:~:text=mbedTLS%20can%20be%20used%20safely%20in%20a%20multi%2Dthreaded%20environment%20provided%20that%20mbedTLS%20is%20compiled%20with%20MBEDTLS%5FTHREADING%5FC%20enabled%2E
[4] - https://mbed-tls.readthedocs.io/projects/api/en/development/api/file/crypto__config_8h/#c.MBEDTLS_THREADING_ALT
[5] -

curl/build.zig

Lines 228 to 231 in 6b43eca

	if (enable_threaded_resolver) {
	curl.root_module.addCMacro("HAVE_PTHREAD_H", "1");
	curl.root_module.linkSystemLibrary("pthread", .{});
	}

[xydone]

On second thought, this issue would be better suited for upstream. I'll leave it here for now as to not spam issue boards needlessly.

[Techatrix]

Enabling threading when building mbedtls as part of curl makes sense to me.
Does #4 fix your issue?

[xydone]

It looks good to me on paper, I can't verify the with the original code right now, but an issue I encountered when patching this issue for another library was curl not knowing mbedtls was compiled with these flags, which caused every request to lock up indefinitely (presumably mutex shenanigans). The way we fixed it there was providing the same macros to curl as well as mbedtls during compilation, but I believe it should also work fine if mbedtls is compiled before curl. As long as that is not an issue, I believe it should fix multi-threaded.

[Techatrix]

I could not find any occurrence in the curl source code where it checks for MBEDTLS_THREADING_C. I also don't see how that could even affect anything since the curl compilation unit(s) cannot observe with what flags mbedtls was built with.

The closest this I could find is this check that is present in versions of curl before 8.18:
https://github.com/curl/curl/blob/400fffa90f30c7a2dc762fa33009d24851bd2016/lib/vtls/mbedtls.c#L110-L113

Do you have a reproduction of your issue that I could take a look at?

[xydone]

I'm not exactly sure why it occurred either, but in the case of zig-curl, disabling these lines would make every request hang. Naturally, as things go, I have suddenly lost all sort of ability to reproduce it in a small and reasonable manner, but going back to the project which I'm working on which inspired this whole fix, commenting out those lines do indeed cause a freeze for that library. That being said, given the fact you cannot replicate it on here, I assume it might be an issue that manifested there because of the different build process, quite frankly I'm not too knowledgeable on the ins and outs of zig's build system.

[Techatrix]

In the absence of a reproduction or any actionable issue regarding allyourcodebase/curl, I am gong to close this issue.