diff --git a/.gitignore b/.gitignore index 712a7ea3a..5f7f9dd97 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,4 @@ autowiring-*-*.dmg autowiring-*-*.deb autowiring-*-*.rpm autowiring-*-*.msi +autowiring.xcodeproj \ No newline at end of file diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index 7cee21350..f49c1b7e2 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -7,6 +7,7 @@ #include "AutowiringEvents.h" #include "autowiring_error.h" #include "Bolt.h" +#include "CoreContextStateBlock.h" #include "CoreRunnable.h" #include "ContextMember.h" #include "CreationRules.h" @@ -25,12 +26,12 @@ #include "TypeUnifier.h" #include -#include TYPE_INDEX_HEADER #include MEMORY_HEADER +#include FUNCTIONAL_HEADER +#include TYPE_INDEX_HEADER #include STL_UNORDERED_MAP #include STL_UNORDERED_SET -struct CoreContextStateBlock; class AutoInjectable; class AutoPacketFactory; class DeferrableAutowiring; @@ -131,7 +132,7 @@ class CoreContext: // This is a list of concrete types, indexed by the true type of each element. std::vector m_concreteTypes; - // This is a memoization map used to memoize any already-detected interfaces. The map + // This is a memoization map used to memoize any already-detected interfaces. mutable std::unordered_map m_typeMemos; // All known context members, exception filters: @@ -368,6 +369,14 @@ class CoreContext: /// void FindByTypeUnsafe(AnySharedPointer& reference) const; + /// + /// Recursive locking for Autowire satisfaction search + /// + /// + /// The argument &&reference enables implicit type from AnySharedPointerT. + /// + void FindByTypeRecursiveUnsafe(AnySharedPointer&& reference, const std::function& terminal) const; + /// /// Returns or constructs a new AutoPacketFactory instance /// @@ -376,7 +385,7 @@ class CoreContext: /// /// Adds the specified deferrable autowiring as a general recipient of autowiring events /// - void AddDeferred(const AnySharedPointer& reference, DeferrableAutowiring* deferrable); + void AddDeferredUnsafe(const AnySharedPointer& reference, DeferrableAutowiring* deferrable); /// /// Adds a snooper to the snoopers set @@ -850,9 +859,9 @@ class CoreContext: /// template void FindByType(std::shared_ptr& slot) const { - AnySharedPointerT ptr; - FindByType(ptr); - slot = ptr.slot()->template as(); + AnySharedPointerT reference; + FindByType(reference); + slot = reference.slot()->template as(); } /// @@ -860,14 +869,14 @@ class CoreContext: /// template bool FindByTypeRecursive(std::shared_ptr& slot) { - // First-chance resolution in this context and ancestor contexts: - for(CoreContext* pCur = this; pCur; pCur = pCur->m_pParent.get()) { - pCur->FindByType(slot); - if(slot) - return true; + { + std::lock_guard guard(m_stateBlock->m_lock); + FindByTypeRecursiveUnsafe(AnySharedPointerT(), + [&slot](AnySharedPointer& reference){ + slot = reference.slot()->template as(); + }); } - - return false; + return static_cast(slot); } /// @@ -875,19 +884,25 @@ class CoreContext: /// template bool Autowire(AutowirableSlot& slot) { - if(FindByTypeRecursive(slot)) - return true; - - // Failed, defer - AddDeferred(AnySharedPointerT(), &slot); - return false; + { + std::lock_guard lk(m_stateBlock->m_lock); + FindByTypeRecursiveUnsafe(AnySharedPointerT(), + [this, &slot](AnySharedPointer& reference){ + slot = reference.slot()->template as(); + if (!slot) { + AddDeferredUnsafe(AnySharedPointerT(), &slot); + } + }); + } + return static_cast(slot); } /// /// Adds a post-attachment listener in this context for a particular autowired member /// /// - /// A pointer to a deferrable autowiring function which the caller may safely ignore if it's not needed + /// A pointer to a deferrable autowiring function which the caller may safely ignore if it's not needed. + /// Returns nullptr if the call was made immediately. /// /// /// This method will succeed if slot was constructed in this context or any parent context. If the @@ -906,12 +921,27 @@ class CoreContext: /// template const AutowirableSlotFn* NotifyWhenAutowired(Fn&& listener) { - auto retVal = MakeAutowirableSlotFn( - shared_from_this(), - std::forward(listener) - ); - - AddDeferred(AnySharedPointerT(), retVal); + bool found = false; + AutowirableSlotFn* retVal = nullptr; + { + std::lock_guard lk(m_stateBlock->m_lock); + FindByTypeRecursiveUnsafe(AnySharedPointerT(), + [this, &listener, &retVal, &found](AnySharedPointer& reference) { + if (reference) { + found = true; + } else { + retVal = MakeAutowirableSlotFn( + shared_from_this(), + std::forward(listener) + ); + AddDeferredUnsafe(reference, retVal); + } + }); + } + if (found) + // Make call outside of lock + // NOTE: existential guarantees of context enable this. + listener(); return retVal; } diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index 011de19c1..2e1dddc18 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -4,7 +4,6 @@ #include "AutoInjectable.h" #include "AutoPacketFactory.h" #include "BoltBase.h" -#include "CoreContextStateBlock.h" #include "CoreThread.h" #include "GlobalCoreContext.h" #include "JunctionBox.h" @@ -285,6 +284,26 @@ void CoreContext::FindByTypeUnsafe(AnySharedPointer& reference) const { m_typeMemos[type].m_value = reference; } +void CoreContext::FindByTypeRecursiveUnsafe(AnySharedPointer&& reference, const std::function& terminal) const { + FindByTypeUnsafe(reference); + if (reference) { + // Type satisfied in current context + terminal(reference); + return; + } + + if (m_pParent) { + std::lock_guard guard(m_pParent->m_stateBlock->m_lock); + // Recurse while holding lock on this context + // NOTE: Racing Deadlock is only possible if there is a simultaneous descending locked chain, + // but by definition of contexts this is forbidden. + m_pParent->FindByTypeRecursiveUnsafe(std::move(reference), terminal); + } else { + // Call function while holding all locks through global scope. + terminal(reference); + } +} + std::shared_ptr CoreContext::GetGlobal(void) { return std::static_pointer_cast(GlobalCoreContext::Get()); } @@ -304,6 +323,11 @@ std::vector> CoreContext::CopyBasicThreadList(void) } void CoreContext::Initiate(void) { + // First-pass check, used to prevent recursive deadlocks traceable to here that might + // result from entities trying to initiate subcontexts from CoreRunnable::Start + if(m_initiated || m_isShutdown) + return; + { std::lock_guard lk(m_stateBlock->m_lock); if(m_initiated) @@ -481,6 +505,9 @@ void CoreContext::BuildCurrentState(void) { } void CoreContext::CancelAutowiringNotification(DeferrableAutowiring* pDeferrable) { + if (!pDeferrable) + return; + std::lock_guard lk(m_stateBlock->m_lock); auto q = m_typeMemos.find(pDeferrable->GetType()); if(q == m_typeMemos.end()) @@ -764,10 +791,7 @@ std::shared_ptr CoreContext::GetPacketFactory(void) { return pf; } -void CoreContext::AddDeferred(const AnySharedPointer& reference, DeferrableAutowiring* deferrable) -{ - std::lock_guard lk(m_stateBlock->m_lock); - +void CoreContext::AddDeferredUnsafe(const AnySharedPointer& reference, DeferrableAutowiring* deferrable) { // Determine whether a type memo exists right now for the thing we're trying to defer. If it doesn't // exist, we need to inject one in order to allow deferred satisfaction to know what kind of type we // are trying to satisfy at this point. diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 087d34955..d8bbaecd1 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -11,6 +11,7 @@ set(AutowiringTest_SRCS BoltTest.cpp CoreContextTest.cpp CoreJobTest.cpp + CoreRunnableTest.cpp ContextCleanupTest.cpp ContextEnumeratorTest.cpp ContextMapTest.cpp diff --git a/src/autowiring/test/CoreRunnableTest.cpp b/src/autowiring/test/CoreRunnableTest.cpp new file mode 100644 index 000000000..dea9650cf --- /dev/null +++ b/src/autowiring/test/CoreRunnableTest.cpp @@ -0,0 +1,30 @@ +// Copyright (C) 2012-2014 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include +#include + +class CoreRunnableTest: + public testing::Test +{}; + +class StartsSubcontextWhileStarting: + public CoreRunnable +{ +public: + AutoCreateContext m_myContext; + + bool Start(std::shared_ptr outstanding) override { + m_myContext->Initiate(); + return true; + } + + void Stop(bool graceful) override {} + bool IsRunning(void) const override { return false; } + bool ShouldStop(void) const override { return true; } + void Wait(void) override {} +}; + +TEST_F(CoreRunnableTest, CanStartSubcontextWhileInitiating) { + AutoRequired(); + AutoCurrentContext()->Initiate(); +} \ No newline at end of file diff --git a/src/autowiring/test/PostConstructTest.cpp b/src/autowiring/test/PostConstructTest.cpp index d20a9692d..8691d1f0d 100644 --- a/src/autowiring/test/PostConstructTest.cpp +++ b/src/autowiring/test/PostConstructTest.cpp @@ -264,10 +264,9 @@ TEST_F(PostConstructTest, ContextNotifyWhenAutowired) { // Now we'd like to be notified when SimpleObject gets added: ctxt->NotifyWhenAutowired( - [called] { + [called] { *called = true; - } - ); + }); // Should only be two uses, at this point, of the capture of the above lambda: EXPECT_EQ(2L, called.use_count()) << "Unexpected number of references held in a capture lambda"; @@ -283,3 +282,75 @@ TEST_F(PostConstructTest, ContextNotifyWhenAutowired) { ASSERT_TRUE(called.unique()) << "Autowiring notification lambda was not properly cleaned up"; } +TEST_F(PostConstructTest, ContextNotifyWhenAutowiredPostConstruct) { + auto called = std::make_shared(false); + AutoCurrentContext ctxt; + + // Create an object that will satisfy subsequent notification call: + AutoRequired sobj; + + // Notification should be immediate: + ctxt->NotifyWhenAutowired( + [called] { + *called = true; + }); + + // Insert the SimpleObject, see if the lambda got hit: + ASSERT_TRUE(*called) << "Context-wide autowiring notification was not hit as expected when a matching type was injected into a context"; + + // Our shared pointer should be unique by this point, because the lambda should have been destroyed + ASSERT_TRUE(called.unique()) << "Autowiring notification lambda was not properly cleaned up"; +} + +class OtherObject : public SimpleObject {}; + +TEST_F(PostConstructTest, RecursiveNotificationPreConstruction) { + auto called = std::make_shared(false); + AutoCurrentContext ctxt; + + // Ensure that nested calls do not created deadlock + // Notification should be deferred: + ctxt->NotifyWhenAutowired( + [called] { + AutoCurrentContext()->NotifyWhenAutowired( + [called] { + *called = true; + }); + }); + + // Create an object that will satisfy subsequent notification call: + AutoRequired sobj; + AutoRequired oobj; + + // Insert the SimpleObject, see if the lambda got hit: + ASSERT_TRUE(*called) << "Context-wide autowiring notification was not hit as expected when a matching type was injected into a context"; + + // Our shared pointer should be unique by this point, because the lambda should have been destroyed + ASSERT_TRUE(called.unique()) << "Autowiring notification lambda was not properly cleaned up"; +} + +TEST_F(PostConstructTest, RecursiveNotificationPostConstruction) { + auto called = std::make_shared(false); + AutoCurrentContext ctxt; + + // Create an object that will satisfy subsequent notification call: + AutoRequired sobj; + AutoRequired oobj; + + // Ensure that nested calls do not created deadlock + // Notification should be immediate: + ctxt->NotifyWhenAutowired( + [called] { + AutoCurrentContext()->NotifyWhenAutowired( + [called] { + *called = true; + }); + }); + + // Insert the SimpleObject, see if the lambda got hit: + ASSERT_TRUE(*called) << "Context-wide autowiring notification was not hit as expected when a matching type was injected into a context"; + + // Our shared pointer should be unique by this point, because the lambda should have been destroyed + ASSERT_TRUE(called.unique()) << "Autowiring notification lambda was not properly cleaned up"; +} +