weidai11 / cryptopp

free C++ class library of cryptographic schemes
https://cryptopp.com
Other
4.67k stars 1.47k forks source link

Incorrect use of the release fence in Singleton::Ref #1208

Closed Komfr closed 1 year ago

Komfr commented 1 year ago

Both the last released version 8.7 and the current master branch (2038ab1) have an incorrect order of the release fence and pointer store operations when creating the object.

The current current code is

T *newObject = m_objectFactory();
s_pObject.store(newObject, std::memory_order_relaxed);
std::atomic_thread_fence(std::memory_order_release);

however the correct sequence, which is also described in the article linked from the function documentation, should be

T *newObject = m_objectFactory();
std::atomic_thread_fence(std::memory_order_release);
s_pObject.store(newObject, std::memory_order_relaxed);

Otherwise there is no guarantee that the object content will be already fully stored in the memory when another thread might see the pointer and attempt to access it.

noloader commented 1 year ago

Thanks @Komfr,

This should be cleared at Commit a23f78d116c9.

We just released Crypto++ 8.8, so this change should get good testing coverage before the next release.