From: Chip Turner Date: Wed, 30 Jul 2014 22:38:28 +0000 (-0700) Subject: Make using folly::Singleton easier: names and direct referencing X-Git-Tag: v0.22.0~379 X-Git-Url: http://plrg.eecs.uci.edu/git/?a=commitdiff_plain;h=95f17b5283ce53886181d6315b1945149696f7eb;p=folly.git Make using folly::Singleton easier: names and direct referencing Summary: There are times when you want multiple singletons of the same underlying type -- for instance, a fast and slow request handling singleton. This diff allows for that with an optional name that disambiguates multiple singletons of the same type. In addition, we now also allow direct dereferencing of the Singleton object to get to the underlying singleton. This is most useful in cases where a singleton is used inside of the same cpp file it is defined. Finally, make get() faster by caching the underlying pointer rather than accessing the shared pointer. If you're using it, you already have a race condition and hopefully your singleton lifecycle is sane and safe from problems this may cause. Test Plan: runtests Reviewed By: hans@fb.com Subscribers: njormrod, lins, anca FB internal diff: D1485887 --- diff --git a/folly/experimental/Singleton.h b/folly/experimental/Singleton.h index 0c12519b..ef966dee 100644 --- a/folly/experimental/Singleton.h +++ b/folly/experimental/Singleton.h @@ -34,12 +34,30 @@ // std::weak_ptr instance = // Singleton::get_weak(); // +// You also can directly access it by the variable defining the +// singleton rather than via get(), and even treat that variable like +// a smart pointer (dereferencing it or using the -> operator). +// +// Please note, however, that all non-weak_ptr interfaces are +// inherently subject to races with destruction. Use responsibly. +// // The singleton will be created on demand. If the constructor for // MyExpensiveService actually makes use of *another* Singleton, then // the right thing will happen -- that other singleton will complete // construction before get() returns. However, in the event of a // circular dependency, a runtime error will occur. // +// You can have multiple singletons of the same underlying type, but +// each must be given a unique name: +// +// namespace { +// folly::Singleton s1("name1"); +// folly::Singleton s2("name2"); +// } +// ... +// MyExpensiveService* svc1 = Singleton::get("name1"); +// MyExpensiveService* svc2 = Singleton::get("name2"); +// // By default, the singleton instance is constructed via new and // deleted via delete, but this is configurable: // @@ -59,6 +77,7 @@ #pragma once #include +#include #include #include @@ -92,6 +111,54 @@ namespace folly { // In general, you don't need to worry about any of the above; just // ensure registrationComplete() is called near the top of your main() // function, otherwise no singletons can be instantiated. + +namespace detail { + +const char* const kDefaultTypeDescriptorName = "(default)"; +// A TypeDescriptor is the unique handle for a given singleton. It is +// a combinaiton of the type and of the optional name, and is used as +// a key in unordered_maps. +class TypeDescriptor { + public: + TypeDescriptor(const std::type_info& ti, std::string name) + : ti_(ti), name_(name) { + if (name_ == kDefaultTypeDescriptorName) { + LOG(DFATAL) << "Caller used the default name as their literal name; " + << "name your singleton something other than " + << kDefaultTypeDescriptorName; + } + } + + std::string name() const { + std::string ret = ti_.name(); + ret += "/"; + if (name_.empty()) { + ret += kDefaultTypeDescriptorName; + } else { + ret += name_; + } + return ret; + } + + friend class TypeDescriptorHasher; + + bool operator==(const TypeDescriptor& other) const { + return ti_ == other.ti_ && name_ == other.name_; + } + + private: + const std::type_index ti_; + const std::string name_; +}; + +class TypeDescriptorHasher { + public: + size_t operator()(const TypeDescriptor& ti) const { + return folly::hash::hash_combine(ti.ti_, ti.name_); + } +}; +} + class SingletonVault { public: SingletonVault() {}; @@ -102,7 +169,7 @@ class SingletonVault { // Register a singleton of a given type with the create and teardown // functions. - void registerSingleton(const std::type_info& type, + void registerSingleton(detail::TypeDescriptor type, CreateFunc create, TeardownFunc teardown) { std::lock_guard guard(mutex_); @@ -136,51 +203,22 @@ class SingletonVault { void destroyInstances(); // Retrieve a singleton from the vault, creating it if necessary. - std::shared_ptr get_shared(const std::type_info& type) { + std::shared_ptr get_shared(detail::TypeDescriptor type) { std::unique_lock lock(mutex_); - if (state_ != SingletonVaultState::Running) { - throw std::logic_error( - "Attempt to load a singleton before " - "SingletonVault::registrationComplete was called (hint: you probably " - "didn't call initFacebook)"); - } - - auto it = singletons_.find(type); - if (it == singletons_.end()) { - throw std::out_of_range(std::string("non-existent singleton: ") + - type.name()); - } - - auto& entry = it->second; - std::unique_lock entry_lock(entry->mutex_); - - if (entry->state == SingletonEntryState::BeingBorn) { - throw std::out_of_range(std::string("circular singleton dependency: ") + - type.name()); - } - - if (entry->instance == nullptr) { - CHECK(entry->state == SingletonEntryState::Dead); - entry->state = SingletonEntryState::BeingBorn; - - entry_lock.unlock(); - lock.unlock(); - // Can't use make_shared -- no support for a custom deleter, sadly. - auto instance = std::shared_ptr(entry->create(), entry->teardown); - lock.lock(); - entry_lock.lock(); - - CHECK(entry->state == SingletonEntryState::BeingBorn); - entry->instance = instance; - entry->state = SingletonEntryState::Living; - - creation_order_.push_back(type); - } - CHECK(entry->state == SingletonEntryState::Living); - + auto entry = get_entry(type, &lock); return entry->instance; } + // This function is inherently racy since we don't hold the + // shared_ptr that contains the Singleton. It is the caller's + // responsibility to be sane with this, but it is preferable to use + // the weak_ptr interface for true safety. + void* get_ptr(detail::TypeDescriptor type) { + std::unique_lock lock(mutex_); + auto entry = get_entry(type, &lock); + return entry->instance_ptr; + } + // For testing; how many registered and living singletons we have. size_t registeredSingletonCount() const { std::lock_guard guard(mutex_); @@ -225,6 +263,7 @@ class SingletonVault { struct SingletonEntry { std::mutex mutex_; std::shared_ptr instance; + void* instance_ptr = nullptr; CreateFunc create = nullptr; TeardownFunc teardown = nullptr; SingletonEntryState state = SingletonEntryState::Dead; @@ -236,10 +275,61 @@ class SingletonVault { SingletonEntry(SingletonEntry&&) = delete; }; + // Get a pointer to the living SingletonEntry for the specified + // type. The singleton is created as part of this function, if + // necessary. + SingletonEntry* get_entry(detail::TypeDescriptor type, + std::unique_lock* lock) { + // mutex_ must be held when calling this function + if (state_ != SingletonVaultState::Running) { + throw std::logic_error( + "Attempt to load a singleton before " + "SingletonVault::registrationComplete was called (hint: you probably " + "didn't call initFacebook)"); + } + + auto it = singletons_.find(type); + if (it == singletons_.end()) { + throw std::out_of_range(std::string("non-existent singleton: ") + + type.name()); + } + + auto& entry = it->second; + std::unique_lock entry_lock(entry->mutex_); + + if (entry->state == SingletonEntryState::BeingBorn) { + throw std::out_of_range(std::string("circular singleton dependency: ") + + type.name()); + } + + if (entry->instance == nullptr) { + CHECK(entry->state == SingletonEntryState::Dead); + entry->state = SingletonEntryState::BeingBorn; + + entry_lock.unlock(); + lock->unlock(); + // Can't use make_shared -- no support for a custom deleter, sadly. + auto instance = std::shared_ptr(entry->create(), entry->teardown); + lock->lock(); + entry_lock.lock(); + + CHECK(entry->state == SingletonEntryState::BeingBorn); + entry->instance = instance; + entry->instance_ptr = instance.get(); + entry->state = SingletonEntryState::Living; + + creation_order_.push_back(type); + } + CHECK(entry->state == SingletonEntryState::Living); + return entry.get(); + } + mutable std::mutex mutex_; typedef std::unique_ptr SingletonEntryPtr; - std::unordered_map singletons_; - std::vector creation_order_; + std::unordered_map singletons_; + std::vector creation_order_; SingletonVaultState state_ = SingletonVaultState::Registering; }; @@ -258,21 +348,55 @@ class Singleton { // get() repeatedly rather than saving the reference, and then not // call get() during process shutdown. static T* get(SingletonVault* vault = nullptr /* for testing */) { - return get_shared(vault).get(); + return get_ptr({typeid(T), ""}, vault); + } + + static T* get(const char* name, + SingletonVault* vault = nullptr /* for testing */) { + return get_ptr({typeid(T), name}, vault); } // If, however, you do need to hold a reference to the specific // singleton, you can try to do so with a weak_ptr. Avoid this when // possible but the inability to lock the weak pointer can be a // signal that the vault has been destroyed. - static std::weak_ptr get_weak(SingletonVault* vault = - nullptr /* for testing */) { - return std::weak_ptr(get_shared(vault)); + static std::weak_ptr get_weak( + SingletonVault* vault = nullptr /* for testing */) { + return get_weak("", vault); + } + + static std::weak_ptr get_weak( + const char* name, SingletonVault* vault = nullptr /* for testing */) { + return std::weak_ptr(get_shared({typeid(T), name}, vault)); + } + + std::weak_ptr get_weak(const char* name) { + return std::weak_ptr(get_shared({typeid(T), name}, vault_)); } - Singleton(Singleton::CreateFunc c = nullptr, - Singleton::TeardownFunc t = nullptr, - SingletonVault* vault = nullptr /* for testing */) { + // Allow the Singleton instance to also retrieve the underlying + // singleton, if desired. + T* ptr() { return get_ptr(type_descriptor_, vault_); } + T& operator*() { return *ptr(); } + T* operator->() { return ptr(); } + + explicit Singleton(Singleton::CreateFunc c = nullptr, + Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */) + : Singleton({typeid(T), ""}, c, t, vault) {} + + explicit Singleton(const char* name, + Singleton::CreateFunc c = nullptr, + Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */) + : Singleton({typeid(T), name}, c, t, vault) {} + + private: + explicit Singleton(detail::TypeDescriptor type, + Singleton::CreateFunc c = nullptr, + Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */) + : type_descriptor_(type) { if (c == nullptr) { c = []() { return new T; }; } @@ -286,11 +410,16 @@ class Singleton { if (vault == nullptr) { vault = SingletonVault::singleton(); } + vault_ = vault; + vault->registerSingleton(type, c, teardown); + } - vault->registerSingleton(typeid(T), c, teardown); + static T* get_ptr(detail::TypeDescriptor type_descriptor = {typeid(T), ""}, + SingletonVault* vault = nullptr /* for testing */) { + return static_cast( + (vault ?: SingletonVault::singleton())->get_ptr(type_descriptor)); } - private: // Don't use this function, it's private for a reason! Using it // would defeat the *entire purpose* of the vault in that we lose // the ability to guarantee that, after a destroyInstances is @@ -301,10 +430,14 @@ class Singleton { // Yes, you can just get the weak pointer and lock it, but hopefully // if you have taken the time to read this far, you see why that // would be bad. - static std::shared_ptr get_shared(SingletonVault* vault = - nullptr /* for testing */) { + static std::shared_ptr get_shared( + detail::TypeDescriptor type_descriptor = {typeid(T), ""}, + SingletonVault* vault = nullptr /* for testing */) { return std::static_pointer_cast( - (vault ?: SingletonVault::singleton())->get_shared(typeid(T))); + (vault ?: SingletonVault::singleton())->get_shared(type_descriptor)); } + + detail::TypeDescriptor type_descriptor_; + SingletonVault* vault_; }; } diff --git a/folly/experimental/test/SingletonTest.cpp b/folly/experimental/test/SingletonTest.cpp index f0815d91..91c8bb79 100644 --- a/folly/experimental/test/SingletonTest.cpp +++ b/folly/experimental/test/SingletonTest.cpp @@ -41,6 +41,7 @@ struct Watchdog { } const size_t serial_number; + size_t livingWatchdogCount() const { return creation_order.size(); } Watchdog(const Watchdog&) = delete; Watchdog& operator=(const Watchdog&) = delete; @@ -115,6 +116,59 @@ TEST(Singleton, BasicUsage) { EXPECT_EQ(vault.livingSingletonCount(), 0); } +TEST(Singleton, DirectUsage) { + SingletonVault vault; + + EXPECT_EQ(vault.registeredSingletonCount(), 0); + + // Verify we can get to the underlying singletons via directly using + // the singleton definition. + Singleton watchdog(nullptr, nullptr, &vault); + Singleton named_watchdog("named", nullptr, nullptr, &vault); + EXPECT_EQ(vault.registeredSingletonCount(), 2); + vault.registrationComplete(); + + EXPECT_NE(watchdog.ptr(), nullptr); + EXPECT_EQ(watchdog.ptr(), Singleton::get(&vault)); + EXPECT_NE(watchdog.ptr(), named_watchdog.ptr()); + EXPECT_EQ(watchdog->livingWatchdogCount(), 2); + EXPECT_EQ((*watchdog).livingWatchdogCount(), 2); +} + +TEST(Singleton, NamedUsage) { + SingletonVault vault; + + EXPECT_EQ(vault.registeredSingletonCount(), 0); + + // Define two named Watchdog singletons and one unnamed singleton. + Singleton watchdog1_singleton( + "watchdog1", nullptr, nullptr, &vault); + EXPECT_EQ(vault.registeredSingletonCount(), 1); + Singleton watchdog2_singleton( + "watchdog2", nullptr, nullptr, &vault); + EXPECT_EQ(vault.registeredSingletonCount(), 2); + Singleton watchdog3_singleton(nullptr, nullptr, &vault); + EXPECT_EQ(vault.registeredSingletonCount(), 3); + + vault.registrationComplete(); + + // Verify our three singletons are distinct and non-nullptr. + Watchdog* s1 = Singleton::get("watchdog1", &vault); + EXPECT_EQ(s1, watchdog1_singleton.ptr()); + Watchdog* s2 = Singleton::get("watchdog2", &vault); + EXPECT_EQ(s2, watchdog2_singleton.ptr()); + EXPECT_NE(s1, s2); + Watchdog* s3 = Singleton::get(&vault); + EXPECT_EQ(s3, watchdog3_singleton.ptr()); + EXPECT_NE(s3, s1); + EXPECT_NE(s3, s2); + + // Verify the "default" singleton is the same as the empty string + // singleton. + Watchdog* s4 = Singleton::get("", &vault); + EXPECT_EQ(s4, watchdog3_singleton.ptr()); +} + // Some pathological cases such as getting unregistered singletons, // double registration, etc. TEST(Singleton, NaughtyUsage) { @@ -163,6 +217,8 @@ TEST(Singleton, SharedPtrUsage) { Singleton child_watchdog_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); + Singleton named_watchdog_singleton( + "a_name", nullptr, nullptr, &vault); vault.registrationComplete(); Watchdog* s1 = Singleton::get(&vault); @@ -178,9 +234,15 @@ TEST(Singleton, SharedPtrUsage) { EXPECT_EQ(shared_s1.get(), s1); EXPECT_EQ(shared_s1.use_count(), 2); + { + auto named_weak_s1 = Singleton::get_weak("a_name", &vault); + auto locked = named_weak_s1.lock(); + EXPECT_NE(locked.get(), shared_s1.get()); + } + LOG(ERROR) << "The following log message regarding ref counts is expected"; vault.destroyInstances(); - EXPECT_EQ(vault.registeredSingletonCount(), 2); + EXPECT_EQ(vault.registeredSingletonCount(), 3); EXPECT_EQ(vault.livingSingletonCount(), 0); EXPECT_EQ(shared_s1.use_count(), 1);