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

Correct recursive context initialization #382

Merged
merged 2 commits into from
Jan 30, 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
4 changes: 2 additions & 2 deletions autowiring/CoreContext.h
Original file line number Diff line number Diff line change
Expand Up @@ -453,9 +453,9 @@ class CoreContext:

/// \internal
/// <summary>
/// Invoked by a parent context on a child context when the parent has transitioned to the Running state
/// Invoked by a parent context when the parent has transitioned to the Running state
/// </summary>
void TryTransitionToRunState(void);
void TryTransitionChildrenToRunState(void);

/// <summary>
/// Registers a factory _function_, a lambda which is capable of constructing decltype(fn())
Expand Down
102 changes: 54 additions & 48 deletions src/autowiring/CoreContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -440,34 +440,8 @@ void CoreContext::Initiate(void) {
for (auto q = beginning; q != m_threads.end(); ++q)
(*q)->Start(outstanding);
}
lk.lock();

// We have to hold this to prevent dtors from running in a synchronized context
std::shared_ptr<CoreContext> prior;
for (auto childWeak : m_children) {
// Obtain child pointer and lock it down so our iterator stays stable
auto child = childWeak.lock();
if (!child)
// Expiration while we were attempting to dereference, circle around
continue;

// Cannot hold a lock safely if we hand off control to a child context
lk.unlock();

// Control handoff to the child context must happen outside of a lock:
child->TryTransitionToRunState();

// Permit a prior context to expire if needed
prior.reset();

// Need to preserve current child context pointer before it goes out of scope in order to
// preserve our iterator.
lk.lock();
prior = child;
}

// Release our lock before letting `prior` expire, we can't hold a lock through such an event
lk.unlock();

TryTransitionChildrenToRunState();
}

void CoreContext::InitiateCoreThreads(void) {
Expand Down Expand Up @@ -999,31 +973,63 @@ void CoreContext::AddPacketSubscriber(const AutoFilterDescriptor& rhs) {
Inject<AutoPacketFactory>()->AddSubscriber(rhs);
}

void CoreContext::TryTransitionToRunState(void) {
void CoreContext::TryTransitionChildrenToRunState(void) {
std::unique_lock<std::mutex> lk(m_stateBlock->m_lock);
switch (m_state) {
case State::Initiated:
// Can transition to the running state now

// We have to hold this to prevent dtors from running in a synchronized context
std::shared_ptr<CoreContext> prior;
for (auto childWeak : m_children) {
// Obtain child pointer and lock it down so our iterator stays stable
auto child = childWeak.lock();
if (!child)
// Expiration while we were attempting to dereference, circle around
continue;

// Cannot hold a lock safely if we hand off control to a child context
lk.unlock();
{
auto q = m_threads.begin();
m_state = State::Running;
lk.unlock();

auto outstanding = IncrementOutstandingThreadCount();
while (q != m_threads.end()) {
(*q)->Start(outstanding);
q++;
// Get lock of child while we're modifying it's state
std::unique_lock<std::mutex> childLk(child->m_stateBlock->m_lock);

switch (child->m_state) {
case State::Initiated:
// Can transition to the running state now
{
auto q = child->m_threads.begin();
child->m_state = State::Running;
childLk.unlock();

auto outstanding = child->IncrementOutstandingThreadCount();
while (q != child->m_threads.end()) {
(*q)->Start(outstanding);
q++;
}
}
break;
case State::CanRun:
case State::NotStarted:
case State::Running:
case State::Shutdown:
case State::Abandoned:
// No action need be taken for these states
return;
}
}
break;
case State::CanRun:
case State::NotStarted:
case State::Running:
case State::Shutdown:
case State::Abandoned:
// No action need be taken for these states
return;

// Recursivly try to transition children
child->TryTransitionChildrenToRunState();

// Permit a prior context to expire if needed
prior.reset();

// Need to preserve current child context pointer before it goes out of scope in order to
// preserve our iterator.
lk.lock();
prior = child;
}

// Release our lock before letting `prior` expire, we can't hold a lock through such an event
lk.unlock();
}

void CoreContext::UnsnoopAutoPacket(const ObjectTraits& traits) {
Expand Down
40 changes: 40 additions & 0 deletions src/autowiring/test/CoreContextTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -232,4 +232,44 @@ TEST_F(CoreContextTest, InitiateCausesDelayedHold) {
// Starting up the outer context should cause the inner one to self destruct
AutoCurrentContext()->Initiate();
ASSERT_TRUE(ctxtWeak.expired()) << "Subcontext containing no threads incorrectly persisted after termination";
}

TEST_F(CoreContextTest, InitiateOrder) {
AutoCurrentContext testCtxt;
testCtxt->Initiate();
{
auto outerCtxt = testCtxt->Create<void>();
auto middleCtxt = outerCtxt->Create<void>();
auto innerCtxt = middleCtxt->Create<void>();

middleCtxt->Initiate();
innerCtxt->Initiate();
outerCtxt->Initiate();

EXPECT_TRUE(outerCtxt->IsRunning());
EXPECT_TRUE(middleCtxt->IsRunning());
EXPECT_TRUE(innerCtxt->IsRunning());

outerCtxt->SignalShutdown(true);
middleCtxt->SignalShutdown(true);
innerCtxt->SignalShutdown(true);
}

{
auto outerCtxt = testCtxt->Create<void>();
auto middleCtxt = outerCtxt->Create<void>();
auto innerCtxt = middleCtxt->Create<void>();

innerCtxt->Initiate();
middleCtxt->Initiate();
outerCtxt->Initiate();

EXPECT_TRUE(outerCtxt->IsRunning());
EXPECT_TRUE(middleCtxt->IsRunning());
EXPECT_TRUE(innerCtxt->IsRunning());

innerCtxt->SignalShutdown(true);
middleCtxt->SignalShutdown(true);
outerCtxt->SignalShutdown(true);
}
}