Commit 736c0e46 authored by Chip Turner's avatar Chip Turner Committed by Dave Watson

Add non-strict mode to folly::Singleton

Summary:
It is difficult to guarantee every binary can call
registrationComplete; for now, support a mode where we don't enforce the
strict registration lifecycle.  In the fullness of time, we can make
strictness the default.

Test Plan: runtests

Reviewed By: lins@fb.com

Subscribers: lins, anca, njormrod

FB internal diff: D1551688
parent 6b82bb37
...@@ -163,7 +163,9 @@ class TypeDescriptorHasher { ...@@ -163,7 +163,9 @@ class TypeDescriptorHasher {
class SingletonVault { class SingletonVault {
public: public:
SingletonVault() {}; enum class Type { Strict, Relaxed };
explicit SingletonVault(Type type = Type::Relaxed) : type_(type) {}
~SingletonVault(); ~SingletonVault();
typedef std::function<void(void*)> TeardownFunc; typedef std::function<void(void*)> TeardownFunc;
...@@ -176,7 +178,7 @@ class SingletonVault { ...@@ -176,7 +178,7 @@ class SingletonVault {
TeardownFunc teardown) { TeardownFunc teardown) {
std::lock_guard<std::mutex> guard(mutex_); std::lock_guard<std::mutex> guard(mutex_);
CHECK_THROW(state_ == SingletonVaultState::Registering, std::logic_error); 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];
if (!entry) { if (!entry) {
...@@ -196,7 +198,7 @@ class SingletonVault { ...@@ -196,7 +198,7 @@ class SingletonVault {
// registered at this point. // registered at this point.
void registrationComplete() { void registrationComplete() {
std::lock_guard<std::mutex> guard(mutex_); std::lock_guard<std::mutex> guard(mutex_);
CHECK_THROW(state_ == SingletonVaultState::Registering, std::logic_error); stateCheck(SingletonVaultState::Registering);
state_ = SingletonVaultState::Running; state_ = SingletonVaultState::Running;
} }
...@@ -259,6 +261,13 @@ class SingletonVault { ...@@ -259,6 +261,13 @@ class SingletonVault {
Living, Living,
}; };
void stateCheck(SingletonVaultState expected,
const char* msg="Unexpected singleton state change") {
if (type_ == Type::Strict && expected != state_) {
throw std::logic_error(msg);
}
}
// An actual instance of a singleton, tracking the instance itself, // An actual instance of a singleton, tracking the instance itself,
// its state as described above, and the create and teardown // its state as described above, and the create and teardown
// functions. // functions.
...@@ -292,12 +301,11 @@ class SingletonVault { ...@@ -292,12 +301,11 @@ class SingletonVault {
SingletonEntry* get_entry(detail::TypeDescriptor type, SingletonEntry* get_entry(detail::TypeDescriptor type,
std::unique_lock<std::mutex>* lock) { std::unique_lock<std::mutex>* lock) {
// mutex must be held when calling this function // mutex must be held when calling this function
if (state_ != SingletonVaultState::Running) { stateCheck(
throw std::logic_error( SingletonVaultState::Running,
"Attempt to load a singleton before " "Attempt to load a singleton before "
"SingletonVault::registrationComplete was called (hint: you probably " "SingletonVault::registrationComplete was called (hint: you probably "
"didn't call initFacebook)"); "didn't call initFacebook)");
}
auto it = singletons_.find(type); auto it = singletons_.find(type);
if (it == singletons_.end()) { if (it == singletons_.end()) {
...@@ -357,6 +365,7 @@ class SingletonVault { ...@@ -357,6 +365,7 @@ class SingletonVault {
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::Registering;
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.
......
...@@ -174,7 +174,7 @@ TEST(Singleton, NamedUsage) { ...@@ -174,7 +174,7 @@ TEST(Singleton, NamedUsage) {
// Some pathological cases such as getting unregistered singletons, // Some pathological cases such as getting unregistered singletons,
// double registration, etc. // double registration, etc.
TEST(Singleton, NaughtyUsage) { TEST(Singleton, NaughtyUsage) {
SingletonVault vault; SingletonVault vault(SingletonVault::Type::Strict);
vault.registrationComplete(); vault.registrationComplete();
// Unregistered. // Unregistered.
...@@ -188,10 +188,10 @@ TEST(Singleton, NaughtyUsage) { ...@@ -188,10 +188,10 @@ TEST(Singleton, NaughtyUsage) {
}(), }(),
std::logic_error); std::logic_error);
EXPECT_THROW([]() { Singleton<Watchdog> watchdog_singleton; }(), // Default vault is non-strict; this should work.
std::logic_error); Singleton<Watchdog> global_watchdog_singleton;
SingletonVault vault_2; 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);
// double registration // double registration
......
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