Commit f321edf8 authored by Andrii Grynenko's avatar Andrii Grynenko

Don't re-create singletons

Summary:
This forbids singleton creation/re-creation after destroyInstances() was called.
Only after reset() is called singletons can be created again.

registrationComplete() behavior is also slightly change. We disallow singleton registration after registrationComplete() is called even in Relaxed mode. Strict mode now only controlls whether singletons can be constructed before registation is complete.

Test Plan: unit test

Reviewed By: chip@fb.com

Subscribers: hphp-diffs@, ps, trunkagent, njormrod, mshneer, lins

FB internal diff: D1605136
parent 4928fe78
...@@ -23,6 +23,15 @@ namespace folly { ...@@ -23,6 +23,15 @@ namespace folly {
SingletonVault::~SingletonVault() { destroyInstances(); } SingletonVault::~SingletonVault() { destroyInstances(); }
void SingletonVault::destroyInstances() { void SingletonVault::destroyInstances() {
RWSpinLock::WriteHolder state_wh(&stateMutex_);
if (state_ == SingletonVaultState::Quiescing) {
return;
}
state_ = SingletonVaultState::Quiescing;
RWSpinLock::ReadHolder state_rh(std::move(state_wh));
{ {
RWSpinLock::ReadHolder rh(&mutex_); RWSpinLock::ReadHolder rh(&mutex_);
...@@ -54,6 +63,14 @@ void SingletonVault::destroyInstances() { ...@@ -54,6 +63,14 @@ void SingletonVault::destroyInstances() {
} }
} }
void SingletonVault::reenableInstances() {
RWSpinLock::WriteHolder state_wh(&stateMutex_);
stateCheck(SingletonVaultState::Quiescing);
state_ = SingletonVaultState::Running;
}
SingletonVault* SingletonVault::singleton() { SingletonVault* SingletonVault::singleton() {
static SingletonVault* vault = new SingletonVault(); static SingletonVault* vault = new SingletonVault();
......
...@@ -74,6 +74,9 @@ ...@@ -74,6 +74,9 @@
// TeardownFunc for each singleton, in the reverse order they were // TeardownFunc for each singleton, in the reverse order they were
// created. It is your responsibility to ensure your singletons can // created. It is your responsibility to ensure your singletons can
// handle cases where the singletons they depend on go away, however. // handle cases where the singletons they depend on go away, however.
// Singletons won't be recreated after destroyInstances call. If you
// want to re-enable singleton creation (say after fork was called) you
// should call reenableInstances.
#pragma once #pragma once
#include <folly/Exception.h> #include <folly/Exception.h>
...@@ -179,9 +182,16 @@ class SingletonVault { ...@@ -179,9 +182,16 @@ class SingletonVault {
void registerSingleton(detail::TypeDescriptor type, void registerSingleton(detail::TypeDescriptor type,
CreateFunc create, CreateFunc create,
TeardownFunc teardown) { TeardownFunc teardown) {
RWSpinLock::ReadHolder rh(&stateMutex_);
stateCheck(SingletonVaultState::Running);
if (UNLIKELY(registrationComplete_)) {
throw std::logic_error(
"Registering singleton after registrationComplete().");
}
RWSpinLock::WriteHolder wh(&mutex_); RWSpinLock::WriteHolder wh(&mutex_);
stateCheck(SingletonVaultState::Registering);
CHECK_THROW(singletons_.find(type) == singletons_.end(), std::logic_error); CHECK_THROW(singletons_.find(type) == singletons_.end(), std::logic_error);
auto& entry = singletons_[type]; auto& entry = singletons_[type];
entry.reset(new SingletonEntry); entry.reset(new SingletonEntry);
...@@ -198,19 +208,36 @@ class SingletonVault { ...@@ -198,19 +208,36 @@ class SingletonVault {
// Mark registration is complete; no more singletons can be // Mark registration is complete; no more singletons can be
// registered at this point. // registered at this point.
void registrationComplete() { void registrationComplete() {
RWSpinLock::WriteHolder wh(&mutex_); RWSpinLock::WriteHolder wh(&stateMutex_);
stateCheck(SingletonVaultState::Running);
stateCheck(SingletonVaultState::Registering); if (type_ == Type::Strict) {
state_ = SingletonVaultState::Running; for (const auto& id_singleton_entry: singletons_) {
const auto& singleton_entry = *id_singleton_entry.second;
if (singleton_entry.state != SingletonEntryState::Dead) {
throw std::runtime_error(
"Singleton created before registration was complete.");
}
}
}
registrationComplete_ = true;
} }
// Destroy all singletons; when complete, the vault can create // Destroy all singletons; when complete, the vault can't create
// singletons once again, or remain dormant. // singletons once again until reenableInstances() is called.
void destroyInstances(); void destroyInstances();
// Enable re-creating singletons after destroyInstances() was called.
void reenableInstances();
// Retrieve a singleton from the vault, creating it if necessary. // Retrieve a singleton from the vault, creating it if necessary.
std::shared_ptr<void> get_shared(detail::TypeDescriptor type) { std::shared_ptr<void> get_shared(detail::TypeDescriptor type) {
auto entry = get_entry_create(type); auto entry = get_entry_create(type);
if (UNLIKELY(!entry)) {
return std::shared_ptr<void>();
}
return entry->instance; return entry->instance;
} }
...@@ -220,6 +247,10 @@ class SingletonVault { ...@@ -220,6 +247,10 @@ class SingletonVault {
// the weak_ptr interface for true safety. // the weak_ptr interface for true safety.
void* get_ptr(detail::TypeDescriptor type) { void* get_ptr(detail::TypeDescriptor type) {
auto entry = get_entry_create(type); auto entry = get_entry_create(type);
if (UNLIKELY(!entry)) {
throw std::runtime_error(
"Raw pointer to a singleton requested after its destruction.");
}
return entry->instance_ptr; return entry->instance_ptr;
} }
...@@ -251,8 +282,8 @@ class SingletonVault { ...@@ -251,8 +282,8 @@ class SingletonVault {
private: private:
// The two stages of life for a vault, as mentioned in the class comment. // The two stages of life for a vault, as mentioned in the class comment.
enum class SingletonVaultState { enum class SingletonVaultState {
Registering,
Running, Running,
Quiescing,
}; };
// Each singleton in the vault can be in three states: dead // Each singleton in the vault can be in three states: dead
...@@ -266,7 +297,7 @@ class SingletonVault { ...@@ -266,7 +297,7 @@ class SingletonVault {
void stateCheck(SingletonVaultState expected, void stateCheck(SingletonVaultState expected,
const char* msg="Unexpected singleton state change") { const char* msg="Unexpected singleton state change") {
if (type_ == Type::Strict && expected != state_) { if (expected != state_) {
throw std::logic_error(msg); throw std::logic_error(msg);
} }
} }
...@@ -301,13 +332,6 @@ class SingletonVault { ...@@ -301,13 +332,6 @@ class SingletonVault {
SingletonEntry* get_entry(detail::TypeDescriptor type) { SingletonEntry* get_entry(detail::TypeDescriptor type) {
RWSpinLock::ReadHolder rh(&mutex_); RWSpinLock::ReadHolder rh(&mutex_);
// mutex must be held when calling this function
stateCheck(
SingletonVaultState::Running,
"Attempt to load a singleton before "
"SingletonVault::registrationComplete was called (hint: you probably "
"didn't call initFacebook)");
auto it = singletons_.find(type); auto it = singletons_.find(type);
if (it == singletons_.end()) { if (it == singletons_.end()) {
throw std::out_of_range(std::string("non-existent singleton: ") + throw std::out_of_range(std::string("non-existent singleton: ") +
...@@ -339,6 +363,11 @@ class SingletonVault { ...@@ -339,6 +363,11 @@ class SingletonVault {
} }
if (entry->instance == nullptr) { if (entry->instance == nullptr) {
RWSpinLock::ReadHolder rh(&stateMutex_);
if (state_ == SingletonVaultState::Quiescing) {
return nullptr;
}
CHECK(entry->state == SingletonEntryState::Dead); CHECK(entry->state == SingletonEntryState::Dead);
entry->state = SingletonEntryState::BeingBorn; entry->state = SingletonEntryState::BeingBorn;
entry->creating_thread = std::this_thread::get_id(); entry->creating_thread = std::this_thread::get_id();
...@@ -370,8 +399,10 @@ class SingletonVault { ...@@ -370,8 +399,10 @@ class SingletonVault {
SingletonEntryPtr, SingletonEntryPtr,
detail::TypeDescriptorHasher> singletons_; detail::TypeDescriptorHasher> singletons_;
std::vector<detail::TypeDescriptor> creation_order_; std::vector<detail::TypeDescriptor> creation_order_;
SingletonVaultState state_ = SingletonVaultState::Registering; SingletonVaultState state_{SingletonVaultState::Running};
Type type_ = Type::Relaxed; bool registrationComplete_{false};
folly::RWSpinLock stateMutex_;
Type type_{Type::Relaxed};
}; };
// This is the wrapper class that most users actually interact with. // This is the wrapper class that most users actually interact with.
......
...@@ -188,9 +188,6 @@ TEST(Singleton, NaughtyUsage) { ...@@ -188,9 +188,6 @@ TEST(Singleton, NaughtyUsage) {
}(), }(),
std::logic_error); std::logic_error);
// Default vault is non-strict; this should work.
Singleton<Watchdog> global_watchdog_singleton;
SingletonVault vault_2(SingletonVault::Type::Strict); SingletonVault vault_2(SingletonVault::Type::Strict);
EXPECT_THROW(Singleton<Watchdog>::get(&vault_2), std::logic_error); EXPECT_THROW(Singleton<Watchdog>::get(&vault_2), std::logic_error);
Singleton<Watchdog> watchdog_singleton(nullptr, nullptr, &vault_2); Singleton<Watchdog> watchdog_singleton(nullptr, nullptr, &vault_2);
...@@ -263,6 +260,12 @@ TEST(Singleton, SharedPtrUsage) { ...@@ -263,6 +260,12 @@ TEST(Singleton, SharedPtrUsage) {
locked_s1 = weak_s1.lock(); locked_s1 = weak_s1.lock();
EXPECT_TRUE(weak_s1.expired()); EXPECT_TRUE(weak_s1.expired());
auto empty_s1 = Singleton<Watchdog>::get_weak(&vault);
EXPECT_FALSE(empty_s1.lock());
vault.reenableInstances();
// Singleton should be re-created only after reenableInstances() was called.
Watchdog* new_s1 = Singleton<Watchdog>::get(&vault); Watchdog* new_s1 = Singleton<Watchdog>::get(&vault);
EXPECT_NE(new_s1->serial_number, old_serial); EXPECT_NE(new_s1->serial_number, old_serial);
} }
......
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