Refactor Stoppable:

The Stoppable interface aids in the enforcement of invariants needed to
successful start and stop a multi-threaded application composed of classes
that depend on each other in complex ways.
* Test written to confirm the current behavior.
* Comments updated to reflect the current behavior.
* Public API reduced to what is currently in use.
* Protected data members made private.
* volatile bool members changed to std::atomic<bool>.
* std::atomic<int> members changed to std::atomic<bool>.
* Name storage uses std::string
This commit is contained in:
Howard Hinnant
2014-10-29 14:23:41 -04:00
committed by Vinnie Falco
parent 68fe1a7c8f
commit eb64a4387d
4 changed files with 481 additions and 31 deletions

View File

@@ -62,9 +62,10 @@ class RootStoppable;
3. onPrepare()
This override is called for all Stoppable objects in the hierarchy
during the prepare stage. Objects are called from the bottom up.
It is guaranteed that all child Stoppable objects have already been
prepared when this is called.
during the prepare stage. It is guaranteed that all child Stoppable
objects have already been prepared when this is called.
Objects are called children first.
4. start()
@@ -77,9 +78,10 @@ class RootStoppable;
5. onStart()
This override is called for all Stoppable objects in the hierarchy
during the start stage. Objects are called from the bottom up.
It is guaranteed that all child Stoppable objects have already been
started when this is called.
during the start stage. It is guaranteed that no child Stoppable
objects have been started when this is called.
Objects are called parent first.
This is the sequence of events involved in stopping:
@@ -103,6 +105,8 @@ class RootStoppable;
timers, signal that threads should exit, queue cleanup jobs, and perform
any other necessary final actions in preparation for exit.
Objects are called parent first.
9. onChildrenStopped()
This override is called when all the children have stopped. This informs
@@ -110,6 +114,8 @@ class RootStoppable;
into its member functions. A Stoppable that has no children will still
have this function called.
Objects are called children first.
10. stopped()
The derived class calls this function to inform the Stoppable API that
@@ -183,9 +189,11 @@ public:
/** Returns `true` if all children have stopped. */
bool areChildrenStopped () const;
protected:
/** Called by derived classes to indicate that the stoppable has stopped. */
void stopped ();
private:
/** Override called during preparation.
Since all other Stoppable objects in the tree have already been
constructed, this provides an opportunity to perform initialization which
@@ -242,7 +250,6 @@ public:
*/
virtual void onChildrenStopped ();
private:
friend class RootStoppable;
struct Child;
@@ -262,13 +269,12 @@ private:
void stopAsyncRecursive ();
void stopRecursive (Journal journal);
protected:
char const* m_name;
std::string m_name;
RootStoppable& m_root;
Child m_child;
std::atomic <int> m_started;
bool volatile m_stopped;
bool volatile m_childrenStopped;
std::atomic<bool> m_started;
std::atomic<bool> m_stopped;
std::atomic<bool> m_childrenStopped;
Children m_children;
WaitableEvent m_stoppedEvent;
};
@@ -309,7 +315,7 @@ public:
Safe to call from any thread not associated with a Stoppable.
*/
void stop (Journal journal = Journal());
private:
/** Notify a root stoppable and children to stop, without waiting.
Has no effect if the stoppable was already notified.
@@ -318,10 +324,9 @@ public:
*/
void stopAsync ();
private:
std::atomic <int> m_prepared;
std::atomic <int> m_calledStop;
std::atomic <int> m_calledStopAsync;
std::atomic<bool> m_prepared;
std::atomic<bool> m_calledStop;
std::atomic<bool> m_calledStopAsync;
};
/** @} */

View File

@@ -24,6 +24,7 @@
#include <beast/threads/impl/RecursiveMutex.cpp>
#include <beast/threads/impl/ServiceQueue.cpp>
#include <beast/threads/impl/Stoppable.cpp>
#include <beast/threads/impl/Stoppable.test.cpp>
#include <beast/threads/impl/Thread.cpp>
#include <beast/threads/impl/WaitableEvent.cpp>

