Curl_resolver_init is not thread-safe
#13065
Comments
- Don't use a static variable to store the ares version. Prior to this change several threads could write the same data to a static int variable at the same time. Though in practice it's not a problem ThreadSanitizer may warn. Reported-by: Nikita Taranov Fixes curl#13065 Closes #xxxxx
|
I don't see get_ares_version in our source or the c-ares api. See #13066 for proposed fix |
- Store the c-ares version during global init. Prior to this change several threads could write the same data to a static int variable at the same time. Though in practice it's not a problem ThreadSanitizer may warn. Reported-by: Nikita Taranov Fixes curl#13065 Closes #xxxxx
|
I'm pretty sure this is still thread safe, even if we should make it done nicer. |
|
thank you, guys |
|
The next release ships on March 27, 2024 and will become version 8.7.0. The pending release notes for the coming release are always available on this URL: https://curl.se/dev/release-notes.html |
|
I'd also love to get your advice on how we should proceed on our end. is it safe to use master version instead of the last release? to get the fix earlier. or maybe we could back-port the fix to the last release branch? |
Do not use master in production. master is our development branch. I suggest use git to cherry pick the fixes you need. |
I believe sticking to the previous version is fine until the next release ships. I think this not thread-safe warning was a warning and it should be and was fixed, but the effects of the problem are totally harmless. |
- Store the c-ares version during global init. Prior to this change several threads could write the same data to a static int variable at the same time. Though in practice it's not a problem ThreadSanitizer may warn. Reported-by: Nikita Taranov Assisted-by: Jay Satiro Fixes curl#13065 Closes curl#13000
- Store the c-ares version during global init. Prior to this change several threads could write the same data to a static int variable at the same time. Though in practice it's not a problem ThreadSanitizer may warn. Reported-by: Nikita Taranov Assisted-by: Jay Satiro Fixes curl#13065 Closes curl#13000
I did this
Hi!
We start getting sanitiser reports like this in our CI.
I assume that this code is supposed to be thread-safe, please correct me if I'm wrong.
If so, seems like the problematic code was introduced recently here:
4224d6e
so static variable
ares_vercould be simultaneously read and modified by two threads.it seems enough to replace l. 176 with
get_ares_version()will do the same call, but now compiler will guarantee single initialisation (btw, I'm not a C expert, do we have this guarantee in C?).I expected the following
no sanitiser complains
curl/libcurl version
curl 8.6.0+
operating system
linux
The text was updated successfully, but these errors were encountered: