Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix autorequired parentinject #464

Merged
merged 4 commits into from
Mar 25, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 11 additions & 7 deletions autowiring/CoreContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,8 @@ class CoreContext:
/// </summary>
struct MemoEntry {
MemoEntry(void) :
pFirst(nullptr)
pFirst(nullptr),
m_local(true)
{}

// The first deferrable autowiring which requires this type, if one exists:
Expand All @@ -174,6 +175,9 @@ class CoreContext:
// Once this memo entry is satisfied, this will contain the AnySharedPointer instance that performs
// the satisfaction
AnySharedPointer m_value;

// A flag to determine if this memo entry was set from the current context.
bool m_local;
};

protected:
Expand Down Expand Up @@ -345,7 +349,7 @@ class CoreContext:
/// <summary>
/// Updates all deferred autowiring fields, generally called after a new member has been added
/// </summary>
void UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry);
void UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry, bool local);

/// \internal
/// <summary>
Expand Down Expand Up @@ -432,13 +436,13 @@ class CoreContext:
/// The memo entry where this type was found
/// </returns>
/// <param name="reference">An initialized shared pointer slot which may be used in type detection</param>
void FindByType(AnySharedPointer& reference) const;
void FindByType(AnySharedPointer& reference, bool localOnly = false) const;

/// \internal
/// <summary>
/// Unsynchronized version of FindByType
/// </summary>
MemoEntry& FindByTypeUnsafe(AnySharedPointer& reference) const;
MemoEntry& FindByTypeUnsafe(AnySharedPointer& reference, bool localOnly = false) const;

