From b2f445a27494e122632e7987d7171b7b33fed9fb Mon Sep 17 00:00:00 2001 From: Nicholas Ormrod Date: Fri, 6 Feb 2015 13:34:15 -0800 Subject: [PATCH 1/1] Revert "Using type-tags for test SingletonVaults" Summary: This reverts commit 315cfed207895455ecd359d0c1b9d98f28ed0519. Test Plan: fbconfig -r folly && fbmake dbg Reviewed By: robbert@fb.com Subscribers: sdwilsh, folly-diffs@, yfeldblum FB internal diff: D1831932 Signature: t1:1831932:1423258329:0962b939a93bbf1722a9e1c90090dcc024b63765 Blame Revision: D1823663 --- folly/experimental/Singleton.cpp | 5 + folly/experimental/Singleton.h | 57 +++--- folly/experimental/test/SingletonTest.cpp | 223 +++++++++------------- 3 files changed, 128 insertions(+), 157 deletions(-) diff --git a/folly/experimental/Singleton.cpp b/folly/experimental/Singleton.cpp index 20522b24..560175fb 100644 --- a/folly/experimental/Singleton.cpp +++ b/folly/experimental/Singleton.cpp @@ -86,6 +86,11 @@ void SingletonVault::reenableInstances() { state_ = SingletonVaultState::Running; } +SingletonVault* SingletonVault::singleton() { + static SingletonVault* vault = new SingletonVault(); + return vault; +} + void SingletonVault::scheduleDestroyInstances() { RequestContext::getStaticContext(); diff --git a/folly/experimental/Singleton.h b/folly/experimental/Singleton.h index 6cb94857..a715a278 100644 --- a/folly/experimental/Singleton.h +++ b/folly/experimental/Singleton.h @@ -399,17 +399,7 @@ class SingletonVault { // A well-known vault; you can actually have others, but this is the // default. - static SingletonVault* singleton() { - return singleton<>(); - } - - // Gets singleton vault for any Tag. Non-default tag should be used in unit - // tests only. - template - static SingletonVault* singleton() { - static SingletonVault* vault = new SingletonVault(); - return vault; - } + static SingletonVault* singleton(); private: // The two stages of life for a vault, as mentioned in the class comment. @@ -544,9 +534,7 @@ class SingletonVault { // singletons. Create instances of this class in the global scope of // type Singleton to register your singleton for later access via // Singleton::get(). -template +template class Singleton { public: typedef std::function CreateFunc; @@ -555,9 +543,9 @@ class Singleton { // Generally your program life cycle should be fine with calling // get() repeatedly rather than saving the reference, and then not // call get() during process shutdown. - static T* get() { + static T* get(SingletonVault* vault = nullptr /* for testing */) { return static_cast( - SingletonVault::singleton()->get_ptr(typeDescriptor())); + (vault ?: SingletonVault::singleton())->get_ptr(typeDescriptor())); } // Same as get, but should be preffered to it in the same compilation @@ -566,7 +554,7 @@ class Singleton { if (LIKELY(entry_->state == detail::SingletonEntryState::Living)) { return reinterpret_cast(entry_->instance_ptr); } else { - return get(); + return get(vault_); } } @@ -574,9 +562,10 @@ class Singleton { // 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() { + static std::weak_ptr get_weak( + SingletonVault* vault = nullptr /* for testing */) { auto weak_void_ptr = - (SingletonVault::singleton())->get_weak(typeDescriptor()); + (vault ?: SingletonVault::singleton())->get_weak(typeDescriptor()); // This is ugly and inefficient, but there's no other way to do it, because // there's no static_pointer_cast for weak_ptr. @@ -599,7 +588,7 @@ class Singleton { } return std::static_pointer_cast(shared_void_ptr); } else { - return get_weak(); + return get_weak(vault_); } } @@ -610,19 +599,26 @@ class Singleton { T* operator->() { return ptr(); } explicit Singleton(std::nullptr_t _ = nullptr, - Singleton::TeardownFunc t = nullptr) : - Singleton ([]() { return new T; }, std::move(t)) { + Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr) : + Singleton ([]() { return new T; }, + std::move(t), + vault) { } explicit Singleton(Singleton::CreateFunc c, - Singleton::TeardownFunc t = nullptr) { + Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr) { if (c == nullptr) { throw std::logic_error( "nullptr_t should be passed if you want T to be default constructed"); } - auto vault = SingletonVault::singleton(); + if (vault == nullptr) { + vault = SingletonVault::singleton(); + } + vault_ = vault; entry_ = &(vault->registerSingleton(typeDescriptor(), c, getTeardownFunc(t))); } @@ -638,18 +634,22 @@ class Singleton { * regular singletons. */ static void make_mock(std::nullptr_t c = nullptr, - typename Singleton::TeardownFunc t = nullptr) { - make_mock([]() { return new T; }, t); + typename Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */ ) { + make_mock([]() { return new T; }, t, vault); } static void make_mock(CreateFunc c, - typename Singleton::TeardownFunc t = nullptr) { + typename Singleton::TeardownFunc t = nullptr, + SingletonVault* vault = nullptr /* for testing */ ) { if (c == nullptr) { throw std::logic_error( "nullptr_t should be passed if you want T to be default constructed"); } - auto vault = SingletonVault::singleton(); + if (vault == nullptr) { + vault = SingletonVault::singleton(); + } vault->registerMockSingleton( typeDescriptor(), @@ -680,6 +680,7 @@ class Singleton { // We rely on the fact that Singleton destructor won't reset this pointer, so // it can be "safely" used even after static Singleton object is destroyed. detail::SingletonEntry* entry_; + SingletonVault* vault_; }; } diff --git a/folly/experimental/test/SingletonTest.cpp b/folly/experimental/test/SingletonTest.cpp index 57b25f40..25b270a6 100644 --- a/folly/experimental/test/SingletonTest.cpp +++ b/folly/experimental/test/SingletonTest.cpp @@ -83,34 +83,30 @@ TEST(Singleton, MissingSingleton) { std::out_of_range); } -struct BasicUsageTag {}; -template -using SingletonBasicUsage = Singleton ; - // Exercise some basic codepaths ensuring registration order and // destruction order happen as expected, that instances are created // when expected, etc etc. TEST(Singleton, BasicUsage) { - auto& vault = *SingletonVault::singleton(); + SingletonVault vault; EXPECT_EQ(vault.registeredSingletonCount(), 0); - SingletonBasicUsage watchdog_singleton; + Singleton watchdog_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 1); - SingletonBasicUsage child_watchdog_singleton; + Singleton child_watchdog_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); vault.registrationComplete(); - Watchdog* s1 = SingletonBasicUsage::get(); + Watchdog* s1 = Singleton::get(&vault); EXPECT_NE(s1, nullptr); - Watchdog* s2 = SingletonBasicUsage::get(); + Watchdog* s2 = Singleton::get(&vault); EXPECT_NE(s2, nullptr); EXPECT_EQ(s1, s2); - auto s3 = SingletonBasicUsage::get(); + auto s3 = Singleton::get(&vault); EXPECT_NE(s3, nullptr); EXPECT_NE(s2, s3); @@ -122,38 +118,28 @@ TEST(Singleton, BasicUsage) { EXPECT_EQ(vault.livingSingletonCount(), 0); } -struct DirectUsageTag {}; -template -using SingletonDirectUsage = Singleton ; - TEST(Singleton, DirectUsage) { - auto& vault = *SingletonVault::singleton(); + SingletonVault vault; EXPECT_EQ(vault.registeredSingletonCount(), 0); // Verify we can get to the underlying singletons via directly using // the singleton definition. - SingletonDirectUsage watchdog; + Singleton watchdog(nullptr, nullptr, &vault); struct TestTag {}; - SingletonDirectUsage named_watchdog; + Singleton named_watchdog(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); vault.registrationComplete(); EXPECT_NE(watchdog.ptr(), nullptr); - EXPECT_EQ(watchdog.ptr(), SingletonDirectUsage::get()); + EXPECT_EQ(watchdog.ptr(), Singleton::get(&vault)); EXPECT_NE(watchdog.ptr(), named_watchdog.ptr()); EXPECT_EQ(watchdog->livingWatchdogCount(), 2); EXPECT_EQ((*watchdog).livingWatchdogCount(), 2); - - vault.destroyInstances(); } -struct NamedUsageTag {}; -template -using SingletonNamedUsage = Singleton ; - TEST(Singleton, NamedUsage) { - auto& vault = *SingletonVault::singleton(); + SingletonVault vault; EXPECT_EQ(vault.registeredSingletonCount(), 0); @@ -161,105 +147,96 @@ TEST(Singleton, NamedUsage) { struct Watchdog1 {}; struct Watchdog2 {}; typedef detail::DefaultTag Watchdog3; - SingletonNamedUsage watchdog1_singleton; + Singleton watchdog1_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 1); - SingletonNamedUsage watchdog2_singleton; + Singleton watchdog2_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); - SingletonNamedUsage watchdog3_singleton; + Singleton watchdog3_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 3); vault.registrationComplete(); // Verify our three singletons are distinct and non-nullptr. - Watchdog* s1 = SingletonNamedUsage::get(); + Watchdog* s1 = Singleton::get(&vault); EXPECT_EQ(s1, watchdog1_singleton.ptr()); - Watchdog* s2 = SingletonNamedUsage::get(); + Watchdog* s2 = Singleton::get(&vault); EXPECT_EQ(s2, watchdog2_singleton.ptr()); EXPECT_NE(s1, s2); - Watchdog* s3 = SingletonNamedUsage::get(); + 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 DefaultTag-tagged // singleton. - Watchdog* s4 = SingletonNamedUsage::get(); + Watchdog* s4 = Singleton::get(&vault); EXPECT_EQ(s4, watchdog3_singleton.ptr()); - - vault.destroyInstances(); } -struct NaughtyUsageTag {}; -template -using SingletonNaughtyUsage = Singleton ; -struct NaughtyUsageTag2 {}; -template -using SingletonNaughtyUsage2 = Singleton ; - // Some pathological cases such as getting unregistered singletons, // double registration, etc. TEST(Singleton, NaughtyUsage) { - auto& vault = *SingletonVault::singleton(); - + SingletonVault vault(SingletonVault::Type::Strict); vault.registrationComplete(); // Unregistered. EXPECT_THROW(Singleton::get(), std::out_of_range); - EXPECT_THROW(SingletonNaughtyUsage::get(), std::out_of_range); - - vault.destroyInstances(); - - auto& vault2 = *SingletonVault::singleton(); - - EXPECT_THROW(SingletonNaughtyUsage2::get(), std::logic_error); - SingletonNaughtyUsage2 watchdog_singleton; + EXPECT_THROW(Singleton::get(&vault), std::out_of_range); + + // Registring singletons after registrationComplete called. + EXPECT_THROW([&vault]() { + Singleton watchdog_singleton( + nullptr, nullptr, &vault); + }(), + std::logic_error); + + SingletonVault vault_2(SingletonVault::Type::Strict); + EXPECT_THROW(Singleton::get(&vault_2), std::logic_error); + Singleton watchdog_singleton(nullptr, nullptr, &vault_2); // double registration - EXPECT_THROW([]() { - SingletonNaughtyUsage2 watchdog_singleton; - }(), - std::logic_error); - vault2.destroyInstances(); + EXPECT_THROW([&vault_2]() { + Singleton watchdog_singleton( + nullptr, nullptr, &vault_2); + }(), + std::logic_error); + vault_2.destroyInstances(); // double registration after destroy - EXPECT_THROW([]() { - SingletonNaughtyUsage2 watchdog_singleton; - }(), - std::logic_error); + EXPECT_THROW([&vault_2]() { + Singleton watchdog_singleton( + nullptr, nullptr, &vault_2); + }(), + std::logic_error); } -struct SharedPtrUsageTag {}; -template -using SingletonSharedPtrUsage = Singleton ; - TEST(Singleton, SharedPtrUsage) { - auto& vault = *SingletonVault::singleton(); + SingletonVault vault; EXPECT_EQ(vault.registeredSingletonCount(), 0); - SingletonSharedPtrUsage watchdog_singleton; + Singleton watchdog_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 1); - SingletonSharedPtrUsage child_watchdog_singleton; + Singleton child_watchdog_singleton(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 2); struct ATag {}; - SingletonSharedPtrUsage named_watchdog_singleton; + Singleton named_watchdog_singleton(nullptr, nullptr, &vault); vault.registrationComplete(); - Watchdog* s1 = SingletonSharedPtrUsage::get(); + Watchdog* s1 = Singleton::get(&vault); EXPECT_NE(s1, nullptr); - Watchdog* s2 = SingletonSharedPtrUsage::get(); + Watchdog* s2 = Singleton::get(&vault); EXPECT_NE(s2, nullptr); EXPECT_EQ(s1, s2); - auto weak_s1 = SingletonSharedPtrUsage::get_weak(); + auto weak_s1 = Singleton::get_weak(&vault); auto shared_s1 = weak_s1.lock(); EXPECT_EQ(shared_s1.get(), s1); EXPECT_EQ(shared_s1.use_count(), 2); { - auto named_weak_s1 = - SingletonSharedPtrUsage::get_weak(); + auto named_weak_s1 = Singleton::get_weak(&vault); auto locked = named_weak_s1.lock(); EXPECT_NE(locked.get(), shared_s1.get()); } @@ -291,16 +268,16 @@ TEST(Singleton, SharedPtrUsage) { locked_s1 = weak_s1.lock(); EXPECT_TRUE(weak_s1.expired()); - auto empty_s1 = SingletonSharedPtrUsage::get_weak(); + auto empty_s1 = Singleton::get_weak(&vault); EXPECT_FALSE(empty_s1.lock()); vault.reenableInstances(); // Singleton should be re-created only after reenableInstances() was called. - Watchdog* new_s1 = SingletonSharedPtrUsage::get(); + Watchdog* new_s1 = Singleton::get(&vault); EXPECT_NE(new_s1->serial_number, old_serial); - auto new_s1_weak = SingletonSharedPtrUsage::get_weak(); + auto new_s1_weak = Singleton::get_weak(&vault); auto new_s1_shared = new_s1_weak.lock(); std::thread t([new_s1_shared]() mutable { std::this_thread::sleep_for(std::chrono::seconds{2}); @@ -321,51 +298,43 @@ TEST(Singleton, SharedPtrUsage) { // Some classes to test singleton dependencies. NeedySingleton has a // dependency on NeededSingleton, which happens during its // construction. -struct NeedyTag {}; -template -using SingletonNeedy = Singleton ; +SingletonVault needy_vault; struct NeededSingleton {}; struct NeedySingleton { NeedySingleton() { - auto unused = SingletonNeedy::get(); + auto unused = Singleton::get(&needy_vault); EXPECT_NE(unused, nullptr); } }; // Ensure circular dependencies fail -- a singleton that needs itself, whoops. -struct SelfNeedyTag {}; -template -using SingletonSelfNeedy = Singleton ; - +SingletonVault self_needy_vault; struct SelfNeedySingleton { SelfNeedySingleton() { - auto unused = SingletonSelfNeedy::get(); + auto unused = Singleton::get(&self_needy_vault); EXPECT_NE(unused, nullptr); } }; TEST(Singleton, SingletonDependencies) { - SingletonNeedy needed_singleton; - SingletonNeedy needy_singleton; - auto& needy_vault = *SingletonVault::singleton(); - + Singleton needed_singleton(nullptr, nullptr, &needy_vault); + Singleton needy_singleton(nullptr, nullptr, &needy_vault); needy_vault.registrationComplete(); EXPECT_EQ(needy_vault.registeredSingletonCount(), 2); EXPECT_EQ(needy_vault.livingSingletonCount(), 0); - auto needy = SingletonNeedy::get(); + auto needy = Singleton::get(&needy_vault); EXPECT_EQ(needy_vault.livingSingletonCount(), 2); - SingletonSelfNeedy self_needy_singleton; - auto& self_needy_vault = *SingletonVault::singleton(); - + Singleton self_needy_singleton( + nullptr, nullptr, &self_needy_vault); self_needy_vault.registrationComplete(); EXPECT_THROW([]() { - SingletonSelfNeedy::get(); - }(), - std::out_of_range); + Singleton::get(&self_needy_vault); + }(), + std::out_of_range); } // A test to ensure multiple threads contending on singleton creation @@ -376,21 +345,17 @@ class Slowpoke : public Watchdog { Slowpoke() { std::this_thread::sleep_for(std::chrono::milliseconds(10)); } }; -struct ConcurrencyTag {}; -template -using SingletonConcurrency = Singleton ; - TEST(Singleton, SingletonConcurrency) { - auto& vault = *SingletonVault::singleton(); - SingletonConcurrency slowpoke_singleton; + SingletonVault vault; + Singleton slowpoke_singleton(nullptr, nullptr, &vault); vault.registrationComplete(); std::mutex gatekeeper; gatekeeper.lock(); - auto func = [&gatekeeper]() { + auto func = [&vault, &gatekeeper]() { gatekeeper.lock(); gatekeeper.unlock(); - auto unused = SingletonConcurrency::get(); + auto unused = Singleton::get(&vault); }; EXPECT_EQ(vault.livingSingletonCount(), 0); @@ -408,18 +373,14 @@ TEST(Singleton, SingletonConcurrency) { EXPECT_EQ(vault.livingSingletonCount(), 1); } -struct ConcurrencyStressTag {}; -template -using SingletonConcurrencyStress = Singleton ; - TEST(Singleton, SingletonConcurrencyStress) { - auto& vault = *SingletonVault::singleton(); - SingletonConcurrencyStress slowpoke_singleton; + SingletonVault vault; + Singleton slowpoke_singleton(nullptr, nullptr, &vault); std::vector ts; for (size_t i = 0; i < 100; ++i) { ts.emplace_back([&]() { - slowpoke_singleton.get_weak().lock(); + slowpoke_singleton.get_weak(&vault).lock(); }); } @@ -451,28 +412,23 @@ int* getNormalSingleton() { return &normal_singleton_value; } -struct MockTag {}; -template -using SingletonMock = Singleton ; - // Verify that existing Singleton's can be overridden // using the make_mock functionality. -TEST(Singleton, MockTest) { - auto& vault = *SingletonVault::singleton(); - - SingletonMock watchdog_singleton; +TEST(Singleton, make_mock) { + SingletonVault vault(SingletonVault::Type::Strict); + Singleton watchdog_singleton(nullptr, nullptr, &vault); vault.registrationComplete(); // Registring singletons after registrationComplete called works // with make_mock (but not with Singleton ctor). EXPECT_EQ(vault.registeredSingletonCount(), 1); - int serial_count_first = SingletonMock::get()->serial_number; + int serial_count_first = Singleton::get(&vault)->serial_number; // Override existing mock using make_mock. - SingletonMock::make_mock(); + Singleton::make_mock(nullptr, nullptr, &vault); EXPECT_EQ(vault.registeredSingletonCount(), 1); - int serial_count_mock = SingletonMock::get()->serial_number; + int serial_count_mock = Singleton::get(&vault)->serial_number; // If serial_count value is the same, then singleton was not replaced. EXPECT_NE(serial_count_first, serial_count_mock); @@ -494,25 +450,34 @@ BENCHMARK_RELATIVE(MeyersSingleton, n) { } } -struct BenchmarkTag {}; -template -using SingletonBenchmark = Singleton ; - -SingletonBenchmark benchmark_singleton; - BENCHMARK_RELATIVE(FollySingletonSlow, n) { + SingletonVault benchmark_vault; + Singleton benchmark_singleton( + nullptr, nullptr, &benchmark_vault); + benchmark_vault.registrationComplete(); + for (size_t i = 0; i < n; ++i) { - doNotOptimizeAway(SingletonBenchmark::get()); + doNotOptimizeAway(Singleton::get(&benchmark_vault)); } } BENCHMARK_RELATIVE(FollySingletonFast, n) { + SingletonVault benchmark_vault; + Singleton benchmark_singleton( + nullptr, nullptr, &benchmark_vault); + benchmark_vault.registrationComplete(); + for (size_t i = 0; i < n; ++i) { doNotOptimizeAway(benchmark_singleton.get_fast()); } } BENCHMARK_RELATIVE(FollySingletonFastWeak, n) { + SingletonVault benchmark_vault; + Singleton benchmark_singleton( + nullptr, nullptr, &benchmark_vault); + benchmark_vault.registrationComplete(); + for (size_t i = 0; i < n; ++i) { benchmark_singleton.get_weak_fast(); } -- 2.34.1