Skip to content
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

Use std::call_once instead of double-checked locking in singletons #1042

Merged
merged 4 commits into from
Apr 18, 2024
Merged

Use std::call_once instead of double-checked locking in singletons #1042

merged 4 commits into from
Apr 18, 2024

Conversation

bllanos
Copy link
Contributor

@bllanos bllanos commented Apr 10, 2024

Fixes #1041

  • I used std::call_once to initialize and destroy singleton classes.
  • I added basic tests for singleton class initialization and shutdown functions.

Notes

I added an assertion to the ExitInstance() function to help users detect if they call it multiple times in the same program by accident. Correct me if I am mistaken, but I assumed singleton classes do not need to be initialized and shutdown more than once. The previous version of the code allowed for multiple cycles of initialization and shutdown.

Other

The CONTRIBUTING.md file mentions format-check.sh that I don't see in the repository, and it also links to a different location for the repository, https://github.com/whoshuu/cpr.

@COM8 COM8 self-assigned this Apr 10, 2024
Copy link
Member

@COM8 COM8 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bllanos thank you very much for your PR! Looks good. Only a small assert code style change request ;)

include/cpr/singleton.h Outdated Show resolved Hide resolved
@COM8
Copy link
Member

COM8 commented Apr 10, 2024

On your note: No, I'm not aware of any case where we use the singleton being initialized and destroyed multiple times. (Isn't this a bit against what a singleton is - in theory?).

Arg, yes the contribution docs. We've started rewriting them and extending them over here but yeah. There is room for improvement.

@COM8 COM8 merged commit d47fd88 into libcpr:master Apr 18, 2024
46 of 53 checks passed
@bllanos bllanos deleted the singleton_fix_locking branch April 18, 2024 16:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible race condition in singleton class GetInstance() function
2 participants