Avoid missing type satisfaction due to simultaneous injection in to a…

… parent context: FindByTypeRecursiveUnsafe makes calls while holding ALL necessary locks.
leapmotion · Aug 22, 2014 · b0ac22b · b0ac22b
1 parent 530dd13
commit b0ac22b
Show file tree

Hide file tree

Showing 2 changed files with 38 additions and 26 deletions.
diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h
@@ -25,8 +25,9 @@
 #include "TypeUnifier.h"
 
 #include <list>
-#include TYPE_INDEX_HEADER
 #include MEMORY_HEADER
+#include FUNCTIONAL_HEADER
+#include TYPE_INDEX_HEADER
 #include STL_UNORDERED_MAP
 #include STL_UNORDERED_SET
 
@@ -370,7 +371,10 @@ class CoreContext:
   /// <summary>
   /// Recursive locking for Autowire satisfaction search
   /// </summary>
-  void FindByTypeRecursiveUnsafe(AnySharedPointer& reference) const;
+  /// <remarks>
+  /// The argument &&reference enables implicit type from AnySharedPointerT<T>.
+  /// </remarks>
+  void FindByTypeRecursiveUnsafe(AnySharedPointer&& reference, const std::function<void(AnySharedPointer&)>& terminal) const;
 
   /// <summary>
   /// Returns or constructs a new AutoPacketFactory instance
@@ -861,12 +865,13 @@ class CoreContext:
   /// </summary>
   template<class T>
   bool FindByTypeRecursive(std::shared_ptr<T>& slot) {
-    AnySharedPointerT<T> reference;
     {
       std::lock_guard<std::mutex> guard(m_stateBlock->m_lock);
-      FindByTypeRecursiveUnsafe(reference);
+      FindByTypeRecursiveUnsafe(AnySharedPointerT<T>(),
+      [&slot](AnySharedPointer& reference){
+        slot = reference.slot()->template as<T>();
+      });
     }
-    slot = reference.slot()->template as<T>();
     return static_cast<bool>(slot);
   }
 
@@ -875,15 +880,16 @@ class CoreContext:
   /// </summary>
   template<class T>
   bool Autowire(AutowirableSlot<T>& slot) {
-    AnySharedPointerT<T> reference;
     {
       std::lock_guard<std::mutex> lk(m_stateBlock->m_lock);
-      FindByTypeRecursiveUnsafe(reference);
-      if (!slot) {
-        AddDeferredUnsafe(AnySharedPointerT<T>(), &slot);
-      }
+      FindByTypeRecursiveUnsafe(AnySharedPointerT<T>(),
+      [this, &slot](AnySharedPointer& reference){
+        slot = reference.slot()->template as<T>();
+        if (!slot) {
+          AddDeferredUnsafe(AnySharedPointerT<T>(), &slot);
+        }
+      });
     }
-    slot = reference.slot()->template as<T>();
     return static_cast<bool>(slot);
   }
 
@@ -912,19 +918,20 @@ class CoreContext:
   template<class T, class Fn>
   const AutowirableSlotFn<T, Fn>* NotifyWhenAutowired(Fn&& listener) {
     AutowirableSlotFn<T, Fn>* retVal = nullptr;
-    AnySharedPointerT<T> asp;
     {
       std::lock_guard<std::mutex> lk(m_stateBlock->m_lock);
-      FindByTypeRecursiveUnsafe(asp);
-      if (asp) {
-        listener();
-      } else {
-        retVal = MakeAutowirableSlotFn<T>(
-          shared_from_this(),
-          std::forward<Fn>(listener)
-        );
-        AddDeferredUnsafe(asp, retVal);
-      }
+      FindByTypeRecursiveUnsafe(AnySharedPointerT<T>(),
+      [this, &listener, &retVal](AnySharedPointer& reference) {
+        if (reference) {
+          listener();
+        } else {
+          retVal = MakeAutowirableSlotFn<T>(
+            shared_from_this(),
+            std::forward<Fn>(listener)
+          );
+          AddDeferredUnsafe(reference, retVal);
+        }
+      });
     }
     return retVal;
   }

diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp
@@ -284,18 +284,23 @@ void CoreContext::FindByTypeUnsafe(AnySharedPointer& reference) const {
   m_typeMemos[type].m_value = reference;
 }
 
-void CoreContext::FindByTypeRecursiveUnsafe(AnySharedPointer& reference) const {
+void CoreContext::FindByTypeRecursiveUnsafe(AnySharedPointer&& reference, const std::function<void(AnySharedPointer&)>& terminal) const {
   FindByTypeUnsafe(reference);
-  if (reference)
+  if (reference) {
     // Type satisfied in current context
+    terminal(reference);
     return;
+  }
 
   if (m_pParent) {
+    std::lock_guard<std::mutex> guard(m_pParent->m_stateBlock->m_lock);
     // Recurse while holding lock on this context
     // NOTE: Deadlock is only possible if there is a simultaneous descending locked chain,
     // but by definition of contexts this is forbidden.
-    std::lock_guard<std::mutex> guard(m_pParent->m_stateBlock->m_lock);
-    m_pParent->FindByTypeRecursiveUnsafe(reference);
+    m_pParent->FindByTypeRecursiveUnsafe(std::move(reference), terminal);
+  } else {
+    // Call function while holding all locks through global scope.
+    terminal(reference);
   }
 }