Commit c1d0b986 authored by Rob Sherwood's avatar Rob Sherwood Committed by Facebook GitHub Bot

Really,really make ConstructorCallback -Wglobal-constructor safe

Summary:
Same plan as D28395301 (https://github.com/facebook/folly/commit/99f856ae2009a80b157b5121e44b1f70f61bd7c9), but with a better implementation and tests
in the diff to make sure it actually works.

Reviewed By: bschlinker

Differential Revision: D28421062

fbshipit-source-id: d9b052b38c7b938a78bbaccd4df8d276af2b6d6c
parent e4cfd54f
...@@ -84,6 +84,8 @@ class ConstructorCallback { ...@@ -84,6 +84,8 @@ class ConstructorCallback {
using NewConstructorCallback = folly::Function<void(T*)>; using NewConstructorCallback = folly::Function<void(T*)>;
using This = ConstructorCallback<T, MaxCallbacks>; using This = ConstructorCallback<T, MaxCallbacks>;
using CallbackArray =
std::array<typename This::NewConstructorCallback, MaxCallbacks>;
explicit ConstructorCallback(T* t) { explicit ConstructorCallback(T* t) {
// This code depends on the C++ standard where values that are // This code depends on the C++ standard where values that are
...@@ -127,12 +129,20 @@ class ConstructorCallback { ...@@ -127,12 +129,20 @@ class ConstructorCallback {
* @throw std::length_error() if this callback would exceed our max * @throw std::length_error() if this callback would exceed our max
*/ */
static void addNewConstructorCallback(NewConstructorCallback cb) { static void addNewConstructorCallback(NewConstructorCallback cb) {
// Ensure that a single callback is added at a time
std::lock_guard<SharedMutex> g(This::getMutex()); std::lock_guard<SharedMutex> g(This::getMutex());
auto idx = nConstructorCallbacks_.load(std::memory_order_acquire); auto idx = nConstructorCallbacks_.load(std::memory_order_acquire);
if (callbacks_ == nullptr) { if (callbacks_ == nullptr) {
// initialize the array if unallocated // Get a pointer to the callback array if we've not already done
callbacks_ = std::make_unique< // so.
std::array<This::NewConstructorCallback, MaxCallbacks>>(); //
// NOTE: this only happens once per class (e.g., when the first
// callback is registered) and we're already locked on getMutex()
// so this should be very cheap.
//
// NOTE: store a raw/dumb pointer to avoid a race condition
// and -Wglobal-error issues on shutdown
callbacks_ = This::getCallbackArray();
} }
if (idx >= (*callbacks_).size()) { if (idx >= (*callbacks_).size()) {
throw std::length_error( throw std::length_error(
...@@ -146,9 +156,10 @@ class ConstructorCallback { ...@@ -146,9 +156,10 @@ class ConstructorCallback {
private: private:
// allocate an array internal to function to avoid init() races // allocate an array internal to function to avoid init() races
static std::unique_ptr<std::array<NewConstructorCallback, MaxCallbacks>>
callbacks_;
static folly::SharedMutex& getMutex(); static folly::SharedMutex& getMutex();
static This::CallbackArray* getCallbackArray();
static This::CallbackArray* callbacks_;
static std::atomic<int> nConstructorCallbacks_; static std::atomic<int> nConstructorCallbacks_;
}; };
...@@ -163,9 +174,19 @@ folly::SharedMutex& ConstructorCallback<T, MaxCallbacks>::getMutex() { ...@@ -163,9 +174,19 @@ folly::SharedMutex& ConstructorCallback<T, MaxCallbacks>::getMutex() {
} }
template <class T, std::size_t MaxCallbacks> template <class T, std::size_t MaxCallbacks>
std::unique_ptr<std::array< typename ConstructorCallback<T, MaxCallbacks>::CallbackArray*
typename ConstructorCallback<T, MaxCallbacks>::NewConstructorCallback, ConstructorCallback<T, MaxCallbacks>::callbacks_;
MaxCallbacks>>
ConstructorCallback<T, MaxCallbacks>::callbacks_{nullptr}; template <class T, std::size_t MaxCallbacks>
typename ConstructorCallback<T, MaxCallbacks>::CallbackArray*
ConstructorCallback<T, MaxCallbacks>::getCallbackArray() {
// NOTE the compiler implicitly generates a lock here
// to avoid double (static) allocation of this object
// we avoid paying the cost of the lock by only calling this
// function once per class (while we're already locked on getMutex())
// and recording the pointer info
static ConstructorCallback<T, MaxCallbacks>::CallbackArray a{};
return &a;
}
} // namespace folly } // namespace folly
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment