Fix issue with unlink teardown behavior

autowiring/Autowired.h

@@ -140,6 +140,23 @@ class Autowired:
     return this->operator const std::shared_ptr<T>&().get();
   }
 
+  template<typename Fn, typename... Args>
+  struct wrapper {
+    wrapper(Autowired<T>* field, Fn&& fn) :
+      field(field),
+      fn(std::forward<Fn&&>(fn))
+    {}
+
+    Autowired<T>* field;
+    Fn fn;
+
+    void operator()(const Args&... args) {
+      if (field->IsAutowired()) {
+        fn(args...);
+      }
+    }
+  };
+
   //A small temporary structure for the info required to register an event
   //with a member signal on an autowired field.
   template<typename U, typename... Args>
@@ -155,10 +172,11 @@ class Autowired:
     void operator+=(Fn&& rhs) {
       Autowired<T>* l_field = field; //lambda capture of references doesn't work right.
       autowiring::signal<void(Args...)> U::* l_member = member;
+      auto wrapped = wrapper<Fn, Args...>(l_field, std::forward<Fn&&>(rhs));
 
-      field->NotifyWhenAutowired([l_field, l_member, rhs] {
+      field->NotifyWhenAutowired([l_field, l_member, wrapped] {
         auto& sig = static_cast<U*>(l_field->get())->*l_member;
-        l_field->m_events.push_back(sig += rhs);
+        l_field->m_events.push_back(sig += wrapped);
       });
     }
   };
@@ -173,8 +191,9 @@ class Autowired:
   void reset(void) override {
     // And remove any events that were added to the object by this autowired field
     auto localEvents = std::move(m_events);
-    for (auto& localEvent : localEvents)
-      *localEvent.owner -= localEvent;
+    for (auto& localEvent : localEvents) {
+      *localEvent.owner-=localEvent;
+    }
 
     // Linked list unwind deletion:
     for (

autowiring/auto_signal.h

@@ -214,7 +214,7 @@ namespace autowiring {
 
       template<typename _Fn>
       entry(signal&, _Fn fn) : fn(std::forward<_Fn>(fn)) {}
-      const Fn fn;
+      Fn fn;
       void operator()(const Args&... args) override { fn(args...); }
     };
 
@@ -479,10 +479,12 @@ namespace autowiring {
         case SignalState::Free:
         case SignalState::Updating:
           // Spurious failure, or insertion.
-          // We cannot delegate control to insertion, and spurious failure shoudl be retried.
+          // We cannot delegate control to insertion, and spurious failure should be retried.
           continue;
-        default:
-          // Asserting or deferred for awhile, we need to take another option
+          case SignalState::Asserting:
+          case SignalState::Deferred:
+          // Some other thread is already asserting this signal, doing work, we need to ensure
+          // that thread takes responsibility for this call.
           break;
         }
 
@@ -494,22 +496,27 @@ namespace autowiring {
           }
         );
 
-        // Now ensure that someone is going to take responsibility to make this invocation
+        // Now ensure that someone is going to take responsibility to make this invocation.  We do
+        // this by trying to transition the Asserting state to Deferred before the currently
+        // asserting thread manages to transition from Asserting to Free.  This is an implicit
+        // race condition, whoever wins the race gets to return control to the caller.
         do {
           state = SignalState::Asserting;
           if (m_state.compare_exchange_weak(state, SignalState::Deferred))
             // Success, we have transferred responsibility for this call to the other entity.
             return;
+
           if (state == SignalState::Deferred)
-            // Failure, egg came back on our face.  Responsibility was reflected back on to us.
-            // Take charge.
-            break;
+            // State is already deferred, maybe by someone else.  We can return control directly.
+            return;
+
           state = SignalState::Free;
         } while (!m_state.compare_exchange_weak(state, SignalState::Asserting));
 
-        // Great.  If we got here it's because everyone got away and we're left holding the bag.
-        // We thought we were going to need to pend a callable signal but instead we're still going
-        // to need to invoke it ourselves.
+        // Great.  If we got here it's because we were able to transition from Free to Asserting, which
+        // means that everyone else has already returned to their callers.  We are the only ones still
+        // in operator().  We are going to have to call the callable_signal that we just got finished
+        // pending.
         break;
       }
 

src/autowiring/test/AutoSignalTest.cpp

@@ -616,3 +616,79 @@ TEST_F(AutoSignalTest, PathologicalSyncTest) {
     ASSERT_FALSE(*x) << "Lambda was invoked after it was destroyed";
   }
 }
+
+namespace {
+  class OuterType {
+  public:
+    autowiring::signal<void()> sig;
+  };
+
+  class WiresInOuterScope {
+  public:
+    WiresInOuterScope(void) {
+      outer(&OuterType::sig) += [this] {
+        ASSERT_EQ(0xDEADBEEFC0FECAFE, magic);
+      };
+    }
+
+    ~WiresInOuterScope(void) {
+      magic = 0xEEEE0000FFFF0000;
+    }
+
+    Autowired<OuterType> outer;
+    uint64_t magic = 0xDEADBEEFC0FECAFE;
+  };
+}
+
+TEST_F(AutoSignalTest, OuterPostDereference) {
+  AutoRequired<OuterType> outer;
+
+  AutoCreateContext ctxt;
+  outer->sig += [&] {
+    // This will cause the outer context to be destroyed, which will cause one
+    // of the signal handlers to be reset.  Unfortunatley, however, we aren't yet
+    // done hitting all signal handlers, so this statement will cause the very next
+    // invoked signal--registered in WiresInOuterScope's ctor--to possibly be
+    // called even though its context is gone.
+    ctxt.reset();
+  };
+
+  ctxt->Inject<WiresInOuterScope>();
+
+  // This should trigger an exception:
+  outer->sig();
+}
+
+namespace {
+  class WithSignal {
+  public:
+    autowiring::signal<void()> sig;
+  };
+}
+
+TEST_F(AutoSignalTest, ReallocationCheck) {
+  AutoCurrentContext ctxt;
+  size_t hitCount = 0;
+
+  std::unique_ptr<Autowired<WithSignal>> contrived{
+    new Autowired<WithSignal>{}
+  };
+
+  {
+    // Delete the Autowired field while the signal handler is being called.  This is intended to simulate
+    // a legal possible multithreaded scenario where a signal handler is being asserted at about the same
+    // time as an Autowired field is being destroyed.
+    (*contrived)(&WithSignal::sig) += [&] {
+      hitCount++;
+      contrived.reset();
+
+      Autowired<WithSignal> ws;
+      ws->sig();
+    };
+    ctxt->Inject<WithSignal>();
+  }
+  Autowired<WithSignal> sig3;
+  sig3->sig();
+
+  ASSERT_EQ(1UL, hitCount) << "A signal handler was hit even though the base Autowired field was destroyed";
+}