search

libcpr / cpr

C++ Requests: Curl for People, a spiritual port of Python Requests.
https://docs.libcpr.org/
Other
6.29k stars 903 forks source link

Possible race condition in singleton class GetInstance() function #1041

Closed bllanos closed 2 months ago

bllanos commented 2 months ago

Description

I looked at the implementation of singleton classes and noticed that there is a check for a null instance pointer before the mutex is locked for initializing the singleton instance. I think this might produce a race condition as it differs from the patterns shown at https://en.wikipedia.org/wiki/Double-checked_locking#Usage_in_C++11 for lazy initialization of singleton classes. The paper describing ThreadSanitizer mentions double-checked locking as a common source of race conditions because of possible instruction reordering or stale hardware cache memory.

I was troubleshooting a segmentation fault that seemed to be resolved when I used a local cpr::ThreadPool object instead of the singleton cpr::GlobalThreadPool, but I now think the memory management issues were unrelated to this library.

Example/How to Reproduce

This is a hypothetical issue and I have not needed to write code that could trigger it.

Possible Fix

No response

Where did you get it from?

GitHub (branch e.g. master)

Additional Context/Your Environment

COM8 commented 2 months ago

Thanks for reporting @bllanos! I agree, double checked locking is not how we should do stuff in a modern c++ environment any more. Would you like to provide a PR that addresses this? Perhaps utilizing std::call_once?