folly:: singleton: ubsan: fix calling overriden method from ctor

Summary:
Ubsan complains when overriden method is called from ctor:
```
0x000000b04d40: note: object is of type 'folly::detail::SingletonHolder<XYZ>' 00 00 00 00  08 50 c3 8d da 7f 00 00  e8 4f c3 8d da 7f 00 00  18 0d 4c b4 da 7f 00 00  a0 16 90 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              vptr for 'folly::detail::SingletonHolder<XYZ>'
```

called from `folly::SingletonVault::registerSingleton(folly::detail::SingletonHolderBase*)`

Reviewed By: ot

Differential Revision: D3462037

fbshipit-source-id: 6df283dd53df42d5d318990f60aba370ceed6395

diff --git a/folly/Singleton-inl.h b/folly/Singleton-inl.h
index a643e0374d413614d78ac627ac303eec5474b198..6c6e995cb81085b2898d2a111392fd98c52cb333 100644 (file)
--- a/folly/Singleton-inl.h
+++ b/folly/Singleton-inl.h
@@ -54,7 +54,7 @@ void SingletonHolder<T>::registerSingleton(CreateFunc c, TeardownFunc t) {
      * Singleton<int> b([] { return new int(4); });
      *
      */
-    singletonWarnDoubleRegistrationAndAbort(type_);
+    singletonWarnDoubleRegistrationAndAbort(type());
   }
 
   create_ = std::move(c);
@@ -66,8 +66,8 @@ void SingletonHolder<T>::registerSingleton(CreateFunc c, TeardownFunc t) {
 template <typename T>
 void SingletonHolder<T>::registerSingletonMock(CreateFunc c, TeardownFunc t) {
   if (state_ == SingletonHolderState::NotRegistered) {
-    LOG(FATAL)
-        << "Registering mock before singleton was registered: " << type_.name();
+    LOG(FATAL) << "Registering mock before singleton was registered: "
+               << type().name();
   }
   destroyInstance();
 
@@ -89,7 +89,7 @@ T* SingletonHolder<T>::get() {
     throw std::runtime_error(
         "Raw pointer to a singleton requested after its destruction."
         " Singleton type is: " +
-        type_.name());
+        type().name());
   }
 
   return instance_ptr_;
@@ -125,11 +125,6 @@ folly::ReadMostlySharedPtr<T> SingletonHolder<T>::try_get_fast() {
   return instance_weak_fast_.lock();
 }
 
-template <typename T>
-TypeDescriptor SingletonHolder<T>::type() {
-  return type_;
-}
-
 template <typename T>
 bool SingletonHolder<T>::hasLiveInstance() {
   return !instance_weak_.expired();
@@ -145,7 +140,7 @@ void SingletonHolder<T>::destroyInstance() {
       std::chrono::steady_clock::now() + kDestroyWaitTime);
     if (!wait_result) {
       print_destructor_stack_trace_->store(true);
-      LOG(ERROR) << "Singleton of type " << type_.name() << " has a "
+      LOG(ERROR) << "Singleton of type " << type().name() << " has a "
                  << "living reference at destroyInstances time; beware! Raw "
                  << "pointer is " << instance_ptr_ << ". It is very likely "
                  << "that some other singleton is holding a shared_ptr to it. "
@@ -156,10 +151,10 @@ void SingletonHolder<T>::destroyInstance() {
 }
 
 template <typename T>
-SingletonHolder<T>::SingletonHolder(TypeDescriptor type__,
-                                    SingletonVault& vault) :
-    type_(type__), vault_(vault) {
-}
+SingletonHolder<T>::SingletonHolder(
+    TypeDescriptor typeDesc,
+    SingletonVault& vault)
+    : SingletonHolderBase(typeDesc), vault_(vault) {}
 
 template <typename T>
 bool SingletonHolder<T>::creationStarted() {
@@ -181,7 +176,7 @@ template <typename T>
 void SingletonHolder<T>::createInstance() {
   if (creating_thread_.load(std::memory_order_acquire) ==
         std::this_thread::get_id()) {
-    LOG(FATAL) << "circular singleton dependency: " << type_.name();
+    LOG(FATAL) << "circular singleton dependency: " << type().name();
   }
 
   std::lock_guard<std::mutex> entry_lock(mutex_);
@@ -192,9 +187,10 @@ void SingletonHolder<T>::createInstance() {
         SingletonHolderState::NotRegistered) {
     auto ptr = SingletonVault::stackTraceGetter().load();
     LOG(FATAL) << "Creating instance for unregistered singleton: "
-               << type_.name() << "\n"
+               << type().name() << "\n"
                << "Stacktrace:"
-               << "\n" << (ptr ? (*ptr)() : "(not available)");
+               << "\n"
+               << (ptr ? (*ptr)() : "(not available)");
   }
 
   if (state_.load(std::memory_order_acquire) == SingletonHolderState::Living) {
@@ -219,7 +215,7 @@ void SingletonHolder<T>::createInstance() {
   auto print_destructor_stack_trace =
     std::make_shared<std::atomic<bool>>(false);
   auto teardown = teardown_;
-  auto type_name = type_.name();
+  auto type_name = type().name();
 
   // Can't use make_shared -- no support for a custom deleter, sadly.
   std::shared_ptr<T> instance(
@@ -264,7 +260,7 @@ void SingletonHolder<T>::createInstance() {
 
   {
     RWSpinLock::WriteHolder wh(&vault_.mutex_);
-    vault_.creation_order_.push_back(type_);
+    vault_.creation_order_.push_back(type());
   }
 }
 

diff --git a/folly/Singleton.h b/folly/Singleton.h
index 78fa83d2f36a1aaaf38508fc441dc08f73ebe29f..89bc28be9d87645831ba5a2b5c454613dbadf6fc 100644 (file)
--- a/folly/Singleton.h
+++ b/folly/Singleton.h
@@ -218,13 +218,19 @@ class TypeDescriptorHasher {
 // SingletonHolders.
 class SingletonHolderBase {
  public:
+  explicit SingletonHolderBase(TypeDescriptor typeDesc) : type_(typeDesc) {}
   virtual ~SingletonHolderBase() = default;
 
-  virtual TypeDescriptor type() = 0;
+  TypeDescriptor type() const {
+    return type_;
+  }
   virtual bool hasLiveInstance() = 0;
   virtual void createInstance() = 0;
   virtual bool creationStarted() = 0;
   virtual void destroyInstance() = 0;
+
+ private:
+  TypeDescriptor type_;
 };
 
 // An actual instance of a singleton, tracking the instance itself,
@@ -246,7 +252,6 @@ struct SingletonHolder : public SingletonHolderBase {
 
   void registerSingleton(CreateFunc c, TeardownFunc t);
   void registerSingletonMock(CreateFunc c, TeardownFunc t);
-  virtual TypeDescriptor type() override;
   virtual bool hasLiveInstance() override;
   virtual void createInstance() override;
   virtual bool creationStarted() override;
@@ -261,7 +266,6 @@ struct SingletonHolder : public SingletonHolderBase {
     Living,
   };
 
-  TypeDescriptor type_;
   SingletonVault& vault_;
 
   // mutex protects the entire entry during construction/destruction