clp icon indicating copy to clipboard operation
clp copied to clipboard

core-clp: Add support for appropriate `libcurl` global resource management.

Open LinZhihao-723 opened this issue 8 months ago • 0 comments

Description

Before this PR, we reply on static methods clp::NetworkReader::init/deinit to initialize libcurl global resources. However, there are downsides of the current implementation:

  1. Curl resource management should not be specific to a particular application of libcurl; there should be a dedicated approach, instead of being a part of clp::NetworkReader
  2. The current way of doing init/deinit is just a wrapper to libcurl's C style resource management. It has nothing guaranteed about the object life cycle. This is the root cause of some of the workflow failure, such as this one: https://github.com/y-scope/clp/actions/runs/9613911655/job/26517641592, where the global deinit happens before clp::NetworkReader instance gets out of its life cycle.

This PR introduces a specialized class clp::CurlGlobalInstance for managing curl resources using the RAII pattern. Similar to mongodb, users must create an instance of a specific class to initialize the resources. Resource deallocation occurs automatically when the object is destroyed. To manage multiple instances, a thread-safe static reference count is used to track all active instances. This prevents double-initialization and ensures that global resources are not deallocated while they are still needed by other instances.

Validation performed

  1. Ensure the code gets built properly without linter errors
  2. Ensure clp::NetworkReader works as expected after switching to this new class

LinZhihao-723 avatar Jun 25 '24 01:06 LinZhihao-723