View File

@@ -25,7 +25,7 @@ Stoppable::Stoppable (char const* name, RootStoppable& root)
: m_name (name)
, m_root (root)
, m_child (this)
, m_started (0)
, m_started (false)
, m_stopped (false)
, m_childrenStopped (false)
{
@@ -35,7 +35,7 @@ Stoppable::Stoppable (char const* name, Stoppable& parent)
: m_name (name)
, m_root (parent.m_root)
, m_child (this)
, m_started (0)
, m_started (false)
, m_stopped (false)
, m_childrenStopped (false)
{
@@ -48,7 +48,7 @@ Stoppable::Stoppable (char const* name, Stoppable& parent)
Stoppable::~Stoppable ()
{
// Children must be stopped.
bassert (m_started.load () == 0 || m_childrenStopped);
bassert (!m_started || m_childrenStopped);
}
bool Stoppable::isStopping() const
@@ -124,7 +124,6 @@ void Stoppable::stopRecursive (Journal journal)
// if we get here then all children have stopped
//
memoryBarrier ();
m_childrenStopped = true;
onChildrenStopped ();
@@ -145,39 +144,39 @@ void Stoppable::stopRecursive (Journal journal)
RootStoppable::RootStoppable (char const* name)
: Stoppable (name, *this)
, m_prepared (0)
, m_calledStop (0)
, m_calledStopAsync (0)
, m_prepared (false)
, m_calledStop (false)
, m_calledStopAsync (false)
{
}
bool RootStoppable::isStopping() const
{
return m_calledStopAsync.load () != 0;
return m_calledStopAsync;
}
void RootStoppable::prepare ()
{
if (m_prepared.exchange (1) == 0)
if (m_prepared.exchange (true) == false)
prepareRecursive ();
}
void RootStoppable::start ()
{
// Courtesy call to prepare.
if (m_prepared.exchange (1) == 0)
if (m_prepared.exchange (true) == false)
prepareRecursive ();
if (m_started.exchange (1) == 0)
if (m_started.exchange (true) == false)
startRecursive ();
}
void RootStoppable::stop (Journal journal)
{
// Must have a prior call to start()
bassert (m_started.load () != 0);
bassert (m_started);
if (m_calledStop.exchange (1) == 1)
if (m_calledStop.exchange (true) == true)
{
journal.warning << "Stoppable::stop called again";
return;
@@ -189,7 +188,7 @@ void RootStoppable::stop (Journal journal)
void RootStoppable::stopAsync ()
{
if (m_calledStopAsync.exchange (1) == 0)
if (m_calledStopAsync.exchange (true) == false)
stopAsyncRecursive ();
}

View File

@@ -0,0 +1,445 @@
//------------------------------------------------------------------------------
/*
This file is part of Beast: https://github.com/vinniefalco/Beast
Copyright 2012, Vinnie Falco <vinnie.falco@gmail.com>
Permission to use, copy, modify, and/or distribute this software for any
purpose with or without fee is hereby granted, provided that the above
copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES
WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR
ANY SPECIAL , DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES
WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
*/
//==============================================================================
#include <beast/threads/Stoppable.h>
namespace beast {
class Stoppable_test
: public unit_test::suite
{
/*
R
/ | \
/ | \
A B C
/ | \ /\ |
D E F G H I
|
J
*/
unsigned count = 0;
class D
: public Stoppable
{
Stoppable_test& test_;
public:
D(Stoppable& parent, Stoppable_test& test)
: Stoppable("D", parent)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 9, "D::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 0, "D::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 11, "D::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 2, "D::onChildrenStopped called out of order");
}
};
class J
: public Stoppable
{
Stoppable_test& test_;
public:
J(Stoppable& parent, Stoppable_test& test)
: Stoppable("J", parent)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 7, "J::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 1, "J::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 10, "J::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 4, "J::onChildrenStopped called out of order");
}
};
class E
: public Stoppable
{
J j_;
Stoppable_test& test_;
public:
E(Stoppable& parent, Stoppable_test& test)
: Stoppable("E", parent)
, j_(*this, test)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 8, "E::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 2, "E::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 9, "E::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 3, "E::onChildrenStopped called out of order");
}
};
class F
: public Stoppable
{
Stoppable_test& test_;
public:
F(Stoppable& parent, Stoppable_test& test)
: Stoppable("F", parent)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 6, "F::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 3, "F::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 8, "F::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 5, "F::onChildrenStopped called out of order");
}
};
class A
: public Stoppable
{
enum {running, please_stop, have_stopped};
D d_;
E e_;
F f_;
Stoppable_test& test_;
std::atomic<int> stop_;
public:
A(Stoppable& parent, Stoppable_test& test)
: Stoppable("A", parent)
, d_(*this, test)
, e_(*this, test)
, f_(*this, test)
, test_(test)
, stop_(running)
{}
void run()
{
while (stop_ == running)
;
stop_ = have_stopped;
}
void onPrepare() override
{
test_.expect(++test_.count == 10, "A::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 4, "A::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 7, "A::onStop called out of order");
}
void onChildrenStopped() override
{
stop_ = please_stop;
while (stop_ != have_stopped)
;
Stoppable::stopped();
test_.expect(--test_.count == 1, "A::onChildrenStopped called out of order");
}
};
class G
: public Stoppable
{
Stoppable_test& test_;
public:
G(Stoppable& parent, Stoppable_test& test)
: Stoppable("G", parent)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 4, "G::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 5, "G::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 6, "G::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 7, "G::onChildrenStopped called out of order");
}
};
class H
: public Stoppable
{
Stoppable_test& test_;
public:
H(Stoppable& parent, Stoppable_test& test)
: Stoppable("H", parent)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 3, "H::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 6, "H::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 5, "H::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 8, "H::onChildrenStopped called out of order");
}
};
class B
: public Stoppable
{
G g_;
H h_;
Stoppable_test& test_;
public:
B(Stoppable& parent, Stoppable_test& test)
: Stoppable("B", parent)
, g_(*this, test)
, h_(*this, test)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 5, "B::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 7, "B::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 4, "B::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 6, "B::onChildrenStopped called out of order");
}
};
class I
: public Stoppable
{
Stoppable_test& test_;
public:
I(Stoppable& parent, Stoppable_test& test)
: Stoppable("I", parent)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 1, "I::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 8, "I::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 3, "I::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 10, "I::onChildrenStopped called out of order");
}
};
class C
: public Stoppable
{
I i_;
Stoppable_test& test_;
public:
C(Stoppable& parent, Stoppable_test& test)
: Stoppable("C", parent)
, i_(*this, test)
, test_(test)
{}
void onPrepare() override
{
test_.expect(++test_.count == 2, "C::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 9, "C::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 2, "C::onStop called out of order");
}
void onChildrenStopped() override
{
Stoppable::stopped();
test_.expect(--test_.count == 9, "C::onChildrenStopped called out of order");
}
};
class Root
: public RootStoppable
{
std::thread a_;
B b_;
C c_;
Stoppable_test& test_;
public:
Root(Stoppable_test& test)
: RootStoppable("R")
, a_(&A::run, std::make_unique<A>(*this, test))
, b_(*this, test)
, c_(*this, test)
, test_(test)
{}
void run()
{
prepare();
start();
stop();
}
void onPrepare() override
{
test_.expect(++test_.count == 11, "Root::onPrepare called out of order");
}
void onStart() override
{
test_.expect(--test_.count == 10, "Root::onStart called out of order");
}
void onStop() override
{
test_.expect(++test_.count == 1, "Root::onStop called out of order");
}
void onChildrenStopped() override
{
a_.join();
Stoppable::stopped();
test_.expect(--test_.count == 0, "Root::onChildrenStopped called out of order");
}
};
public:
void run()
{
{
Root rt(*this);
rt.run();
}
pass();
}
};
BEAST_DEFINE_TESTSUITE(Stoppable,beast_core,beast);
}