/// \internal
/// <summary>
Expand Down Expand Up @@ -695,7 +699,7 @@ class CoreContext:
// member does not inherit CoreObject and this member is eventually satisfied by one that does.
{
std::shared_ptr<T> pure;
FindByType(pure);
FindByType(pure, true); //skip non-local slots.
if (pure)
return pure;
}
Expand Down Expand Up @@ -1047,9 +1051,9 @@ class CoreContext:
/// Locates an available context member in this context
/// </summary>
template<class T>
void FindByType(std::shared_ptr<T>& slot) const {
void FindByType(std::shared_ptr<T>& slot, bool localOnly = false) const {
AnySharedPointerT<T> reference;
FindByType(reference);
FindByType(reference, localOnly);
slot = reference.slot()->template as<T>();
}

Expand Down
41 changes: 27 additions & 14 deletions src/autowiring/CoreContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,14 +245,21 @@ void CoreContext::AddInternal(const CoreObjectDescriptor& traits) {
// the value, rather than the type passed in via traits.type, because the proper type might be a
// concrete type defined in another context or potentially a unifier type. Creating a slot here
// is also undesirable because the complete type is not available and we can't create a dynaimc
// caster to identify when this slot gets satisfied.
// caster to identify when this slot gets satisfied. If a slot was non-local, overwrite it.
auto q = m_typeMemos.find(*traits.actual_type);
if(q != m_typeMemos.end()) {
auto& v = q->second;
if(traits.pCoreObject && *v.m_value == traits.pCoreObject)
throw std::runtime_error("An attempt was made to add the same value to the same context more than once");
if(*v.m_value)
throw std::runtime_error("An attempt was made to add the same type to the same context more than once");

if (v.m_local) {
if (traits.pCoreObject && *v.m_value == traits.pCoreObject)
throw std::runtime_error("An attempt was made to add the same value to the same context more than once");
if (*v.m_value)
throw std::runtime_error("An attempt was made to add the same type to the same context more than once");
}
else {
v.m_value = traits.value;
v.m_local = true;
}
}

// Add the new concrete type:
Expand All @@ -270,7 +277,7 @@ void CoreContext::AddInternal(const CoreObjectDescriptor& traits) {

// Notify any autowiring field that is currently waiting that we have a new member
// to be considered.
UpdateDeferredElements(std::move(lk), m_concreteTypes.back());
UpdateDeferredElements(std::move(lk), m_concreteTypes.back(), true);
}

// Moving this outside the lock because AddCoreRunnable will perform the checks inside its function
Expand Down Expand Up @@ -320,19 +327,21 @@ void CoreContext::AddInternal(const AnySharedPointer& ptr) {
UpdateDeferredElement(std::move(lk), entry);
}

void CoreContext::FindByType(AnySharedPointer& reference) const {
void CoreContext::FindByType(AnySharedPointer& reference, bool localOnly) const {
std::lock_guard<std::mutex> lk(m_stateBlock->m_lock);
FindByTypeUnsafe(reference);
FindByTypeUnsafe(reference, localOnly);
}

CoreContext::MemoEntry& CoreContext::FindByTypeUnsafe(AnySharedPointer& reference) const {
CoreContext::MemoEntry& CoreContext::FindByTypeUnsafe(AnySharedPointer& reference, bool localOnly) const {
const std::type_info& type = reference->type();

// If we've attempted to search for this type before, we will return the value of the memo immediately:
auto q = m_typeMemos.find(type);
if(q != m_typeMemos.end()) {
// We can copy over and return here
reference = q->second.m_value;
if (!localOnly || q->second.m_local){
reference = q->second.m_value;
}
return q->second;
}

Expand Down Expand Up @@ -801,7 +810,7 @@ void CoreContext::UpdateDeferredElement(std::unique_lock<std::mutex>&& lk, MemoE
lk.unlock();
}

void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry) {
void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, const CoreObjectDescriptor& entry, bool local) {
{
// Collection of items needing finalization:
std::vector<DeferrableUnsynchronizedStrategy*> delayedFinalize;
Expand All @@ -823,8 +832,8 @@ void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, cons
for (auto& cur : m_typeMemos) {
MemoEntry& value = cur.second;

if (value.m_value)
// This entry is already satisfied, no need to process it
if (value.m_value && value.m_local)
// This entry is already satisfied locally, no need to process it
continue;

// Determine whether the current candidate element satisfies the autowiring we are considering.
Expand All @@ -836,6 +845,9 @@ void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, cons
// Success, assign the traits
value.pObjTraits = &entry;

// Store if it was injected from the local context or not
value.m_local = local;

// Now we need to take on the responsibility of satisfying this deferral. We will do this by
// nullifying the flink, and by ensuring that the memo is satisfied at the point where we
// release the lock.
Expand Down Expand Up @@ -882,7 +894,8 @@ void CoreContext::UpdateDeferredElements(std::unique_lock<std::mutex>&& lk, cons
lk.unlock();
ctxt->UpdateDeferredElements(
std::unique_lock<std::mutex>(ctxt->m_stateBlock->m_lock),
entry
entry,
false
);
lk.lock();
}
Expand Down
106 changes: 106 additions & 0 deletions src/autowiring/test/ScopeTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -170,3 +170,109 @@ TEST_F(ScopeTest, AutowiringOrdering) {
}
}

class InnerContext {};
class MiddleContext {};
TEST_F(ScopeTest, RequireVsWire) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests are awesome. +1 +1

AutoCurrentContext ctxt_outer;
auto ctxt_middle = ctxt_outer->Create<MiddleContext>();
auto ctxt_inner = ctxt_middle->Create<InnerContext>();

CurrentContextPusher pshr(ctxt_inner);

Autowired<A> a_wired_inner;
ASSERT_FALSE(a_wired_inner.IsAutowired()) << "Autowired member became autowired too quickly";
ASSERT_EQ(a_wired_inner.GetContext(), ctxt_inner) << "Autowired member created in the wrong context";

AutoRequired<A> a_required_outer(ctxt_outer);
ASSERT_TRUE(a_required_outer.IsAutowired()) << "AutoRequired member unsatisfied after construction";
ASSERT_EQ(a_required_outer->GetContext(), ctxt_outer) << "AutoRequired member satisfied in the wrong context";

ASSERT_TRUE(a_wired_inner.IsAutowired()) << "AutoRequired member in parent did not satisfy Autowired member in child";
ASSERT_EQ(a_wired_inner.get(), a_required_outer.get()) << "Mismatch between Autowired and Autorequired members";

//this overrides the slot in the middle context;
AutoRequired<A> a_required_middle(ctxt_middle);
ASSERT_TRUE(a_required_middle.IsAutowired()) << "AutoRequired member not satisfied!";
ASSERT_EQ(a_required_middle->GetContext(), ctxt_middle) << "AutoRequired member not created in child context";
ASSERT_NE(a_required_middle, a_required_outer) << "AutoRequired member not constructed in child context";

Autowired<A> a_wired_inner2(ctxt_inner);
ASSERT_TRUE(a_wired_inner2.IsAutowired());
ASSERT_EQ(a_wired_inner2.get(), a_required_middle.get()) << "Autowired member did not redirect to the closest context";
ASSERT_EQ(a_wired_inner.get(), a_required_outer.get()) << "Autowired object changed after satisfaction!";

AutoRequired<B> b_required_middle(ctxt_middle);
AutoRequired<B> b_required_outer(ctxt_outer);
Autowired<B> b_wired_inner;
ASSERT_TRUE(b_wired_inner.IsAutowired()) << "Autowired failed to find member";
ASSERT_EQ(b_wired_inner.get(), b_required_middle.get()) << "Autorequiring overwrote a slot it shouldn't have!";
}

TEST_F(ScopeTest, RequireVsWireFast) {
AutoCurrentContext ctxt_outer;
auto ctxt_middle = ctxt_outer->Create<MiddleContext>();
auto ctxt_inner = ctxt_middle->Create<InnerContext>();

CurrentContextPusher pshr(ctxt_inner);

//This adds a memo for this type to the inner context
AutowiredFast<A> a_wired(ctxt_inner);
ASSERT_FALSE(a_wired.IsAutowired()) << "Autowired member became autowired too quickly";

//This satisfies the memo in the inner context
AutoRequired<A> a_required_outer(ctxt_outer);
ASSERT_TRUE(a_required_outer.IsAutowired()) << "AutoRequired member unsatisfied after construction";
ASSERT_EQ(a_required_outer->GetContext(), ctxt_outer) << "AutoRequired member satisfied in the wrong context";

ASSERT_FALSE(a_wired.IsAutowired()) << "AutowiredFast was satisfied after construction!";

//This overrides the memo in the middle context.
AutoRequired<A> a_required_middle(ctxt_middle);
ASSERT_TRUE(a_required_middle.IsAutowired()) << "AutoRequired member not satisfied!";
ASSERT_EQ(a_required_middle->GetContext(), ctxt_middle) << "AutoRequired member not created in child context";
ASSERT_NE(a_required_middle.get(), a_required_outer.get()) << "AutoRequired member not constructed in child context";

//This should direct to the middle context
AutowiredFast<A> a_wired2(ctxt_inner);
ASSERT_TRUE(a_wired2.IsAutowired());
ASSERT_EQ(a_wired2, a_required_middle) << "Autowired member did not grab the most derived object";
}

class ParentInjector : public ContextMember {
public:
ParentInjector(int v, bool inject = false) : ContextMember("Parent Injector"), value(v) {
if (!inject) {
return;
}

const auto ctxt = m_context.lock();
if (!ctxt) {
return;
}

auto parent = ctxt->GetParentContext();
if (!parent) {
return;
}

parent->Inject<ParentInjector>(9000, false);
}
int value;
};

TEST_F(ScopeTest, RecursiveInject) {
AutoCurrentContext ctxt_outer;
auto ctxt_inner = ctxt_outer->Create<InnerContext>();

CurrentContextPusher pshr(ctxt_inner);

AutoRequired<ParentInjector> a_inner(ctxt_inner, 1, true);

ASSERT_EQ(a_inner->GetContext(), ctxt_inner);
ASSERT_EQ(a_inner->value, 1);

AutoRequired<ParentInjector> a_inner_2(ctxt_inner, 2);

ASSERT_EQ(a_inner, a_inner_2);
ASSERT_EQ(a_inner->GetContext(), a_inner_2->GetContext());
}