Describe your environment main at 3fb1d31, Ubuntu, gcc 14, libcurl 8.14.1, CMake.
HttpCurlGlobalInitializer is the one place the curl client initialises libcurl, and it throws away what that call returns. ext/src/http/client/curl/http_client_curl.cc:49:
HttpCurlGlobalInitializer::HttpCurlGlobalInitializer()
{
curl_global_init(CURL_GLOBAL_ALL);
}curl_global_init says it plainly: "If this function returns non-zero, something went wrong and you cannot use the other curl functions."
Nothing records that, so nothing can honour it. HttpClient's constructor takes the singleton and then calls curl_multi_init straight after it, which is one of the functions the sentence above is about, and every session after that reaches curl_easy_init the same way.
What is already fixed and what is not
#4404 is about the curl client treating failed allocations as success, and the branch for it checks what curl_easy_init, curl_slist_append, curl_multi_init, curl_multi_cleanup, curl_multi_remove_handle and curl_multi_add_handle return. It also fixed the ordering here, since multi_handle_ is declared before curl_global_initializer_ and member initialisation order meant curl_multi_init used to run before curl_global_init had.
So the ordering is right now. What is still missing is the condition: libcurl is initialised first, but nobody asks whether it was initialised successfully.
What it would take
Two shapes, and the choice is a design one.
The small one records the result in the singleton and has HttpClient's constructor skip initMultiHandle() when it is not CURLE_OK, reporting once. Everything downstream then behaves the way a client with no multi handle already behaves, which after #4404 means requests are refused with a reason rather than sent into libcurl. It costs one member on HttpCurlGlobalInitializer, which is a concrete class in an installed header, so it moves that layout.
The larger one gives the client a way to refuse construction, which is a wider conversation about what an exporter does when its HTTP client cannot exist.
Why I have not sent a patch
I cannot make curl_global_init fail in the test binary, and I would rather say so than ship a branch nothing runs.
The suite installs its allocator callbacks with curl_global_init_mem from SetUpTestSuite, and that call is the global initialisation: it "works exactly as curl_global_init with one addition". It has to run there, because installing the callbacks after libcurl is initialised is a silent no-op. So by the time any client exists libcurl is already initialised, and the curl_global_init inside the singleton is a reference count bump that returns CURLE_OK whatever the allocator does.
Reaching the failure needs a seam that the client calls instead of curl_global_init directly. That is worth discussing on its own terms, since the same seam would answer the other injection questions in this area, but it is production surface added for testability and not something to slip into a bug fix.
Happy to send either shape once you say which, and to write the seam if that is the direction.
Found while working through #4404.
Describe your environment
mainat 3fb1d31, Ubuntu, gcc 14, libcurl 8.14.1, CMake.HttpCurlGlobalInitializeris the one place the curl client initialises libcurl, and it throws away what that call returns.ext/src/http/client/curl/http_client_curl.cc:49:curl_global_init says it plainly: "If this function returns non-zero, something went wrong and you cannot use the other curl functions."
Nothing records that, so nothing can honour it.
HttpClient's constructor takes the singleton and then callscurl_multi_initstraight after it, which is one of the functions the sentence above is about, and every session after that reachescurl_easy_initthe same way.What is already fixed and what is not
#4404 is about the curl client treating failed allocations as success, and the branch for it checks what
curl_easy_init,curl_slist_append,curl_multi_init,curl_multi_cleanup,curl_multi_remove_handleandcurl_multi_add_handlereturn. It also fixed the ordering here, sincemulti_handle_is declared beforecurl_global_initializer_and member initialisation order meantcurl_multi_initused to run beforecurl_global_inithad.So the ordering is right now. What is still missing is the condition: libcurl is initialised first, but nobody asks whether it was initialised successfully.
What it would take
Two shapes, and the choice is a design one.
The small one records the result in the singleton and has
HttpClient's constructor skipinitMultiHandle()when it is notCURLE_OK, reporting once. Everything downstream then behaves the way a client with no multi handle already behaves, which after #4404 means requests are refused with a reason rather than sent into libcurl. It costs one member onHttpCurlGlobalInitializer, which is a concrete class in an installed header, so it moves that layout.The larger one gives the client a way to refuse construction, which is a wider conversation about what an exporter does when its HTTP client cannot exist.
Why I have not sent a patch
I cannot make
curl_global_initfail in the test binary, and I would rather say so than ship a branch nothing runs.The suite installs its allocator callbacks with curl_global_init_mem from
SetUpTestSuite, and that call is the global initialisation: it "works exactly as curl_global_init with one addition". It has to run there, because installing the callbacks after libcurl is initialised is a silent no-op. So by the time any client exists libcurl is already initialised, and thecurl_global_initinside the singleton is a reference count bump that returnsCURLE_OKwhatever the allocator does.Reaching the failure needs a seam that the client calls instead of
curl_global_initdirectly. That is worth discussing on its own terms, since the same seam would answer the other injection questions in this area, but it is production surface added for testability and not something to slip into a bug fix.Happy to send either shape once you say which, and to write the seam if that is the direction.
Found while working through #4404.