-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core-clp: Add class to encapsulate libcurl
's global resource management.
#461
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, @kirkrodrigues you can review the rest
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the PR title, how about:
core-clp: Add class to encapsulate libcurl
's global resource management.
libcurl
global resource management.libcurl
's global resource management.
…ment. (y-scope#461) Co-authored-by: kirkrodrigues <2454684+kirkrodrigues@users.noreply.github.com>
Description
Before this PR, we reply on static methods
clp::NetworkReader::init/deinit
to initializelibcurl
global resources. However, there are downsides of the current implementation:libcurl
; there should be a dedicated approach, instead of being a part ofclp::NetworkReader
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 beforeclp::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
clp::NetworkReader
works as expected after switching to this new class