Broken curl_global_init/curl_global_cleanup ref-counting #4636
Comments
|
Confirmed. But then again, when init fails there's really no given mitigation for an application so I presume very few user ever actually try to call it again and expect it to work... |
|
Well, the "GLOBAL CONSTANTS" section of https://curl.haxx.se/libcurl/c/libcurl.html goes into detail explaining how to initialize libcurl in an application that uses modules/plugins/... Exactly the use case described there will be problematic with the current implementation, because the second module trying to initialize libcurl will see a spurious success code. So I guess the true reason this isn't a problem is that initialization very rarely fails (if at all), except for when it's a configuration problem (e.g. a required dependency missing on the system). And in that case (configuration problem), most computer users I know simply accept any kind of misbehavior from an application, even including crashes. However for our use case that's not an acceptable solution. |
... so that failures in the global init function don't count as a working init and it can then be called again. Reported-by: Paul Groke Fixes #4636
... so that failures in the global init function don't count as a working init and it can then be called again. Reported-by: Paul Groke Fixes #4636
I did this
I checked the
curl_global_init/curl_global_cleanupsource code to find out if I was supposed to callcurl_global_cleanupafter getting a non-successful return code fromcurl_global_init.I expected the following
I expected to find to find an implementation that expects me to not call
curl_global_cleanupafter a failed init attempt, because that's the "usual way to do it" (all or nothing, you can't "release" what you didn't "acquire"). OR an implementation that expects me to callcurl_global_cleanup, but at least keeps track of the initialization state, so that only the parts that were really initialized in the failed init attempt are cleaned up.What I found
The
initializedcounter is immediately incremented, before any initialization is attempted. After that there are multiple error-exit points, none of which reset theinitializedcounter. After that the library is in a state wherecurl_global_initagain will immediately "succeed", but not change anything. This is bad, because if the library is used in a module/plugin/..., another part of the application may callcurl_global_init, get the "success" return code, think all is fine and go ahead to try to use it.curl_global_cleanupwill restore theinitializedcounter back to zero, but it may also call cleanup functions of other libraries who's corresponding init function was not even called (or at least not successfully). This again is bad in a module/plugin/..., because if the module that tried to initialize curl callscurl_global_cleanupto try to "undo" the damage to theinitializedcounter, curl may call the cleanup function of one of its dependencies, without having initialized it, thereby potentially undoing the initialization of this dependency that some other module may have done before.curl/libcurl version
Current master, and blame says it's been this way forever (more than 10 years).
operating system
All
The text was updated successfully, but these errors were encountered: