core-clp: Add class to encapsulate libcurl's global resource management.

components/core/CMakeLists.txt

@@ -282,6 +282,8 @@ set(SOURCE_FILES_unitTest
         src/clp/CurlDownloadHandler.cpp
         src/clp/CurlDownloadHandler.hpp
         src/clp/CurlEasyHandle.hpp
+        src/clp/CurlGlobalInstance.cpp
+        src/clp/CurlGlobalInstance.hpp
         src/clp/CurlOperationFailed.hpp
         src/clp/CurlStringList.hpp
         src/clp/database_utils.cpp

components/core/src/clp/CurlGlobalInstance.cpp

@@ -0,0 +1,43 @@
+#include "CurlGlobalInstance.hpp"
+
+#include <mutex>
+
+#include <curl/curl.h>
+
+#include "CurlOperationFailed.hpp"
+#include "ErrorCode.hpp"
+
+namespace clp {
+CurlGlobalInstance::CurlGlobalInstance() {
+    std::lock_guard<std::mutex> const global_lock{m_global_mutex};
+    if (0 == m_num_living_instances) {
+        if (auto const err{curl_global_init(CURL_GLOBAL_ALL)}; CURLE_OK != err) {
+            throw CurlOperationFailed(
+                    ErrorCode_Failure,
+                    __FILE__,
+                    __LINE__,
+                    err,
+                    "`curl_global_init` failed."
+            );
+        }
+    }
+    ++m_num_living_instances;
+}
+
+CurlGlobalInstance::~CurlGlobalInstance() {
+    std::lock_guard<std::mutex> const global_lock{m_global_mutex};
+    --m_num_living_instances;
+    if (0 == m_num_living_instances) {
+#if defined(__APPLE__)
+        // NOTE: On macOS, calling `curl_global_init` after `curl_global_cleanup` will fail with
+        // CURLE_SSL_CONNECT_ERROR. Thus, for now, we skip `deinit` on macOS. Related issues:
+        // - https://github.com/curl/curl/issues/12525
+        // - https://github.com/curl/curl/issues/13805
+        // TODO: Remove this conditional logic when the issues are resolved.
+        return;
+#else
+        curl_global_cleanup();
+#endif
+    }
+}
+}  // namespace clp

components/core/src/clp/CurlGlobalInstance.hpp

@@ -0,0 +1,34 @@
+#ifndef CLP_CURLGLOBALINSTANCE_HPP
+#define CLP_CURLGLOBALINSTANCE_HPP
+
+#include <cstddef>
+#include <mutex>
+
+namespace clp {
+/**
+ * Class to wrap `libcurl`'s global initialization/de-initialization calls using RAII. Before using
+ * any `libcurl` functionalities, an instance of this function must be created to ensure underlying
+ * `libcurl` resources have been initialized. This class maintains a static reference count to all
+ * the living instances. De-initialization happens when the reference count reaches 0.
+ */
+class CurlGlobalInstance {
+public:
+    // Constructors
+    CurlGlobalInstance();
+
+    // Disable copy/move constructors/assignment operators
+    CurlGlobalInstance(CurlGlobalInstance const&) = delete;
+    CurlGlobalInstance(CurlGlobalInstance&&) = delete;
+    auto operator=(CurlGlobalInstance const&) -> CurlGlobalInstance& = delete;
+    auto operator=(CurlGlobalInstance&&) -> CurlGlobalInstance& = delete;
+
+    // Destructor
+    ~CurlGlobalInstance();
+
+private:
+    static inline std::mutex m_global_mutex;
+    static inline size_t m_num_living_instances;
+};
+}  // namespace clp
+
+#endif  // CLP_CURLGLOBALINSTANCE_HPP

components/core/src/clp/NetworkReader.cpp

@@ -110,33 +110,6 @@ curl_write_callback(char* ptr, size_t size, size_t nmemb, void* reader_ptr) -> s
 }
 }  // namespace
 
-bool NetworkReader::m_static_init_complete{false};
-
-auto NetworkReader::init() -> ErrorCode {
-    if (m_static_init_complete) {
-        return ErrorCode_Success;
-    }
-    if (0 != curl_global_init(CURL_GLOBAL_ALL)) {
-        return ErrorCode_Failure;
-    }
-    m_static_init_complete = true;
-    return ErrorCode_Success;
-}
-
-auto NetworkReader::deinit() -> void {
-#if defined(__APPLE__)
-    // NOTE: On macOS, calling `curl_global_init` after `curl_global_cleanup` will fail with
-    // CURLE_SSL_CONNECT_ERROR. Thus, for now, we skip `deinit` on macOS. Related issues:
-    // - https://github.com/curl/curl/issues/12525
-    // - https://github.com/curl/curl/issues/13805
-    // TODO: Remove this conditional logic when the issues are resolved.
-    return;
-#else
-    curl_global_cleanup();
-    m_static_init_complete = false;
-#endif
-}
-
 NetworkReader::NetworkReader(
         std::string_view src_url,
         size_t offset,
@@ -157,9 +130,6 @@ NetworkReader::NetworkReader(
         // NOLINTNEXTLINE(cppcoreguidelines-avoid-c-arrays)
         m_buffer_pool.emplace_back(std::make_unique<char[]>(m_buffer_size));
     }
-    if (false == m_static_init_complete) {
-        throw OperationFailed(ErrorCode_NotReady, __FILE__, __LINE__);
-    }
     m_downloader_thread = std::make_unique<DownloaderThread>(*this, offset, disable_caching);
     m_downloader_thread->start();
 }

components/core/src/clp/NetworkReader.hpp

@@ -69,24 +69,10 @@ class NetworkReader : public ReaderInterface {
     static constexpr size_t cMinBufferPoolSize{2};
     static constexpr size_t cMinBufferSize{512};
 
-    /**
-     * Initializes static resources for this class. This must be called before using the class.
-     * @return ErrorCode_Success on success.
-     * @return ErrorCode_Failure if libcurl initialization failed.
-     */
-    [[nodiscard]] static auto init() -> ErrorCode;
-
-    /**
-     * De-initializes any static resources.
-     */
-    static auto deinit() -> void;
-
     /**
      * Constructs a reader to stream data from the given URL, starting at the given offset.
-     * TODO: the current implementation doesn't handle the case when the given offset is out of
-     * range. The file_pos will be set to an invalid state if this happens, which can be
-     * problematic if the other part of the program depends on this position. It can be fixed by
-     * capturing the error code 416 in the response header.
+     * Note: This class depends on `libcurl`. An instance of `clp::CurlGlobalInstance` must remain
+     * alive for the entire lifespan of any instance of this class to maintain proper functionality.
      * @param src_url
      * @param offset Index of the byte at which to start the download
      * @param disable_caching Whether to disable the caching.
@@ -246,8 +232,6 @@ class NetworkReader : public ReaderInterface {
         bool m_disable_caching{false};
     };
 
-    static bool m_static_init_complete;
-
     /**
      * Submits a request to abort the ongoing curl download session.
      */

components/core/tests/test-NetworkReader.cpp

@@ -12,6 +12,7 @@
 #include <curl/curl.h>
 
 #include "../src/clp/CurlDownloadHandler.hpp"
+#include "../src/clp/CurlGlobalInstance.hpp"
 #include "../src/clp/ErrorCode.hpp"
 #include "../src/clp/FileReader.hpp"
 #include "../src/clp/NetworkReader.hpp"
@@ -66,23 +67,23 @@ auto get_content(clp::ReaderInterface& reader, size_t read_buf_size) -> std::vec
 }  // namespace
 
 TEST_CASE("network_reader_basic", "[NetworkReader]") {
+    clp::CurlGlobalInstance const curl_global_instance;
     clp::FileReader ref_reader;
     ref_reader.open(get_test_input_local_path());
     auto const expected{get_content(ref_reader)};
     ref_reader.close();
 
-    REQUIRE((clp::ErrorCode_Success == clp::NetworkReader::init()));
     clp::NetworkReader reader{get_test_input_remote_url()};
     auto const actual{get_content(reader)};
     auto const ret_code{reader.get_curl_ret_code()};
     REQUIRE(ret_code.has_value());
     // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
     REQUIRE((CURLE_OK == ret_code.value()));
     REQUIRE((actual == expected));
-    clp::NetworkReader::deinit();
 }
 
 TEST_CASE("network_reader_with_offset_and_seek", "[NetworkReader]") {
+    clp::CurlGlobalInstance const curl_global_instance;
     constexpr size_t cOffset{319};
     clp::FileReader ref_reader;
     ref_reader.open(get_test_input_local_path());
@@ -91,8 +92,6 @@ TEST_CASE("network_reader_with_offset_and_seek", "[NetworkReader]") {
     auto const ref_end_pos{ref_reader.get_pos()};
     ref_reader.close();
 
-    REQUIRE((clp::ErrorCode_Success == clp::NetworkReader::init()));
-
     // Read from an offset onwards by starting the download from that offset.
     {
         clp::NetworkReader reader{get_test_input_remote_url(), cOffset};
@@ -117,12 +116,10 @@ TEST_CASE("network_reader_with_offset_and_seek", "[NetworkReader]") {
         REQUIRE((reader.get_pos() == ref_end_pos));
         REQUIRE((actual == expected));
     }
-
-    clp::NetworkReader::deinit();
 }
 
 TEST_CASE("network_reader_destruct", "[NetworkReader]") {
-    REQUIRE((clp::ErrorCode_Success == clp::NetworkReader::init()));
+    clp::CurlGlobalInstance const curl_global_instance;
 
     // We sleep to fill out all the buffers, and then we delete the reader. The destructor will try
     // to abort the underlying download and then destroy the instance. So should ensure destructor
@@ -147,12 +144,10 @@ TEST_CASE("network_reader_destruct", "[NetworkReader]") {
         no_exception = false;
     }
     REQUIRE(no_exception);
-
-    clp::NetworkReader::deinit();
 }
 
 TEST_CASE("network_reader_illegal_offset", "[NetworkReader]") {
-    REQUIRE((clp::ErrorCode_Success == clp::NetworkReader::init()));
+    clp::CurlGlobalInstance const curl_global_instance;
 
     // Try to read from an out-of-bound offset.
     constexpr size_t cIllegalOffset{UINT32_MAX};
@@ -166,6 +161,4 @@ TEST_CASE("network_reader_illegal_offset", "[NetworkReader]") {
             break;
         }
     }
-
-    clp::NetworkReader::deinit();
 }