Commit fa0c6eae authored by Andrii Grynenko's avatar Andrii Grynenko Committed by dcsommer

Don't throw in Singleton::get() if singleton is alive

Summary:
This is a fix for situations where you know that something is keeping singleton alive (by getting a weak_ptr and locking it), and request a singleton instance via get() method (if e.g. it's hard to pass a pointer to singleton instance from a method which locked it). If shutdown was scheduled - an exception was previously thrown even though the object was not destroyed yet.

This simplifies conversion of legacy code to folly::Singleton.

Test Plan:
fbconfig -r gatehouse/usersets/tests
fbmake runtests

Reviewed By: chip@fb.com

Subscribers: trunkagent, njormrod

FB internal diff: D1619311
parent a4eda9d3
...@@ -233,12 +233,9 @@ class SingletonVault { ...@@ -233,12 +233,9 @@ class SingletonVault {
void reenableInstances(); 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::weak_ptr<void> get_weak(detail::TypeDescriptor type) {
auto entry = get_entry_create(type); auto entry = get_entry_create(type);
if (UNLIKELY(!entry)) { return entry->instance_weak;
return std::shared_ptr<void>();
}
return entry->instance;
} }
// This function is inherently racy since we don't hold the // This function is inherently racy since we don't hold the
...@@ -247,7 +244,7 @@ class SingletonVault { ...@@ -247,7 +244,7 @@ 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)) { if (UNLIKELY(entry->instance_weak.expired())) {
throw std::runtime_error( throw std::runtime_error(
"Raw pointer to a singleton requested after its destruction."); "Raw pointer to a singleton requested after its destruction.");
} }
...@@ -318,6 +315,7 @@ class SingletonVault { ...@@ -318,6 +315,7 @@ class SingletonVault {
// The singleton itself and related functions. // The singleton itself and related functions.
std::shared_ptr<void> instance; std::shared_ptr<void> instance;
std::weak_ptr<void> instance_weak;
void* instance_ptr = nullptr; void* instance_ptr = nullptr;
CreateFunc create = nullptr; CreateFunc create = nullptr;
TeardownFunc teardown = nullptr; TeardownFunc teardown = nullptr;
...@@ -375,7 +373,7 @@ class SingletonVault { ...@@ -375,7 +373,7 @@ class SingletonVault {
if (entry->instance == nullptr) { if (entry->instance == nullptr) {
RWSpinLock::ReadHolder rh(&stateMutex_); RWSpinLock::ReadHolder rh(&stateMutex_);
if (state_ == SingletonVaultState::Quiescing) { if (state_ == SingletonVaultState::Quiescing) {
return nullptr; return entry;
} }
CHECK(entry->state == SingletonEntryState::Dead); CHECK(entry->state == SingletonEntryState::Dead);
...@@ -396,6 +394,7 @@ class SingletonVault { ...@@ -396,6 +394,7 @@ class SingletonVault {
CHECK(entry->state == SingletonEntryState::BeingBorn); CHECK(entry->state == SingletonEntryState::BeingBorn);
entry->instance = instance; entry->instance = instance;
entry->instance_weak = instance;
entry->instance_ptr = instance.get(); entry->instance_ptr = instance.get();
entry->state = SingletonEntryState::Living; entry->state = SingletonEntryState::Living;
entry->state_condvar.notify_all(); entry->state_condvar.notify_all();
...@@ -456,7 +455,16 @@ class Singleton { ...@@ -456,7 +455,16 @@ class Singleton {
static std::weak_ptr<T> get_weak( static std::weak_ptr<T> get_weak(
const char* name, SingletonVault* vault = nullptr /* for testing */) { const char* name, SingletonVault* vault = nullptr /* for testing */) {
return std::weak_ptr<T>(get_shared({typeid(T), name}, vault)); auto weak_void_ptr =
(vault ?: SingletonVault::singleton())->get_weak({typeid(T), name});
// This is ugly and inefficient, but there's no other way to do it, because
// there's no static_pointer_cast for weak_ptr.
auto shared_void_ptr = weak_void_ptr.lock();
if (!shared_void_ptr) {
return std::weak_ptr<T>();
}
return std::static_pointer_cast<T>(shared_void_ptr);
} }
// Allow the Singleton<t> instance to also retrieve the underlying // Allow the Singleton<t> instance to also retrieve the underlying
...@@ -532,7 +540,7 @@ class Singleton { ...@@ -532,7 +540,7 @@ class Singleton {
detail::TypeDescriptor type_descriptor = {typeid(T), ""}, detail::TypeDescriptor type_descriptor = {typeid(T), ""},
SingletonVault* vault = nullptr /* for testing */) { SingletonVault* vault = nullptr /* for testing */) {
return std::static_pointer_cast<T>( return std::static_pointer_cast<T>(
(vault ?: SingletonVault::singleton())->get_shared(type_descriptor)); (vault ?: SingletonVault::singleton())->get_weak(type_descriptor).lock());
} }
detail::TypeDescriptor type_descriptor_; detail::TypeDescriptor type_descriptor_;
......
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