embeddedmz / ftpclient-cpp

C++ client for making FTP requests
MIT License
205 stars 65 forks source link

caveat : global initialization of libcurl #4

Closed embeddedmz closed 3 years ago

embeddedmz commented 5 years ago

libcurl global initialization is currently handled in the constructor with a static mutex and a counter of CFTPClient objects :

   s_mtxCurlSession.lock();
   if (s_iCurlSession++ == 0)
   {
      curl_global_init(CURL_GLOBAL_ALL);
   }
   s_mtxCurlSession.unlock();

ditto for the global cleanup in destructor :

   s_mtxCurlSession.lock();
   if (--s_iCurlSession <= 0)
   {
      curl_global_cleanup();
   }
   s_mtxCurlSession.unlock();

If in your application, there's only ftpclient-cpp using libcurl, there's no issues.

However, if you use it with another 3rd party library (e.g. httpclient-cpp) that initializes curl in a thread different from any other thread where CFTPClient objects live, there will be undefined behavior (crashs, bugs, dragons, clowns etc...). As stated here : https://curl.haxx.se/libcurl/c/curl_global_init.html curl_global_init isn't thread safe.

I think it's better to remove this stupid "hack" and explicitly call curl_global_init(CURL_GLOBAL_ALL); at the beginning of your program and call curl_global_cleanup(); at the end of it. Adding a static method is a good way to achieve that.

Finkman commented 4 years ago

The issue seems to come from libraries that are used by libcurl which puts libcurl in the same topic as we are discussing.

Even putting curl_global_init(CURL_GLOBAL_ALL); in a static method like init_globals() might be wrong, since it prevents the caller to set the different options of CURL_GLOBAL_XX which are required in some scenarios.

IMHO, the best option would be a c++ wrapper for curl which does that work and provides those options in it's own scope. All c-like functions could be made in oop-style and your library would be a consumer of that curl-cpp library, so no worries.

A compromise to that, since there is actually no wrapper, would be to make a c++ lib handle like proposed in #7. It could be used in the application and should be needed in the ftpclient ctor to enforce the creation of the handle.

int main(){
  auto client FTPClient{CurlHandle{}, [](auto const& logMsg){}};
}

Beside the handle idea, I'm going to investigate into the idea of a curl-cpp library, maybe it could be useful anyway.

embeddedmz commented 3 years ago

Finkman solution (a static constructor similar to the one that can be made in C# or Java) fixes the issue. I will use the same pattern in the httpclient-cpp and mailclient-cpp projects.