diff --git a/Changes b/Changes index d95afdf26f..dee3877b30 100644 --- a/Changes +++ b/Changes @@ -1,12 +1,20 @@ 10.7.x.x (relative to 10.7.0.0a4) ======== -API ---- +Improvements +------------ +- Canceller : + - Derived from RefCounted, to allow Cancellers to be shared between multiple clients. + - Enabled chaining of Cancellers, via `addChild()` and `removeChild()` methods and a `ScopedChild` utility class. - MessageHandler : Added `msg()` overload that accepts a `fmt::format_string` and arguments. - InternedString : Added specialisation for `fmt::formatter`. +Breaking Changes +---------------- + +- Canceller : Changed base class. + Build ----- diff --git a/include/IECore/Canceller.h b/include/IECore/Canceller.h index 8d53977c73..10863a342d 100644 --- a/include/IECore/Canceller.h +++ b/include/IECore/Canceller.h @@ -35,12 +35,11 @@ #ifndef IECORE_CANCELLER_H #define IECORE_CANCELLER_H -#include "IECore/Export.h" - -#include "boost/noncopyable.hpp" +#include "IECore/RefCounted.h" #include #include +#include namespace IECore { @@ -61,77 +60,81 @@ struct IECORE_API Cancelled /// Example : /// /// ``` -/// Canceller c; +/// CancellerPtr c = new Canceller; /// thread t( /// [&c] { /// while( 1 ) { -/// Canceller::check( &c ); +/// Canceller::check( c.get() ); /// } /// } /// ); /// c.cancel(); /// t.join(); /// ``` -class Canceller : public boost::noncopyable +class IECORE_API Canceller : public IECore::RefCounted { public : - Canceller() - : m_cancelled( false ), m_cancellationTime( 0 ) - { - } + Canceller(); - void cancel() - { - // Store time of first cancellation. We use `compare_exchange_weak()` - // to avoid unwanted updates on subsequent calls. - std::chrono::steady_clock::rep epoch( 0 ); - m_cancellationTime.compare_exchange_weak( - epoch, - std::chrono::steady_clock::now().time_since_epoch().count() - ); - // Set cancellation flag _after_ storing time, so that - // `elapsedTime()` always sees a valid time. - m_cancelled = true; - } - - bool cancelled() const - { - /// \todo Can we reduce overhead by reading - /// with a more relaxed memory ordering here? - return m_cancelled; - } + IE_CORE_DECLAREMEMBERPTR( Canceller ) - static void check( const Canceller *canceller ) - { - if( canceller && canceller->cancelled() ) - { - throw Cancelled(); - } - } + void cancel(); + bool cancelled() const; + + /// Throws `IECore::Cancelled` if `canceller` is non-null and has + /// been cancelled, otherwise does nothing. + static void check( const Canceller *canceller ); /// Returns the time passed since `cancel()` was first called, or `0` if /// it has not been called yet. - std::chrono::steady_clock::duration elapsedTime() const + std::chrono::steady_clock::duration elapsedTime() const; + + /// Adds a child canceller that will be cancelled automatically + /// when this is cancelled. If this is already cancels, then the + /// child is cancelled immediately. + void addChild( const Ptr &child ); + + /// Removes a child canceller. Additions are counted, and actual + /// removal only occurs when the number of removals equals the number + /// of additions. + void removeChild( const Ptr &child ); + + /// Convenience class to manage removal of children in an + /// exception-safe way. + class IECORE_API ScopedChild : boost::noncopyable { - if( m_cancelled ) - { - return std::chrono::steady_clock::now() - std::chrono::steady_clock::time_point( std::chrono::steady_clock::duration( m_cancellationTime ) ); - } - else - { - return std::chrono::steady_clock::duration( 0 ); - } - } + + public : + + /// Adds `child` to `parent`. + ScopedChild( Canceller *parent, const Ptr &child ); + /// Removes `child` from `parent`. + ~ScopedChild(); + + private : + + Canceller *m_parent; + Ptr m_child; + + }; + private : std::atomic_bool m_cancelled; std::atomic m_cancellationTime; + std::mutex m_childrenMutex; + std::unordered_map m_children; + }; +IE_CORE_DECLAREPTR( Canceller ) + }; // namespace IECore +#include "IECore/Canceller.inl" + #endif // IECORE_CANCELLER_H diff --git a/include/IECore/Canceller.inl b/include/IECore/Canceller.inl new file mode 100644 index 0000000000..ddde11169e --- /dev/null +++ b/include/IECore/Canceller.inl @@ -0,0 +1,130 @@ +////////////////////////////////////////////////////////////////////////// +// +// Copyright (c) 2018, Image Engine Design Inc. All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// +// * Redistributions in binary form must reproduce the above copyright +// notice, this list of conditions and the following disclaimer in the +// documentation and/or other materials provided with the distribution. +// +// * Neither the name of Image Engine Design nor the names of any +// other contributors to this software may be used to endorse or +// promote products derived from this software without specific prior +// written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS +// IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, +// THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR +// PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR +// CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, +// EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, +// PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +// PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF +// LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING +// NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS +// SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. +// +////////////////////////////////////////////////////////////////////////// + +#ifndef IECORE_CANCELLER_INL +#define IECORE_CANCELLER_INL + +namespace IECore +{ + +inline Canceller::Canceller() + : m_cancelled( false ), m_cancellationTime( 0 ) +{ +} + +inline void Canceller::cancel() +{ + // Store time of first cancellation. We use `compare_exchange_weak()` + // to avoid unwanted updates on subsequent calls. + std::chrono::steady_clock::rep epoch( 0 ); + m_cancellationTime.compare_exchange_weak( + epoch, + std::chrono::steady_clock::now().time_since_epoch().count() + ); + // Set cancellation flag _after_ storing time, so that + // `elapsedTime()` always sees a valid time. + if( !m_cancelled.exchange( true ) ) + { + std::lock_guard lock( m_childrenMutex ); + for( const auto &[child, count] : m_children ) + { + child->cancel(); + } + }; +} + +inline bool Canceller::cancelled() const +{ + /// \todo Can we reduce overhead by reading + /// with a more relaxed memory ordering here? + return m_cancelled; +} + +inline void Canceller::check( const Canceller *canceller ) +{ + if( canceller && canceller->cancelled() ) + { + throw Cancelled(); + } +} + +inline std::chrono::steady_clock::duration Canceller::elapsedTime() const +{ + if( m_cancelled ) + { + return std::chrono::steady_clock::now() - std::chrono::steady_clock::time_point( std::chrono::steady_clock::duration( m_cancellationTime ) ); + } + else + { + return std::chrono::steady_clock::duration( 0 ); + } +} + +inline void Canceller::addChild( const Ptr &child ) +{ + std::lock_guard lock( m_childrenMutex ); + m_children[child]++; + if( m_cancelled ) + { + child->cancel(); + } +} + +inline void Canceller::removeChild( const Ptr &child ) +{ + std::lock_guard lock( m_childrenMutex ); + auto it = m_children.find( child ); + if( it != m_children.end() ) + { + if( --it->second == 0 ) + { + m_children.erase( it ); + } + } +} + +inline Canceller::ScopedChild::ScopedChild( Canceller *parent, const Ptr &child ) + : m_parent( parent ), m_child( child ) +{ + m_parent->addChild( m_child ); +} + +inline Canceller::ScopedChild::~ScopedChild() +{ + m_parent->removeChild( m_child ); +} + +}; // namespace IECore + +#endif // IECORE_CANCELLER_INL diff --git a/src/IECorePython/CancellerBinding.cpp b/src/IECorePython/CancellerBinding.cpp index bff05d83eb..fce926d8cc 100644 --- a/src/IECorePython/CancellerBinding.cpp +++ b/src/IECorePython/CancellerBinding.cpp @@ -38,9 +38,12 @@ #include "IECorePython/CancellerBinding.h" #include "IECorePython/ExceptionBinding.h" +#include "IECorePython/RefCountedBinding.h" #include "IECore/Canceller.h" +#include + using namespace boost::python; using namespace IECore; @@ -52,6 +55,35 @@ double elapsedTimeWrapper( const Canceller &canceller ) return std::chrono::duration( canceller.elapsedTime() ).count(); } +class ScopedChildWrapper : public boost::noncopyable +{ + + public : + + ScopedChildWrapper( Canceller &parent, Canceller &child ) + : m_parent( &parent ), m_child( &child ) + { + } + + void enter() + { + m_scope.emplace( m_parent.get(), m_child ); + } + + bool exit( boost::python::object excType, boost::python::object excValue, boost::python::object excTraceBack ) + { + m_scope.reset(); + return false; // Don't suppress exceptions + } + + private : + + CancellerPtr m_parent; + CancellerPtr m_child; + std::optional m_scope; + +}; + } // namespace namespace IECorePython @@ -60,15 +92,24 @@ namespace IECorePython void bindCanceller() { - class_( "Canceller" ) - .def( "cancel", &Canceller::cancel ) - .def( "cancelled", &Canceller::cancelled ) - .def( "check", &Canceller::check ) - .staticmethod( "check" ) - .def( "elapsedTime", &elapsedTimeWrapper ) - ; + { + scope s = RefCountedClass( "Canceller" ) + .def( init<>() ) + .def( "cancel", &Canceller::cancel ) + .def( "cancelled", &Canceller::cancelled ) + .def( "check", &Canceller::check ) + .staticmethod( "check" ) + .def( "elapsedTime", &elapsedTimeWrapper ) + .def( "addChild", &Canceller::addChild ) + .def( "removeChild", &Canceller::removeChild ) + ; - register_ptr_to_python>(); + class_( "ScopedChild", no_init ) + .def( init() ) + .def( "__enter__", &ScopedChildWrapper::enter, return_self<>() ) + .def( "__exit__", &ScopedChildWrapper::exit ) + ; + } ExceptionClass( "Cancelled", PyExc_RuntimeError ) .def( init<>() ) diff --git a/test/IECore/CancellerTest.py b/test/IECore/CancellerTest.py index 248145e9a7..4659dca14d 100644 --- a/test/IECore/CancellerTest.py +++ b/test/IECore/CancellerTest.py @@ -168,5 +168,111 @@ def testExceptionLifetime( self ) : del exception self.assertIsNone( w() ) + def testAddChild( self ) : + + parent = IECore.Canceller() + child = IECore.Canceller() + + parent.addChild( child ) + self.assertFalse( parent.cancelled() ) + self.assertFalse( child.cancelled() ) + + parent.cancel() + self.assertTrue( parent.cancelled() ) + self.assertTrue( child.cancelled() ) + + def testChildCancelledIfParentAlreadyCancelled( self ) : + + parent = IECore.Canceller() + parent.cancel() + + child = IECore.Canceller() + parent.addChild( child ) + self.assertTrue( child.cancelled() ) + + def testRemoveChild( self ) : + + parent = IECore.Canceller() + child = IECore.Canceller() + + parent.addChild( child ) + parent.removeChild( child ) + + parent.cancel() + self.assertTrue( parent.cancelled() ) + self.assertFalse( child.cancelled() ) + + def testChildCount( self ) : + + parent = IECore.Canceller() + child = IECore.Canceller() + + parent.addChild( child ) + parent.addChild( child ) + parent.removeChild( child ) + + parent.cancel() + self.assertTrue( parent.cancelled() ) + self.assertTrue( child.cancelled() ) + + parent = IECore.Canceller() + child = IECore.Canceller() + + parent.addChild( child ) + parent.addChild( child ) + parent.removeChild( child ) + parent.removeChild( child ) + + parent.cancel() + self.assertTrue( parent.cancelled() ) + self.assertFalse( child.cancelled() ) + + def testChildCycles( self ) : + + canceller1 = IECore.Canceller() + canceller2 = IECore.Canceller() + canceller1.addChild( canceller2 ) + canceller2.addChild( canceller1 ) + + # This should return quickly despite the infinite parent->child->parent + # loop, because it can stop as soon as everything is cancelled. + canceller1.cancel() + self.assertTrue( canceller1.cancelled() ) + self.assertTrue( canceller2.cancelled() ) + + # In this case, the client is responsible for removing reference + # cycles to allow destruction. + canceller1.removeChild( canceller2 ) + self.assertEqual( canceller2.refCount(), 1 ) + self.assertEqual( canceller1.refCount(), 2 ) + + def testScopedChild( self ) : + + parent = IECore.Canceller() + child = IECore.Canceller() + + with IECore.Canceller.ScopedChild( parent, child ) : + parent.cancel() + self.assertTrue( parent.cancelled() ) + self.assertTrue( child.cancelled() ) + + parent = IECore.Canceller() + child = IECore.Canceller() + with IECore.Canceller.ScopedChild( parent, child ) : + pass + parent.cancel() + + self.assertTrue( parent.cancelled() ) + self.assertFalse( child.cancelled() ) + + parent = IECore.Canceller() + child = IECore.Canceller() + with IECore.Canceller.ScopedChild( parent, child ) : + with IECore.Canceller.ScopedChild( parent, child ) : + pass + parent.cancel() + self.assertTrue( parent.cancelled() ) + self.assertTrue( child.cancelled() ) + if __name__ == "__main__": unittest.main()