From b1a7b13ff58fa01329621c865199d38152ced2d4 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 29 Jan 2015 08:37:07 -0800 Subject: [PATCH 01/41] Adding support for continuation tasks This feature requires abstraction inversion on the `std::future` datatype on a platform-specific basis. Support for this feature will enable us to support `std::future` output types from `AutoFilter`. --- autowiring/auto_future.h | 6 +++++ autowiring/auto_future_win.h | 31 +++++++++++++++++++++++ src/autowiring/AutoPacket.cpp | 1 + src/autowiring/CMakeLists.txt | 2 ++ src/autowiring/test/AutoFutureTest.cpp | 35 ++++++++++++++++++++++++++ src/autowiring/test/CMakeLists.txt | 1 + 6 files changed, 76 insertions(+) create mode 100644 autowiring/auto_future.h create mode 100644 autowiring/auto_future_win.h create mode 100644 src/autowiring/test/AutoFutureTest.cpp diff --git a/autowiring/auto_future.h b/autowiring/auto_future.h new file mode 100644 index 000000000..145cc1880 --- /dev/null +++ b/autowiring/auto_future.h @@ -0,0 +1,6 @@ +#pragma once +#include + +#ifdef _MSC_VER +#include "auto_future_win.h" +#endif diff --git a/autowiring/auto_future_win.h b/autowiring/auto_future_win.h new file mode 100644 index 000000000..ff5c70e1d --- /dev/null +++ b/autowiring/auto_future_win.h @@ -0,0 +1,31 @@ +#pragma once +#include +#include + +namespace autowiring { + class _Task_async_state_unwrap: + public std::_Packaged_state + { + public: + ::Concurrency::task _Task; + }; + + /// + /// Platform-specific operation appending routine + /// + /// The future to be appended to + /// The lambda to be executed after the future is ready + template + void then(std::future& future, Fn&& fn) { + auto ptr = future._Ptr(); + + auto* taskAsync = dynamic_cast*>(ptr); + if (taskAsync) { + auto* unwrap = reinterpret_cast<_Task_async_state_unwrap*>(taskAsync); + unwrap->_Task.then(fn); + return; + } + + throw std::runtime_error("Unrecognized future type"); + } +} \ No newline at end of file diff --git a/src/autowiring/AutoPacket.cpp b/src/autowiring/AutoPacket.cpp index 7babc66a5..97ba37dec 100644 --- a/src/autowiring/AutoPacket.cpp +++ b/src/autowiring/AutoPacket.cpp @@ -10,6 +10,7 @@ #include "SatCounter.h" #include #include +#include using namespace autowiring; diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index 833e34c56..b558dd576 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -15,6 +15,7 @@ set(Autowiring_SRCS atomic_object.h at_exit.h auto_id.h + auto_future.h AutoConfig.cpp AutoConfig.h AutoConfigManager.cpp @@ -180,6 +181,7 @@ if(NOT APPLE) endif() add_windows_sources(Autowiring_SRCS + auto_future_win.h CoreThreadWin.cpp InterlockedExchangeWin.cpp thread_specific_ptr_win.h diff --git a/src/autowiring/test/AutoFutureTest.cpp b/src/autowiring/test/AutoFutureTest.cpp new file mode 100644 index 000000000..2a87cb68c --- /dev/null +++ b/src/autowiring/test/AutoFutureTest.cpp @@ -0,0 +1,35 @@ +#include "stdafx.h" +#include +#include +#include + +class AutoFutureTest: + public testing::Test +{}; + +TEST_F(AutoFutureTest, VerifyFutureExtraction) { + auto f = std::async( + std::launch::async, + [] { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + return 5; + } + ); + + auto pr = std::make_shared>(); + autowiring::then( + f, + [pr] { pr->set_value(201);} + ); + + // Wait for our async launch to conclude + f.get(); + + // Continuation task checks + auto future = pr->get_future(); + ASSERT_EQ( + std::future_status::ready, + future.wait_for(std::chrono::seconds(5)) + ) << "Continuation task did not launch as expected when the main task concluded"; + ASSERT_EQ(201, future.get()) << "Future value not propagated correctly back to the origin"; +} \ No newline at end of file diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 1dfd5b536..02ac09258 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -5,6 +5,7 @@ set(AutowiringTest_SRCS AutoConstructTest.cpp AutoFilterCollapseRulesTest.cpp AutoFilterDiagnosticsTest.cpp + AutoFutureTest.cpp AutoInjectableTest.cpp AutoPacketTest.cpp AutoPacketFactoryTest.cpp From 7f4e07bda5f74053c1fa6476b89d9365a0ec9779 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 29 Jan 2015 17:12:32 -0800 Subject: [PATCH 02/41] Adding a test to ensure destructors get run as expected --- autowiring/auto_future_win.h | 31 +++++++++++++++++++++----- src/autowiring/test/AutoFutureTest.cpp | 22 ++++++++++++++++++ 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/autowiring/auto_future_win.h b/autowiring/auto_future_win.h index ff5c70e1d..02cb13d02 100644 --- a/autowiring/auto_future_win.h +++ b/autowiring/auto_future_win.h @@ -3,6 +3,14 @@ #include namespace autowiring { + template + class _Packaged_state_unwrap: + public std::_Associated_state<_Ret*> + { + public: + std::function<_Ret(_ArgTypes...)> _Fn; + }; + class _Task_async_state_unwrap: public std::_Packaged_state { @@ -17,15 +25,28 @@ namespace autowiring { /// The lambda to be executed after the future is ready template void then(std::future& future, Fn&& fn) { + // Need a pointer to the underlying state block so we can decide what to do auto ptr = future._Ptr(); - auto* taskAsync = dynamic_cast*>(ptr); - if (taskAsync) { + if (auto* taskAsync = dynamic_cast*>(ptr)) { auto* unwrap = reinterpret_cast<_Task_async_state_unwrap*>(taskAsync); unwrap->_Task.then(fn); - return; - } + } else if (auto* deferredAsync = dynamic_cast*>(ptr)) { + auto* packagedState = static_cast*>(deferredAsync); + auto* unwrap = reinterpret_cast<_Packaged_state_unwrap*>(packagedState); - throw std::runtime_error("Unrecognized future type"); + // New function which consists of a call to the original then a call to the continuation + unwrap->_Fn = std::bind( + [] (const std::function& orig, const Fn& fn) { + auto rv = orig(); + fn(); + return rv; + }, + std::move(unwrap->_Fn), + std::forward(fn) + ); + } + else + throw std::runtime_error("Unrecognized future type"); } } \ No newline at end of file diff --git a/src/autowiring/test/AutoFutureTest.cpp b/src/autowiring/test/AutoFutureTest.cpp index 2a87cb68c..26a567914 100644 --- a/src/autowiring/test/AutoFutureTest.cpp +++ b/src/autowiring/test/AutoFutureTest.cpp @@ -32,4 +32,26 @@ TEST_F(AutoFutureTest, VerifyFutureExtraction) { future.wait_for(std::chrono::seconds(5)) ) << "Continuation task did not launch as expected when the main task concluded"; ASSERT_EQ(201, future.get()) << "Future value not propagated correctly back to the origin"; +} + +TEST_F(AutoFutureTest, CorrectDestructionTest) { + auto captured = std::make_shared(false); + { + auto f = std::async( + std::launch::deferred, + [] { + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + return 5; + } + ); + + autowiring::then(f, [captured] { + *captured = true; + }); + f.get(); + ASSERT_FALSE(captured.unique()) << "Lambda was destroyed prematurely"; + } + + ASSERT_TRUE(*captured) << "Continuation lambda did not run as expected"; + ASSERT_TRUE(captured.unique()) << "Continuation lambda leaked memory"; } \ No newline at end of file From 5f6d2570c3b0a4dee91a882ca1d56a95d4077519 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Fri, 30 Jan 2015 15:21:37 -0800 Subject: [PATCH 03/41] Add mac auto_future header --- autowiring/auto_future.h | 2 ++ autowiring/auto_future_mac.h | 15 +++++++++++++++ src/autowiring/CMakeLists.txt | 1 + 3 files changed, 18 insertions(+) create mode 100644 autowiring/auto_future_mac.h diff --git a/autowiring/auto_future.h b/autowiring/auto_future.h index 145cc1880..928c10a08 100644 --- a/autowiring/auto_future.h +++ b/autowiring/auto_future.h @@ -3,4 +3,6 @@ #ifdef _MSC_VER #include "auto_future_win.h" +#elif defined(__APPLE__) +#include "auto_future_mac.h" #endif diff --git a/autowiring/auto_future_mac.h b/autowiring/auto_future_mac.h new file mode 100644 index 000000000..6e1f5d240 --- /dev/null +++ b/autowiring/auto_future_mac.h @@ -0,0 +1,15 @@ +#pragma once +#include +#include + +namespace autowiring { + /// + /// Platform-specific operation appending routine + /// + /// The future to be appended to + /// The lambda to be executed after the future is ready + template + void then(std::future& future, Fn&& fn) { + + } +} \ No newline at end of file diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index b558dd576..88380a4aa 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -188,6 +188,7 @@ add_windows_sources(Autowiring_SRCS ) add_mac_sources(Autowiring_SRCS + auto_future_mac.h CoreThreadMac.cpp ) From 1ba1cf5f83b1755be6343c2c3ad6c072706985eb Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Tue, 20 Jan 2015 10:42:42 -0800 Subject: [PATCH 04/41] Enhance behavior of Unsatisfiable to have some predictable side-effects If an `AutoFilter` accepts an `std::shared_ptr`, it should be invoked if `MarkUnsatisfiable` is called. If it simply accepts `const T&`, then it should not be called. --- autowiring/AutoPacket.h | 70 +++---- autowiring/DecorationDisposition.h | 106 ++++------ autowiring/SharedPointerSlot.h | 10 +- autowiring/auto_arg.h | 7 +- src/autowiring/AutoPacket.cpp | 196 ++++++++++-------- src/autowiring/AutoPacketGraph.cpp | 2 +- src/autowiring/AutoPacketInternal.cpp | 17 +- src/autowiring/test/ArgumentTypeTest.cpp | 4 +- .../test/AutoFilterCollapseRulesTest.cpp | 37 +++- src/autowiring/test/CMakeLists.txt | 1 + src/autowiring/test/DecoratorTest.cpp | 8 +- src/autowiring/test/SatisfiabilityTest.cpp | 32 +++ 12 files changed, 279 insertions(+), 211 deletions(-) create mode 100644 src/autowiring/test/SatisfiabilityTest.cpp diff --git a/autowiring/AutoPacket.h b/autowiring/AutoPacket.h index 5831a3aea..ee503acd0 100644 --- a/autowiring/AutoPacket.h +++ b/autowiring/AutoPacket.h @@ -106,9 +106,9 @@ class AutoPacket: /// The decoration which was just added to this packet /// /// This method results in a call to the AutoFilter method on any subscribers which are - /// satisfied by this decoration. + /// satisfied by this decoration. This method must be called with m_lock held. /// - void UpdateSatisfaction(const DecorationKey& info); + void UpdateSatisfactionUnsafe(std::unique_lock&& lk, const DecorationDisposition& disposition); /// /// Performs a "satisfaction pulse", which will avoid notifying any deferred filters @@ -126,7 +126,7 @@ class AutoPacket: /// /// Performs a decoration operation but does not attach priors to successors. /// - void DecorateUnsafeNoPriors(const AnySharedPointer& ptr, const DecorationKey& key); + void DecorateNoPriors(const AnySharedPointer& ptr, DecorationKey key); /// Runtime counterpart to Decorate void Decorate(const AnySharedPointer& ptr, DecorationKey key); @@ -185,7 +185,7 @@ class AutoPacket: template bool Has(int tshift=0) const { std::lock_guard lk(m_lock); - return HasUnsafe(DecorationKey(auto_id::key(), tshift)); + return HasUnsafe(DecorationKey(auto_id::key(), true, tshift)); } /// @@ -197,7 +197,7 @@ class AutoPacket: const T* retVal; if (!Get(retVal, tshift)) - ThrowNotDecoratedException(DecorationKey(auto_id::key(), tshift)); + ThrowNotDecoratedException(DecorationKey(auto_id::key(), false, tshift)); return *retVal; } @@ -210,7 +210,7 @@ class AutoPacket: /// template bool Get(const T*& out, int tshift=0) const { - const DecorationDisposition* pDisposition = GetDisposition(DecorationKey(auto_id::key(), tshift)); + const DecorationDisposition* pDisposition = GetDisposition(DecorationKey(auto_id::key(), false, tshift)); if (pDisposition) { if (pDisposition->m_decorations.size() == 1) { out = static_cast(pDisposition->m_decorations[0]->ptr()); @@ -242,7 +242,7 @@ class AutoPacket: template bool Get(const std::shared_ptr*& out, int tshift=0) const { // Decoration must be present and the shared pointer itself must also be present - const DecorationDisposition* pDisposition = GetDisposition(DecorationKey(auto_id::key(), tshift)); + const DecorationDisposition* pDisposition = GetDisposition(DecorationKey(auto_id::key(), true, tshift)); if (!pDisposition || pDisposition->m_decorations.size() != 1) { out = nullptr; return false; @@ -262,7 +262,7 @@ class AutoPacket: template bool Get(std::shared_ptr& out, int tshift = 0) const { std::lock_guard lk(m_lock); - auto deco = m_decorations.find(DecorationKey(auto_id::key(), tshift)); + auto deco = m_decorations.find(DecorationKey(auto_id::key(), true, tshift)); if(deco != m_decorations.end() && deco->second.m_state == DispositionState::Satisfied) { auto& disposition = deco->second; if(disposition.m_decorations.size() == 1) { @@ -274,11 +274,14 @@ class AutoPacket: return false; } + /// + /// The shared pointer decoration for the specified type and time shift, or nullptr if no such decoration exists + /// template - const std::shared_ptr& GetShared(int tshift = 0) const { + const std::shared_ptr* GetShared(int tshift = 0) const { const std::shared_ptr* retVal; Get(retVal, tshift); - return *retVal; + return retVal; } /// @@ -291,7 +294,7 @@ class AutoPacket: template const T** GetAll(int tshift = 0) const { std::lock_guard lk(m_lock); - auto q = m_decorations.find(DecorationKey(auto_id::key(), tshift)); + auto q = m_decorations.find(DecorationKey(auto_id::key(), true, tshift)); // If decoration doesn't exist, return empty null-terminated buffer if (q == m_decorations.end()) { @@ -328,22 +331,8 @@ class AutoPacket: /// template void Unsatisfiable(void) { - DecorationKey key(auto_id::key()); - { - // Insert a null entry at this location: - std::lock_guard lk(m_lock); - auto& entry = m_decorations[key]; - entry.SetKey(key); // Ensure correct type if instantiated here - if(entry.m_state == DispositionState::PartlySatisfied || - entry.m_state == DispositionState::Satisfied) - throw std::runtime_error("Cannot mark a decoration as unsatisfiable when that decoration is already present on this packet"); - - // Mark the entry as permanently checked-out - entry.m_state = DispositionState::Unsatisfiable; - } - - // Now trigger a rescan: - MarkUnsatisfiable(key); + MarkUnsatisfiable(DecorationKey(auto_id::key(), false, 0)); + MarkUnsatisfiable(DecorationKey(auto_id::key(), true, 0)); } /// @@ -356,11 +345,12 @@ class AutoPacket: /// template const T& Decorate(T t) { - DecorationKey key(auto_id::key()); - // Create a copy of the input, put the copy in a shared pointer auto ptr = std::make_shared(std::forward(t)); - Decorate(AnySharedPointer(ptr), key); + Decorate( + AnySharedPointer(ptr), + DecorationKey(auto_id::key(), true, 0) + ); return *ptr; } @@ -375,7 +365,7 @@ class AutoPacket: /// template void Decorate(std::shared_ptr ptr) { - DecorationKey key(auto_id::key()); + DecorationKey key(auto_id::key(), true, 0); // We don't want to see this overload used on a const T static_assert(!std::is_const::value, "Cannot decorate a shared pointer to const T with this overload"); @@ -425,8 +415,8 @@ class AutoPacket: // Perform standard decoration with a short initialization: std::unique_lock lk(m_lock); DecorationDisposition* pTypeSubs[1 + sizeof...(Ts)] = { - &DecorateImmediateUnsafe(DecorationKey(auto_id::key()), &immed), - &DecorateImmediateUnsafe(DecorationKey(auto_id::key()), &immeds)... + &DecorateImmediateUnsafe(DecorationKey(auto_id::key(), false, 0), &immed), + &DecorateImmediateUnsafe(DecorationKey(auto_id::key(), false, 0), &immeds)... }; lk.unlock(); @@ -442,12 +432,14 @@ class AutoPacket: // Now trigger a rescan to hit any deferred, unsatisfiable entries: #if autowiring_USE_LIBCXX - for (const std::type_info* ti : {&auto_id::key(), &auto_id::key()...}) - MarkUnsatisfiable(DecorationKey(*ti)); + for (const std::type_info* ti : {&auto_id::key(), &auto_id::key()...}) { + MarkUnsatisfiable(DecorationKey(*ti, true, 0)); + MarkUnsatisfiable(DecorationKey(*ti, false, 0)); + } #else bool dummy[] = { - (MarkUnsatisfiable(DecorationKey(auto_id::key())), false), - (MarkUnsatisfiable(DecorationKey(auto_id::key())), false)... + (MarkUnsatisfiable(DecorationKey(auto_id::key(), false, 0)), false), + (MarkUnsatisfiable(DecorationKey(auto_id::key(), false, 0)), false)... }; (void)dummy; #endif @@ -505,7 +497,9 @@ class AutoPacket: /// True if the indicated type has been requested for use by some consumer template bool HasSubscribers(void) const { - return HasSubscribers(DecorationKey(auto_id::key())); + return + HasSubscribers(DecorationKey(auto_id::key(), false, 0)) || + HasSubscribers(DecorationKey(auto_id::key(), true, 0)); } struct SignalStub { diff --git a/autowiring/DecorationDisposition.h b/autowiring/DecorationDisposition.h index 0aecc8274..d3ad221fa 100644 --- a/autowiring/DecorationDisposition.h +++ b/autowiring/DecorationDisposition.h @@ -9,25 +9,36 @@ struct SatCounter; struct DecorationKey { + DecorationKey(void) : + ti(nullptr), + is_shared(false), + tshift(-1) + {} + DecorationKey(const DecorationKey& rhs) : ti(rhs.ti), + is_shared(rhs.is_shared), tshift(rhs.tshift) {} - explicit DecorationKey(const std::type_info& ti, int tshift = 0) : - ti(ti), + explicit DecorationKey(const std::type_info& ti, bool is_shared, int tshift) : + ti(&ti), + is_shared(is_shared), tshift(tshift) {} // The type index - const std::type_info& ti; + const std::type_info* ti; + + // True if this decoration can be used with AutoFilters that accept a shared_ptr input type + bool is_shared; // Zero refers to a decoration created on this packet, a positive number [tshift] indicates // a decoration attached [tshift] packets ago. int tshift; bool operator==(const DecorationKey& rhs) const { - return ti == rhs.ti && tshift == rhs.tshift; + return ti == rhs.ti && is_shared == rhs.is_shared && tshift == rhs.tshift; } }; @@ -35,10 +46,13 @@ namespace std { template<> struct hash { size_t operator()(const DecorationKey& key) const { + return + key.tshift + + (key.is_shared ? 0x80000 : 0x70000) + #if AUTOWIRING_USE_LIBCXX - return key.ti.hash_code() + key.tshift; + key.ti->hash_code(); #else - return std::type_index(key.ti).hash_code() + key.tshift; + std::type_index(*key.ti).hash_code(); #endif } }; @@ -46,9 +60,20 @@ namespace std { // The possible states for a DecorationDisposition enum class DispositionState { + // No decorations attached Unsatisfied, + + // Some decorations present, but not all of them. Cannot proceed. PartlySatisfied, + + // Everything attached, ready to go Satisfied, + + // Unsatisfiable, and the callers on this decoration cannot accept a non-null + // entry--IE, they accept const references as inputs. + UnsatisfiableNoCall, + + // This decoration will never be satisfied. Calls are generated to the Unsatisfiable }; @@ -57,62 +82,26 @@ enum class DispositionState { /// struct DecorationDisposition { -#if AUTOWIRING_USE_LIBCXX - DecorationDisposition(DecorationDisposition&&) = delete; - void operator=(DecorationDisposition&&) = delete; -#else - // The methods below are needed for c++98 builds - DecorationDisposition(DecorationDisposition&& source) : - m_decorations(source.m_decorations), - m_pImmediate(source.m_pImmediate), - m_publishers(source.m_publishers), - m_state(source.m_state) - {} - DecorationDisposition& operator=(DecorationDisposition&& source) { - m_decorations = std::move(source.m_decorations); - m_pImmediate = source.m_pImmediate; - source.m_pImmediate = nullptr; - m_publishers = std::move(source.m_publishers); - m_state = source.m_state; - return *this; - } -#endif //AUTOWIRING_USE_LIBCXX - DecorationDisposition(void) : - m_type(nullptr), m_pImmediate(nullptr), m_state(DispositionState::Unsatisfied) {} DecorationDisposition(const DecorationDisposition& source) : - m_type(source.m_type), + m_decorations(source.m_decorations), m_pImmediate(source.m_pImmediate), m_publishers(source.m_publishers), m_subscribers(source.m_subscribers), m_state(source.m_state) {} - - DecorationDisposition& operator=(const DecorationDisposition& source) { - m_type = source.m_type; - m_pImmediate = source.m_pImmediate; - m_publishers = source.m_publishers; - m_subscribers = source.m_subscribers; - m_state = source.m_state; - return *this; - } - -private: - // Destructured key for this decoration. Use accessor functions to access - // This is needed because DecorationKey doesn't have a default constructor - const std::type_info* m_type; - int m_tshift; -public: // The decoration proper--potentially, this decoration might be from a prior execution of this // packet. In the case of immediate decorations, this value will be invalid. + // Valid if and only if is_shared is false. std::vector m_decorations; - // A pointer to the immediate decorations, if one is specified, or else nullptr + // A pointer to the immediate decorations, if one is specified, or else nullptr. + // Valid if and only if is_shared is true. const void* m_pImmediate; // Providers for this decoration, where it can be statically inferred. Note that a provider for @@ -126,20 +115,17 @@ struct DecorationDisposition // The current state of this disposition DispositionState m_state; - // Set the key that identifies this decoration - void SetKey(const DecorationKey& key) { - m_type = &key.ti; - m_tshift = key.tshift; - } - - // Get the key that identifies this decoration - DecorationKey GetKey(void) const { - assert(m_type); - return DecorationKey(*m_type, m_tshift); - } - - operator bool() { - return !!m_type; + /// + /// True if all publishers have run on this disposition + /// + /// + /// Publication is complete automatically on this type if there are no statically known publishers, + /// and at least one decoration has been attached to this disposition. + /// + bool IsPublicationComplete(void) const { + return + !m_decorations.empty() && + m_decorations.size() >= m_publishers.size(); } void Reset(void) { diff --git a/autowiring/SharedPointerSlot.h b/autowiring/SharedPointerSlot.h index 915836b30..d7412af1f 100644 --- a/autowiring/SharedPointerSlot.h +++ b/autowiring/SharedPointerSlot.h @@ -135,16 +135,20 @@ struct SharedPointerSlot { /// virtual void reset(void) {} + template + static const std::shared_ptr& null(void) { + static const std::shared_ptr s_empty; + return s_empty; + } + /// /// Attempts to coerce this type to the specified type /// template const std::shared_ptr& as(void) const { - static const std::shared_ptr s_empty; - if (type() == typeid(void)) // This is allowed, we always permit null to be cast to the requested type. - return s_empty; + return null(); if (type() != typeid(auto_id)) throw std::runtime_error("Attempted to obtain a shared pointer for an unrelated type"); diff --git a/autowiring/auto_arg.h b/autowiring/auto_arg.h index c95e7f1d9..b7c054701 100644 --- a/autowiring/auto_arg.h +++ b/autowiring/auto_arg.h @@ -76,7 +76,10 @@ class auto_arg> template static const std::shared_ptr& arg(C& packet) { - return packet.template GetShared(); + auto retVal = packet.template GetShared(); + if (!retVal) + return SharedPointerSlot::null(); + return *retVal; } }; @@ -203,7 +206,7 @@ class auto_arg> static const bool is_input = true; static const bool is_output = false; - static const bool is_shared = false; + static const bool is_shared = true; static const bool is_multi = false; static const int tshift = N; diff --git a/src/autowiring/AutoPacket.cpp b/src/autowiring/AutoPacket.cpp index 71e754157..ef7c229a3 100644 --- a/src/autowiring/AutoPacket.cpp +++ b/src/autowiring/AutoPacket.cpp @@ -28,12 +28,9 @@ AutoPacket::~AutoPacket(void) { // Mark decorations of successor packets that use decorations // originating from this packet as unsatisfiable - for (auto& pair : m_decorations) { - if (!pair.first.tshift && - pair.second.m_state != DispositionState::Satisfied) { - MarkSuccessorsUnsatisfiable(DecorationKey(pair.first.ti, 0)); - } - } + for (auto& pair : m_decorations) + if (!pair.first.tshift && pair.second.m_state != DispositionState::Satisfied) + MarkSuccessorsUnsatisfiable(DecorationKey(*pair.first.ti, pair.first.is_shared, 0)); // Needed for the AutoPacketGraph NotifyTeardownListeners(); @@ -65,9 +62,6 @@ DecorationDisposition& AutoPacket::DecorateImmediateUnsafe(const DecorationKey& // Obtain the decoration disposition of the entry we will be returning DecorationDisposition& dec = m_decorations[key]; - // Ensure correct key if instantiated here - dec.SetKey(key); - if (dec.m_state != DispositionState::Unsatisfied) { std::stringstream ss; ss << "Cannot perform immediate decoration with type " << autowiring::demangle(key.ti) @@ -83,10 +77,8 @@ DecorationDisposition& AutoPacket::DecorateImmediateUnsafe(const DecorationKey& void AutoPacket::AddSatCounter(SatCounter& satCounter) { for(auto pCur = satCounter.GetAutoFilterInput(); *pCur; pCur++) { - DecorationKey key(*pCur->ti, pCur->tshift); - + DecorationKey key(*pCur->ti, pCur->is_shared, pCur->tshift); DecorationDisposition& entry = m_decorations[key]; - entry.SetKey(key); // Decide what to do with this entry: if (pCur->is_input) { @@ -97,8 +89,12 @@ void AutoPacket::AddSatCounter(SatCounter& satCounter) { } entry.m_subscribers.push_back(&satCounter); - if (entry.m_state == DispositionState::Satisfied) + switch (entry.m_state) { + case DispositionState::Satisfied: + case DispositionState::Unsatisfiable: satCounter.Decrement(); + break; + } } if (pCur->is_output) { if(!entry.m_publishers.empty()) @@ -114,24 +110,27 @@ void AutoPacket::AddSatCounter(SatCounter& satCounter) { } // Make sure decorations exist for timeshifts less that key's timeshift - for (int tshift = 0; tshift < key.tshift; ++tshift) { - DecorationKey shiftKey(key.ti, tshift); - m_decorations[shiftKey].SetKey(shiftKey); - } + for (int tshift = 0; tshift < key.tshift; ++tshift) + m_decorations[DecorationKey(*key.ti, key.is_shared, tshift)]; } } void AutoPacket::MarkUnsatisfiable(const DecorationKey& key) { - // Perform unsatisfaction logic - if (key.tshift) { - { - std::lock_guard lk(m_lock); - auto& disp = m_decorations[key]; - disp.m_state = DispositionState::Satisfied; - disp.m_decorations.clear(); - } - UpdateSatisfaction(key); - } + // Ensure correct type if instantiated here + std::unique_lock lk(m_lock); + auto& entry = m_decorations[key]; + + if (entry.m_state == DispositionState::PartlySatisfied || entry.m_state == DispositionState::Satisfied) + throw std::runtime_error("Cannot mark a decoration as unsatisfiable when that decoration is already present on this packet"); + + // Mark the entry as permanently unsatisfiable: + entry.m_state = + key.is_shared ? + DispositionState::Unsatisfiable : + DispositionState::UnsatisfiableNoCall; + + // Notify all consumers + UpdateSatisfactionUnsafe(std::move(lk), entry); } void AutoPacket::MarkSuccessorsUnsatisfiable(DecorationKey key) { @@ -150,24 +149,24 @@ void AutoPacket::MarkSuccessorsUnsatisfiable(DecorationKey key) { } } -void AutoPacket::UpdateSatisfaction(const DecorationKey& info) { +void AutoPacket::UpdateSatisfactionUnsafe(std::unique_lock&& lk, const DecorationDisposition& disposition) { std::vector callQueue; - { - std::lock_guard lk(m_lock); - auto dFind = m_decorations.find(info); - if(dFind == m_decorations.end()) - // Trivial return, there's no subscriber to this decoration and so we have nothing to do - return; - - // Update satisfaction inside of lock - DecorationDisposition& decoration = dFind->second; - for(SatCounter* satCounter : decoration.m_subscribers) - if(satCounter->Decrement()) + // Update satisfaction inside of lock + switch (disposition.m_state) { + case DispositionState::Unsatisfiable: + case DispositionState::Satisfied: + for (SatCounter* satCounter : disposition.m_subscribers) + if (satCounter->Decrement()) callQueue.push_back(satCounter); + break; + default: + // Nothing to do + return; } // Make calls outside of lock, to avoid deadlock from decorations in methods + lk.unlock(); for (SatCounter* call : callQueue) call->GetCall()(call->GetAutoFilter(), *this); } @@ -178,17 +177,13 @@ void AutoPacket::PulseSatisfaction(DecorationDisposition* pTypeSubs[], size_t nI // First pass, decrement what we can: { std::lock_guard lk(m_lock); - for(size_t i = nInfos; i--;) { + for(size_t i = nInfos; i--;) for(SatCounter* cur : pTypeSubs[i]->m_subscribers) { if( // We only care about sat counters that aren't deferred--skip everyone else // Deferred calls will be too late. !cur->IsDeferred() && - // We cannot satisfy shared_ptr arguments, since the implied existence - // guarantee of shared_ptr is violated - !cur->GetArgumentType(&pTypeSubs[i]->GetKey().ti)->is_shared && - // Now do the decrementation and proceed even if optional > 0, // since this is the only opportunity to fulfill the arguments cur->Decrement() @@ -196,7 +191,6 @@ void AutoPacket::PulseSatisfaction(DecorationDisposition* pTypeSubs[], size_t nI // Finally, queue a call for this type callQueue.push_back(cur); } - } } // Make calls outside of lock, to avoid deadlock from decorations in methods @@ -219,47 +213,71 @@ bool AutoPacket::HasUnsafe(const DecorationKey& key) const { return q->second.m_state == DispositionState::Satisfied; } -void AutoPacket::DecorateUnsafeNoPriors(const AnySharedPointer& ptr, const DecorationKey& key) { - auto& entry = m_decorations[key]; - entry.SetKey(key); // Ensure correct key if instantiated here +void AutoPacket::DecorateNoPriors(const AnySharedPointer& ptr, DecorationKey key) { + DecorationDisposition* dispositionA; + DecorationDisposition* dispositionB; + { + std::lock_guard lk(m_lock); - if (entry.m_state == DispositionState::Satisfied) { - std::stringstream ss; - ss << "Cannot decorate this packet with type " << autowiring::demangle(ptr) - << ", the requested decoration is already satisfied"; - throw std::runtime_error(ss.str()); - } - if (entry.m_state == DispositionState::Unsatisfiable) { - std::stringstream ss; - ss << "Cannot check out decoration of type " << autowiring::demangle(ptr) - << ", it has been marked unsatisfiable"; - throw std::runtime_error(ss.str()); - } + auto transition = [&](DecorationKey& key){ + DecorationDisposition& disposition = m_decorations[key]; + switch (disposition.m_state) { + case DispositionState::Satisfied: + { + std::stringstream ss; + ss << "Cannot decorate this packet with type " << autowiring::demangle(ptr) + << ", the requested decoration is already satisfied"; + throw std::runtime_error(ss.str()); + } + break; + case DispositionState::Unsatisfiable: + case DispositionState::UnsatisfiableNoCall: + { + std::stringstream ss; + ss << "Cannot check out decoration of type " << autowiring::demangle(ptr) + << ", it has been marked unsatisfiable"; + throw std::runtime_error(ss.str()); + } + break; + } - entry.m_decorations.push_back(ptr); + // Decoration attaches here + disposition.m_decorations.push_back(ptr); + return &disposition; + }; - if (entry.m_decorations.size() >= entry.m_publishers.size()) { - entry.m_state = DispositionState::Satisfied; - } else { - entry.m_state = DispositionState::PartlySatisfied; + key.is_shared = false; + dispositionA = transition(key); + key.is_shared = true; + dispositionB = transition(key); } - // This allows us to retrieve correct entries for decorated input requests - AnySharedPointer decoration(entry.m_decorations.back()); + // Uniformly advance state: + switch (dispositionA->m_state) { + case DispositionState::Unsatisfied: + case DispositionState::PartlySatisfied: + // Permit a transition to another state + if (dispositionA->IsPublicationComplete() && dispositionB->IsPublicationComplete()) { + dispositionA->m_state = dispositionB->m_state = DispositionState::Satisfied; + + UpdateSatisfactionUnsafe(std::unique_lock(m_lock), *dispositionA); + UpdateSatisfactionUnsafe(std::unique_lock(m_lock), *dispositionB); + } + else + dispositionA->m_state = dispositionB->m_state = DispositionState::PartlySatisfied; + break; + default: + // Do nothing, no advancing to any states from here + break; + } } void AutoPacket::Decorate(const AnySharedPointer& ptr, DecorationKey key) { auto cur = shared_from_this(); do { - // Decorate, first, while holding down a lock - (std::lock_guard)cur->m_lock, - cur->DecorateUnsafeNoPriors(ptr, key); - - // Now update satisfaction set on this entry - if (m_decorations[key].m_state == DispositionState::Satisfied) { - cur->UpdateSatisfaction(key); - } + // Update satisfaction set on this entry + cur->DecorateNoPriors(ptr, key); // Obtain the successor cur = cur->Successor(); @@ -317,28 +335,22 @@ void AutoPacket::ForwardAll(std::shared_ptr recipient) const { // Copy decorations into an internal decorations maintenance collection. The values // in this collection are guaranteed to be stable in memory, and there are stable states // that can be relied upon without synchronization. - std::vector> dd; + std::vector dd; { std::lock_guard lk(m_lock); for (const auto& decoration : m_decorations) - // Only fully satisfied decorations are considered for propagation - if (decoration.second.m_state == DispositionState::Satisfied) - for (const auto& single : decoration.second.m_decorations) - dd.push_back(std::make_pair(&decoration.second, single)); + // Only fully satisfied shared decorations are considered for propagation + if ( + decoration.first.is_shared && + decoration.second.m_state == DispositionState::Satisfied + ) + dd.push_back(decoration); } - // Lock down recipient colleciton while we go through and attach decorations: - for (auto& cur : dd) { - { - std::lock_guard lk(recipient->m_lock); - if (recipient->HasUnsafe(cur.first->GetKey())) - // Key already present, circle around - continue; - } - - // Decorate while unsynchronized: - recipient->Decorate(cur.second, cur.first->GetKey()); - } + // Lock down recipient collection while we go through and attach decorations: + for (auto& cur : dd) + for (const auto& decoration : cur.second.m_decorations) + recipient->Decorate(decoration, cur.first); } const SatCounter* AutoPacket::AddRecipient(const AutoFilterDescriptor& descriptor) { diff --git a/src/autowiring/AutoPacketGraph.cpp b/src/autowiring/AutoPacketGraph.cpp index ae4933be5..3259bf791 100644 --- a/src/autowiring/AutoPacketGraph.cpp +++ b/src/autowiring/AutoPacketGraph.cpp @@ -81,7 +81,7 @@ void AutoPacketGraph::AutoFilter(AutoPacket& packet) { packet.AddTeardownListener([this, &packet] () { for (auto& cur : packet.GetDecorations()) { auto& decoration = cur.second; - auto type = &decoration.GetKey().ti; + auto type = cur.first.ti; for (auto& publisher : decoration.m_publishers) { if (!publisher->remaining) { diff --git a/src/autowiring/AutoPacketInternal.cpp b/src/autowiring/AutoPacketInternal.cpp index 4556942b3..a3e6a8183 100644 --- a/src/autowiring/AutoPacketInternal.cpp +++ b/src/autowiring/AutoPacketInternal.cpp @@ -21,16 +21,14 @@ void AutoPacketInternal::Initialize(bool isFirstPacket) { // Find all subscribers with no required or optional arguments: std::vector callCounters; for (auto* satCounter = m_firstCounter; satCounter; satCounter = satCounter->flink) { - // Prime the satisfaction graph for element: AddSatCounter(*satCounter); - if (!satCounter->remaining) callCounters.push_back(satCounter); } // Mark timeshifted decorations as unsatisfiable on the first packet - if (isFirstPacket) { + if (isFirstPacket) for (auto& dec : m_decorations) { auto& key = dec.first; if (key.tshift) { @@ -38,7 +36,6 @@ void AutoPacketInternal::Initialize(bool isFirstPacket) { MarkSuccessorsUnsatisfiable(key); } } - } // Call all subscribers with no required or optional arguments: // NOTE: This may result in decorations that cause other subscribers to be called. @@ -46,7 +43,17 @@ void AutoPacketInternal::Initialize(bool isFirstPacket) { call->GetCall()(call->GetAutoFilter(), *this); // First-call indicated by argumument type AutoPacket&: - UpdateSatisfaction(DecorationKey(typeid(auto_arg::id_type))); + for (bool is_shared : {false, true}) { + std::unique_lock lk(m_lock); + + // Don't modify the decorations set if nobody expects an AutoPacket input + auto q = m_decorations.find(DecorationKey(typeid(auto_arg::id_type), is_shared, 0)); + if (q == m_decorations.end()) + continue; + + q->second.m_state = DispositionState::Satisfied; + UpdateSatisfactionUnsafe(std::move(lk), q->second); + } } std::shared_ptr AutoPacketInternal::SuccessorInternal(void) { diff --git a/src/autowiring/test/ArgumentTypeTest.cpp b/src/autowiring/test/ArgumentTypeTest.cpp index 3f98fd989..020881872 100644 --- a/src/autowiring/test/ArgumentTypeTest.cpp +++ b/src/autowiring/test/ArgumentTypeTest.cpp @@ -87,7 +87,7 @@ TEST_F(ArgumentTypeTest, TestAutoIn) { // Deduced Type const auto& arg = t_argShared::arg(*packet); - ASSERT_TRUE(arg.unique()) << "AutoPacket should store the sole shared pointer reference"; + ASSERT_EQ(2UL, arg.use_count()) << "AutoPacket should store exactly two shared pointer references"; } TEST_F(ArgumentTypeTest, TestAutoOut) { @@ -113,6 +113,6 @@ TEST_F(ArgumentTypeTest, TestAutoOut) { const Argument<0>* arg = nullptr; ASSERT_TRUE(packet->Get(arg)) << "Missing output"; - ASSERT_EQ(a0, packet->GetShared>()) << "Shared pointer copied incorrectly"; + ASSERT_EQ(a0, *packet->GetShared>()) << "Shared pointer copied incorrectly"; ASSERT_EQ(1, arg->i) << "Output was not copied"; } diff --git a/src/autowiring/test/AutoFilterCollapseRulesTest.cpp b/src/autowiring/test/AutoFilterCollapseRulesTest.cpp index 7ce36aab9..6c1cc8894 100644 --- a/src/autowiring/test/AutoFilterCollapseRulesTest.cpp +++ b/src/autowiring/test/AutoFilterCollapseRulesTest.cpp @@ -83,7 +83,7 @@ TEST_F(AutoFilterCollapseRulesTest, SharedPtrCollapse) { auto packet = factory->NewPacket(); packet->DecorateImmediate(constr_int); ASSERT_EQ(1, constr_filter->m_called) << "Called const reference method " << constr_filter->m_called << " times"; - ASSERT_EQ(0, shared_filter->m_called) << "Called shared pointer method " << shared_filter->m_called << " times"; + ASSERT_EQ(1, shared_filter->m_called) << "Called shared pointer method " << shared_filter->m_called << " times"; } constr_filter->m_called = 0; shared_filter->m_called = 0; @@ -129,14 +129,21 @@ TEST_F(AutoFilterCollapseRulesTest, ConstCollapse) { TEST_F(AutoFilterCollapseRulesTest, SharedPointerAliasingRules) { AutoRequired factory; - AutoRequired>> genFilter1; - AutoRequired> genFilter2; + bool gen1Called = false; + bool gen2Called = false; + + *factory += [&gen1Called](AutoPacket&, std::shared_ptr) { + gen1Called = true; + }; + *factory += [&gen2Called](AutoPacket&, int) { + gen2Called = true; + }; auto packet = factory->NewPacket(); packet->Decorate(55); - ASSERT_EQ(1UL, genFilter1->m_called) << "AutoFilter accepting a shared pointer was not called as expected"; - ASSERT_EQ(1UL, genFilter2->m_called) << "AutoFilter accepting a decorated type was not called as expected"; + ASSERT_TRUE(gen1Called) << "AutoFilter accepting a shared pointer was not called as expected"; + ASSERT_TRUE(gen2Called) << "AutoFilter accepting a decorated type was not called as expected"; } class ProducesSharedPointer { @@ -207,3 +214,23 @@ TEST_F(AutoFilterCollapseRulesTest, CtorRequiredWPI) { AutoRequired(); AutoRequired()->NewPacket(); } + +TEST_F(AutoFilterCollapseRulesTest, UnsatisfiableDecoration) { + AutoRequired factory; + + bool bInvoked = false; + *factory += [&](std::shared_ptr> dec) { + ASSERT_FALSE(dec) << "Decoration was attached to the packet when it should not have been"; + bInvoked = true; + }; + *factory += [](const Decoration<0>&) { + FAIL() << "A filter was invoked when it should have been unsatisfiable"; + }; + + // Kick off what should be a launch of the shared pointer-accepting input + auto packet = factory->NewPacket(); + packet->Decorate(std::shared_ptr>()); + + // Ensure the correct decoration was invoked + ASSERT_TRUE(bInvoked) << "AutoFilter was not invoked in an unsatisfiable case as expected"; +} \ No newline at end of file diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 1dfd5b536..68933270e 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -45,6 +45,7 @@ set(AutowiringTest_SRCS SelfSelectingFixtureTest.cpp TeardownNotifierTest.cpp TypeRegistryTest.cpp + SatisfiabilityTest.cpp ScopeTest.cpp SnoopTest.cpp TupleTest.cpp diff --git a/src/autowiring/test/DecoratorTest.cpp b/src/autowiring/test/DecoratorTest.cpp index 3ded3b1cb..9c3822894 100644 --- a/src/autowiring/test/DecoratorTest.cpp +++ b/src/autowiring/test/DecoratorTest.cpp @@ -68,6 +68,8 @@ TEST_F(DecoratorTest, VerifySimplePacketDecoration) { } TEST_F(DecoratorTest, VerifyDecoratorAwareness) { + auto filter = [](const Decoration<0>& zero, const Decoration<1>& one) {}; + // Create a packet while the factory has no subscribers: AutoRequired factory; auto packet1 = factory->NewPacket(); @@ -76,17 +78,17 @@ TEST_F(DecoratorTest, VerifyDecoratorAwareness) { ASSERT_FALSE(packet1->HasSubscribers>()) << "Subscription exists where one should not have existed"; // Create another packet where a subscriber exists: - AutoRequired filterA; + *factory += filter; auto packet2 = factory->NewPacket(); // Verify the first packet still does not have subscriptions: - ASSERT_THROW(packet1->GetSatisfaction(), autowiring_error) << "Subscription was incorrectly, retroactively added to a packet"; + ASSERT_THROW(packet1->GetSatisfaction(), autowiring_error) << "Subscription was incorrectly, retroactively added to a packet"; ASSERT_FALSE(packet1->HasSubscribers>()) << "Subscription was incorrectly, retroactively added to a packet"; ASSERT_EQ(0UL, packet1->GetDecorationTypeCount()) << "Subscription was incorrectly, retroactively added to a packet"; ASSERT_FALSE(packet1->HasSubscribers>()) << "Subscription was incorrectly, retroactively added to a packet"; // Verify the second one does: - ASSERT_THROW(packet2->GetSatisfaction(), autowiring_error) << "Packet lacked an expected subscription"; + ASSERT_THROW(packet2->GetSatisfaction(), autowiring_error) << "Packet lacked an expected subscription"; ASSERT_EQ(2UL, packet2->GetDecorationTypeCount()) << "Incorrect count of expected decorations"; ASSERT_TRUE(packet2->HasSubscribers>()) << "Packet lacked an expected subscription"; } diff --git a/src/autowiring/test/SatisfiabilityTest.cpp b/src/autowiring/test/SatisfiabilityTest.cpp new file mode 100644 index 000000000..931799734 --- /dev/null +++ b/src/autowiring/test/SatisfiabilityTest.cpp @@ -0,0 +1,32 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. +#include "stdafx.h" +#include +#include "TestFixtures/Decoration.hpp" + +class SatisfiabilityTest: + public testing::Test +{ +public: + SatisfiabilityTest(void) { + AutoCurrentContext()->Initiate(); + } + + AutoRequired factory; +}; + +TEST_F(SatisfiabilityTest, MarkUnsatisfiableCalls) { + auto packet = factory->NewPacket(); + + // This filter shouldn't be called, because it expects a reference as an input + bool bRefCalled = false; + *packet += [&bRefCalled](const Decoration<0>& dec) { bRefCalled = true; }; + + // This one should be called because the input is a shared pointer + bool bSharedPtrCalled = false; + *packet += [&bSharedPtrCalled](std::shared_ptr> dec) { bSharedPtrCalled = true; }; + + packet->Unsatisfiable>(); + + ASSERT_FALSE(bRefCalled) << "Reference version should not have been called"; + ASSERT_TRUE(bSharedPtrCalled) << "Shared pointer version should have been called as a result of Unsatisfiable"; +} \ No newline at end of file From 73e4e175ff02e852ed88c994e5481875e5584cfb Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 11 Feb 2015 13:35:41 -0800 Subject: [PATCH 05/41] Bump version number --- version.cmake | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version.cmake b/version.cmake index 2ecac9f14..116908e6e 100644 --- a/version.cmake +++ b/version.cmake @@ -1 +1 @@ -set(autowiring_VERSION 0.4.3) +set(autowiring_VERSION 0.4.4) From d6a750d3ecea65bb0412c2f9088e06631df34187 Mon Sep 17 00:00:00 2001 From: Jonathan Marsden Date: Wed, 11 Feb 2015 15:24:30 -0800 Subject: [PATCH 06/41] Support autoboost::atomic_thread_fence with libstdc++ --- autowiring/C++11/boost_atomic.h | 1 + src/autowiring/BasicThread.cpp | 4 ---- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/autowiring/C++11/boost_atomic.h b/autowiring/C++11/boost_atomic.h index f10b09cf5..02ef8a6ca 100644 --- a/autowiring/C++11/boost_atomic.h +++ b/autowiring/C++11/boost_atomic.h @@ -5,6 +5,7 @@ namespace std { using autoboost::atomic; using autoboost::atomic_flag; + using autoboost::atomic_thread_fence; using autoboost::memory_order_relaxed; using autoboost::memory_order_consume; using autoboost::memory_order_acquire; diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index 561be85e2..d7b859aed 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -71,11 +71,7 @@ void BasicThread::DoRunLoopCleanup(std::shared_ptr&& ctxt, std::sha m_running = false; // Need to ensure that "stop" and "running" are actually updated in memory before we mark "complete" -#if autowiring_USE_LIBCXX std::atomic_thread_fence(std::memory_order_release); -#else - (std::lock_guard)state->m_lock; -#endif m_completed = true; // Tell our CoreRunnable parent that we're done to ensure that our reference count will be cleared. From 5ec43a931e1de39ddcf0e21331ffcd490a857b30 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 12 Feb 2015 13:21:09 -0800 Subject: [PATCH 07/41] Update with explicit constructor --- autowiring/AutoPacket.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/autowiring/AutoPacket.h b/autowiring/AutoPacket.h index ee503acd0..be79fb5a8 100644 --- a/autowiring/AutoPacket.h +++ b/autowiring/AutoPacket.h @@ -591,7 +591,7 @@ template bool AutoPacket::Get(const std::shared_ptr*& out) const { static_assert(!std::is_const::value, "Overload resolution selected an incorrect version of Get"); - const DecorationDisposition* pDisposition = GetDisposition(DecorationKey(auto_id::key())); + const DecorationDisposition* pDisposition = GetDisposition(DecorationKey(auto_id::key(), true, 0)); if (!pDisposition || pDisposition->m_decorations.size() != 1) { out = nullptr; return false; From 23bf5b847d5b99f1b8aa1207d5479a078e487e69 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 12 Feb 2015 14:43:10 -0800 Subject: [PATCH 08/41] Move definitions to source file --- autowiring/DispatchQueue.h | 34 ++-------- src/autowiring/DispatchQueue.cpp | 107 ++++++++++++++++++++----------- 2 files changed, 74 insertions(+), 67 deletions(-) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index c76b14e54..8ddbdc2ed 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -138,26 +138,13 @@ class DispatchQueue { /// Similar to DispatchEvent, but will attempt to dispatch all events currently queued /// /// The total number of events dispatched - int DispatchAllEvents(void) { - int retVal = 0; - while(DispatchEvent()) - retVal++; - return retVal; - } + int DispatchAllEvents(void); public: /// /// Explicit overload for already-constructed dispatch thunk types /// - void AddExisting(DispatchThunkBase* pBase) { - std::unique_lock lk(m_dispatchLock); - if(m_dispatchQueue.size() >= m_dispatchCap) - return; - - m_dispatchQueue.push_back(pBase); - m_queueUpdated.notify_all(); - OnPended(std::move(lk)); - } + void AddExisting(DispatchThunkBase* pBase); /// /// Recommends a point in time to wake up to check for events @@ -216,9 +203,7 @@ class DispatchQueue { /// /// Overload for absolute-time based delayed dispatch thunk /// - DispatchThunkDelayedExpression operator+=(std::chrono::steady_clock::time_point rhs) { - return DispatchThunkDelayedExpression(this, rhs); - } + DispatchThunkDelayedExpression operator+=(std::chrono::steady_clock::time_point rhs); /// /// Directly pends a delayed dispatch thunk @@ -226,18 +211,7 @@ class DispatchQueue { /// /// This overload will always succeed and does not consult the dispatch cap /// - void operator+=(DispatchThunkDelayed&& rhs) { - std::lock_guard lk(m_dispatchLock); - - m_delayedQueue.push(std::forward(rhs)); - if( - m_delayedQueue.top().GetReadyTime() == rhs.GetReadyTime() && - m_dispatchQueue.empty() - ) - // We're becoming the new next-to-execute entity, dispatch queue currently empty, trigger wakeup - // so our newly pended delay thunk is eventually processed. - m_queueUpdated.notify_all(); - } + void operator+=(DispatchThunkDelayed&& rhs); /// /// Generic overload which will pend an arbitrary dispatch type diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index cdf26e125..4c9feaaba 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -23,39 +23,6 @@ DispatchQueue::~DispatchQueue(void) { } } -void DispatchQueue::Abort(void) { - std::lock_guard lk(m_dispatchLock); - m_aborted = true; - - // Do not permit any more lambdas to be pended to our queue: - m_dispatchCap = 0; - - // Destroy the whole dispatch queue: - while(!m_dispatchQueue.empty()) { - delete m_dispatchQueue.front(); - m_dispatchQueue.pop_front(); - } - - // Wake up anyone who is still waiting: - m_queueUpdated.notify_all(); -} - -std::chrono::steady_clock::time_point -DispatchQueue::SuggestSoonestWakeupTimeUnsafe(std::chrono::steady_clock::time_point latestTime) const { - return - m_delayedQueue.empty() ? - - // Nothing in the queue, no way to suggest a shorter time - latestTime : - - // Return the shorter of the maximum wait time and the time of the queue ready--we don't want to tell the - // caller to wait longer than the limit of their interest. - std::min( - m_delayedQueue.top().GetReadyTime(), - latestTime - ); -} - void DispatchQueue::PromoteReadyEventsUnsafe(void) { // Move all ready elements out of the delayed queue and into the dispatch queue: for( @@ -63,8 +30,8 @@ void DispatchQueue::PromoteReadyEventsUnsafe(void) { !m_delayedQueue.empty() && m_delayedQueue.top().GetReadyTime() < now; m_delayedQueue.pop() ) - // This item's ready time has elapsed, we can add it to our dispatch queue now: - m_dispatchQueue.push_back(m_delayedQueue.top().Get()); + // This item's ready time has elapsed, we can add it to our dispatch queue now: + m_dispatchQueue.push_back(m_delayedQueue.top().Get()); } void DispatchQueue::DispatchEventUnsafe(std::unique_lock& lk) { @@ -75,7 +42,7 @@ void DispatchQueue::DispatchEventUnsafe(std::unique_lock& lk) { m_dispatchQueue.pop_front(); bool wasEmpty = m_dispatchQueue.empty(); lk.unlock(); - + MakeAtExit( [this, wasEmpty] { // If we emptied the queue, we'd like to reobtain the lock and tell everyone @@ -87,12 +54,78 @@ void DispatchQueue::DispatchEventUnsafe(std::unique_lock& lk) { (*thunk)(); } +void DispatchQueue::Abort(void) { + std::lock_guard lk(m_dispatchLock); + m_aborted = true; + + // Do not permit any more lambdas to be pended to our queue: + m_dispatchCap = 0; + + // Destroy the whole dispatch queue: + while(!m_dispatchQueue.empty()) { + delete m_dispatchQueue.front(); + m_dispatchQueue.pop_front(); + } + + // Wake up anyone who is still waiting: + m_queueUpdated.notify_all(); +} + bool DispatchQueue::DispatchEvent(void) { std::unique_lock lk(m_dispatchLock); - if(m_dispatchQueue.empty()) + if (m_dispatchQueue.empty()) return false; DispatchEventUnsafe(lk); return true; } +int DispatchQueue::DispatchAllEvents(void) { + int retVal = 0; + while(DispatchEvent()) + retVal++; + return retVal; +} + +void DispatchQueue::AddExisting(DispatchThunkBase* pBase) { + std::unique_lock lk(m_dispatchLock); + if(m_dispatchQueue.size() >= m_dispatchCap) + return; + + m_dispatchQueue.push_back(pBase); + m_queueUpdated.notify_all(); + OnPended(std::move(lk)); +} + +std::chrono::steady_clock::time_point +DispatchQueue::SuggestSoonestWakeupTimeUnsafe(std::chrono::steady_clock::time_point latestTime) const { + return + m_delayedQueue.empty() ? + + // Nothing in the queue, no way to suggest a shorter time + latestTime : + + // Return the shorter of the maximum wait time and the time of the queue ready--we don't want to tell the + // caller to wait longer than the limit of their interest. + std::min( + m_delayedQueue.top().GetReadyTime(), + latestTime + ); +} + +DispatchQueue::DispatchThunkDelayedExpression DispatchQueue::operator+=(std::chrono::steady_clock::time_point rhs) { + return DispatchThunkDelayedExpression(this, rhs); +} + +void DispatchQueue::operator+=(DispatchThunkDelayed&& rhs) { + std::lock_guard lk(m_dispatchLock); + + m_delayedQueue.push(std::forward(rhs)); + if( + m_delayedQueue.top().GetReadyTime() == rhs.GetReadyTime() && + m_dispatchQueue.empty() + ) + // We're becoming the new next-to-execute entity, dispatch queue currently empty, trigger wakeup + // so our newly pended delay thunk is eventually processed. + m_queueUpdated.notify_all(); +} \ No newline at end of file From 19d9340ec4598ada6b008d3c26c660ca2da5356f Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 12 Feb 2015 14:48:22 -0800 Subject: [PATCH 09/41] Promote delayed events that are ready when dispatching an event --- autowiring/DispatchQueue.h | 1 - src/autowiring/DispatchQueue.cpp | 6 +++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index 8ddbdc2ed..e0cd01523 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -230,4 +230,3 @@ class DispatchQueue { OnPended(std::move(lk)); } }; - diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 4c9feaaba..3f1aa396e 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -73,6 +73,10 @@ void DispatchQueue::Abort(void) { bool DispatchQueue::DispatchEvent(void) { std::unique_lock lk(m_dispatchLock); + + // Update queue with delayed events that are ready + PromoteReadyEventsUnsafe(); + if (m_dispatchQueue.empty()) return false; @@ -128,4 +132,4 @@ void DispatchQueue::operator+=(DispatchThunkDelayed&& rhs) { // We're becoming the new next-to-execute entity, dispatch queue currently empty, trigger wakeup // so our newly pended delay thunk is eventually processed. m_queueUpdated.notify_all(); -} \ No newline at end of file +} From d22a626f54d486fc37ac8cf6be4624c800fb7a28 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 12 Feb 2015 16:19:31 -0800 Subject: [PATCH 10/41] Deprecation of PromoteReadyEventsUnsafe, optimization of a control statement --- autowiring/DispatchQueue.h | 9 +++++++- src/autowiring/CoreThread.cpp | 5 +---- src/autowiring/DispatchQueue.cpp | 38 ++++++++++++++++++-------------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/autowiring/DispatchQueue.h b/autowiring/DispatchQueue.h index e0cd01523..f9cef3d71 100644 --- a/autowiring/DispatchQueue.h +++ b/autowiring/DispatchQueue.h @@ -63,7 +63,11 @@ class DispatchQueue { /// /// Moves all ready events from the delayed queue into the dispatch queue /// - void PromoteReadyEventsUnsafe(void); + /// True if at least one dispatcher was promoted + bool PromoteReadyDispatchersUnsafe(void); + + // Identical to PromoteReadyDispatchersUnsafe, invoke that method instead + void DEPRECATED(PromoteReadyEventsUnsafe(void), "Superceded by PromoteReadyDispatchersUnsafe") { PromoteReadyDispatchersUnsafe(); } /// /// Similar to DispatchEvent, except assumes that the dispatch lock is currently held @@ -132,6 +136,9 @@ class DispatchQueue { /// Similar to WaitForEvent, but does not block /// /// True if an event was dispatched, false if the queue was empty when checked + /// + /// If the dispatch queue is empty, this method will check the delayed dispatch queue. + /// bool DispatchEvent(void); /// diff --git a/src/autowiring/CoreThread.cpp b/src/autowiring/CoreThread.cpp index 3f748494d..85bad3001 100644 --- a/src/autowiring/CoreThread.cpp +++ b/src/autowiring/CoreThread.cpp @@ -84,11 +84,8 @@ bool CoreThread::WaitForEventUnsafe(std::unique_lock& lk, std::chron if(m_aborted) throw dispatch_aborted_exception(); - // Pull over any ready events: - PromoteReadyEventsUnsafe(); - // Dispatch events if the queue is now non-empty: - if(!m_dispatchQueue.empty()) + if (!PromoteReadyDispatchersUnsafe()) break; if(status == std::cv_status::timeout) diff --git a/src/autowiring/DispatchQueue.cpp b/src/autowiring/DispatchQueue.cpp index 3f1aa396e..a9c8e6f9d 100644 --- a/src/autowiring/DispatchQueue.cpp +++ b/src/autowiring/DispatchQueue.cpp @@ -2,6 +2,7 @@ #include "stdafx.h" #include "DispatchQueue.h" #include "at_exit.h" +#include dispatch_aborted_exception::dispatch_aborted_exception(void){} dispatch_aborted_exception::~dispatch_aborted_exception(void){} @@ -23,15 +24,19 @@ DispatchQueue::~DispatchQueue(void) { } } -void DispatchQueue::PromoteReadyEventsUnsafe(void) { +bool DispatchQueue::PromoteReadyDispatchersUnsafe(void) { // Move all ready elements out of the delayed queue and into the dispatch queue: + size_t nInitial = m_delayedQueue.size(); for( auto now = std::chrono::steady_clock::now(); !m_delayedQueue.empty() && m_delayedQueue.top().GetReadyTime() < now; m_delayedQueue.pop() ) - // This item's ready time has elapsed, we can add it to our dispatch queue now: - m_dispatchQueue.push_back(m_delayedQueue.top().Get()); + // This item's ready time has elapsed, we can add it to our dispatch queue now: + m_dispatchQueue.push_back(m_delayedQueue.top().Get()); + + // Something was promoted if the dispatch queue size is different + return nInitial != m_delayedQueue.size(); } void DispatchQueue::DispatchEventUnsafe(std::unique_lock& lk) { @@ -74,12 +79,13 @@ void DispatchQueue::Abort(void) { bool DispatchQueue::DispatchEvent(void) { std::unique_lock lk(m_dispatchLock); - // Update queue with delayed events that are ready - PromoteReadyEventsUnsafe(); - - if (m_dispatchQueue.empty()) + // If the queue is empty and we fail to promote anything, return here + // Note that, due to short-circuiting, promotion will not take place if the queue is not empty. + // This behavior is by design. + if (m_dispatchQueue.empty() && !PromoteReadyDispatchersUnsafe()) return false; + assert(!m_dispatchQueue.empty()); DispatchEventUnsafe(lk); return true; } @@ -104,17 +110,17 @@ void DispatchQueue::AddExisting(DispatchThunkBase* pBase) { std::chrono::steady_clock::time_point DispatchQueue::SuggestSoonestWakeupTimeUnsafe(std::chrono::steady_clock::time_point latestTime) const { return - m_delayedQueue.empty() ? + m_delayedQueue.empty() ? - // Nothing in the queue, no way to suggest a shorter time - latestTime : + // Nothing in the queue, no way to suggest a shorter time + latestTime : - // Return the shorter of the maximum wait time and the time of the queue ready--we don't want to tell the - // caller to wait longer than the limit of their interest. - std::min( - m_delayedQueue.top().GetReadyTime(), - latestTime - ); + // Return the shorter of the maximum wait time and the time of the queue ready--we don't want to tell the + // caller to wait longer than the limit of their interest. + std::min( + m_delayedQueue.top().GetReadyTime(), + latestTime + ); } DispatchQueue::DispatchThunkDelayedExpression DispatchQueue::operator+=(std::chrono::steady_clock::time_point rhs) { From 2c1bcd66e13c266be93400bc5d216f9f30f70184 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Thu, 12 Feb 2015 15:47:08 -0800 Subject: [PATCH 11/41] Test static new with arguments --- src/autowiring/test/AutowiringTest.cpp | 67 ++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/src/autowiring/test/AutowiringTest.cpp b/src/autowiring/test/AutowiringTest.cpp index 81a961836..808bf4403 100644 --- a/src/autowiring/test/AutowiringTest.cpp +++ b/src/autowiring/test/AutowiringTest.cpp @@ -111,3 +111,70 @@ TEST_F(AutowiringTest, CanAutowireCompletelyUndefinedType) { Autowired cut; ASSERT_EQ(nullptr, cut.get_unsafe()) << "Autowiring of a completely undefined type succeeded unexpectedly"; } + +class StaticNewInt; + +StaticNewInt* CreateStaticNewIntImpl(); +StaticNewInt* CreateStaticNewIntImpl(std::unique_ptr); + +class StaticNewInt: + public CoreObject +{ +protected: + StaticNewInt(void){} +public: + static StaticNewInt* New(std::unique_ptr value) { + return CreateStaticNewIntImpl(std::move(value)); + } + + static StaticNewInt* New(void) { + return CreateStaticNewIntImpl(); + } + + virtual int getValue(void) = 0; +}; + +class StaticNewIntImpl: + public StaticNewInt +{ +public: + StaticNewIntImpl(void): + m_value(new int(42)) + {} + StaticNewIntImpl(std::unique_ptr val): + m_value(std::move(val)) + {} + + int getValue(void) override { + return *m_value; + } + + std::unique_ptr m_value; +}; + +StaticNewInt* CreateStaticNewIntImpl(){ + return new StaticNewIntImpl(); +} +StaticNewInt* CreateStaticNewIntImpl(std::unique_ptr val){ + return new StaticNewIntImpl(std::move(val)); +} + +TEST_F(AutowiringTest, StaticNewWithArgs) { + const bool static_new = has_static_new>::value; + ASSERT_TRUE(static_new) << "has_static_new didn't correctly identify static New()"; + + { + AutoCreateContext ctxt; + ASSERT_NO_THROW(ctxt->Inject()) << "Exception throws while injecting member"; + AutowiredFast obj(ctxt); + EXPECT_EQ(42, obj->getValue()) << "Wrong constructor called"; + ctxt->SignalShutdown(true); + } + { + AutoCreateContext ctxt; + ASSERT_NO_THROW(auto obj = ctxt->Inject(std::unique_ptr(new int(1337)))) << "Exception throws while injecting member"; + AutowiredFast obj(ctxt); + EXPECT_EQ(1337, obj->getValue()) << "Wrong constructor called"; + ctxt->SignalShutdown(true); + } +} From 910954b5f2ec59f3a2fe0d79c4acb68d0ffd6997 Mon Sep 17 00:00:00 2001 From: Jimmy He Date: Mon, 16 Feb 2015 04:25:40 -0800 Subject: [PATCH 12/41] Fix overflow problems with QueryPerformanceCounter() by using doubles. This is the same way boost (and presumably VS2015) does it --- autowiring/C++11/chrono_with_profiling_clock.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/autowiring/C++11/chrono_with_profiling_clock.h b/autowiring/C++11/chrono_with_profiling_clock.h index ed7312455..172990a03 100644 --- a/autowiring/C++11/chrono_with_profiling_clock.h +++ b/autowiring/C++11/chrono_with_profiling_clock.h @@ -27,11 +27,11 @@ extern "C" { } namespace { - const long long g_Frequency = []() + const double g_nanosecs_per_tic = []() { int64_t frequency; QueryPerformanceFrequency(&reinterpret_cast(frequency)); - return frequency; + return 1e9 / static_cast(frequency); }(); } @@ -48,7 +48,7 @@ namespace std { static time_point now() { int64_t count; QueryPerformanceCounter(&reinterpret_cast(count)); - return time_point(duration(count * static_cast(period::den) / g_Frequency)); + return time_point(duration(static_cast(static_cast(count) * g_nanosecs_per_tic))); } }; } From b5ae7a4d6075fb4deca72f0c7888d19d91ee3ae6 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Tue, 17 Feb 2015 10:43:52 -0800 Subject: [PATCH 13/41] Make factory New() detection work correctly with move-only parameters --- autowiring/has_static_new.h | 7 ++----- src/autowiring/test/AutowiringTest.cpp | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/autowiring/has_static_new.h b/autowiring/has_static_new.h index 442181a6b..f5a99a473 100644 --- a/autowiring/has_static_new.h +++ b/autowiring/has_static_new.h @@ -11,7 +11,7 @@ template struct has_well_formed_static_new { static const bool value = std::is_convertible< - decltype(T::New(*(typename std::remove_reference::type*)nullptr...)), + decltype(T::New(std::forward(*(typename std::remove_reference::type*)nullptr)...)), T* >::value; }; @@ -24,11 +24,8 @@ struct has_well_formed_static_new { template struct has_static_new { - template - struct unnamed_constant; - template - static std::true_type select(decltype(U::New(*(typename std::remove_reference::type*)nullptr...))*); + static std::true_type select(decltype(U::New(std::forward(*(typename std::remove_reference::type*)nullptr)...))*); template static std::false_type select(...); diff --git a/src/autowiring/test/AutowiringTest.cpp b/src/autowiring/test/AutowiringTest.cpp index 808bf4403..48181d0b0 100644 --- a/src/autowiring/test/AutowiringTest.cpp +++ b/src/autowiring/test/AutowiringTest.cpp @@ -162,7 +162,7 @@ StaticNewInt* CreateStaticNewIntImpl(std::unique_ptr val){ TEST_F(AutowiringTest, StaticNewWithArgs) { const bool static_new = has_static_new>::value; ASSERT_TRUE(static_new) << "has_static_new didn't correctly identify static New()"; - + { AutoCreateContext ctxt; ASSERT_NO_THROW(ctxt->Inject()) << "Exception throws while injecting member"; From 8855d24e96910cb215db0a724f2822f16cd3999c Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 18 Feb 2015 08:03:10 -0800 Subject: [PATCH 14/41] Bumped version number in another spot --- Doxyfile | 2 +- publicDoxyfile.conf | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doxyfile b/Doxyfile index 9f7e2e134..26c828484 100644 --- a/Doxyfile +++ b/Doxyfile @@ -32,7 +32,7 @@ PROJECT_NAME = Autowiring # This could be handy for archiving the generated documentation or # if some version control system is used. -PROJECT_NUMBER = "0.4.1" +PROJECT_NUMBER = "0.4.2" # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer diff --git a/publicDoxyfile.conf b/publicDoxyfile.conf index 673058421..a5dfcb4b8 100644 --- a/publicDoxyfile.conf +++ b/publicDoxyfile.conf @@ -38,7 +38,7 @@ PROJECT_NAME = Autowiring # could be handy for archiving the generated documentation or if some version # control system is used. -PROJECT_NUMBER = 0.4.1 +PROJECT_NUMBER = 0.4.2 # Using the PROJECT_BRIEF tag one can provide an optional one line description # for a project that appears at the top of each page and should give viewer a From bf00dc533d57afcbfda13e8d6fe7439212d84d40 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 18 Feb 2015 08:11:42 -0800 Subject: [PATCH 15/41] Cleaning up build scripts and doxygen generation --- Doxyfile | 1 + scripts/buildpublicdocs.sh | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Doxyfile b/Doxyfile index 26c828484..8bea9e696 100644 --- a/Doxyfile +++ b/Doxyfile @@ -706,6 +706,7 @@ FILE_PATTERNS = "at_exit.h" \ "ContextCreator.h" \ "ContextCreatorBase.h" \ "ContextEnumerator.h" \ + ContextMap.h \ "ContextMember.h" \ "CoreContext.h" \ "CoreJob.h" \ diff --git a/scripts/buildpublicdocs.sh b/scripts/buildpublicdocs.sh index 926b86b3e..10597555b 100755 --- a/scripts/buildpublicdocs.sh +++ b/scripts/buildpublicdocs.sh @@ -4,9 +4,9 @@ # Run doxygen on the source and copy graphics # -cd .. +pushd $(dirname "$0")/.. scripts/processcodeexamples.sh doxygen publicDoxyFile.conf -echo "Copy svg files" +echo "Copying svg files" cp -v devguide/diagrams/*.svg docs/html -cd scripts +popd From 4514060c0a3754e09c176f8d95964f70801d2161 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Wed, 18 Feb 2015 13:29:05 -0800 Subject: [PATCH 16/41] Manually create AutowiringEvents AutowiringEvents is sometimes missing from the registry, causing problems for clients. --- src/autowiring/JunctionBoxManager.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/autowiring/JunctionBoxManager.cpp b/src/autowiring/JunctionBoxManager.cpp index d955247db..7ed8fc499 100644 --- a/src/autowiring/JunctionBoxManager.cpp +++ b/src/autowiring/JunctionBoxManager.cpp @@ -16,8 +16,7 @@ JunctionBoxManager::JunctionBoxManager(void) { m_junctionBoxes[p->ti] = p->NewJunctionBox(); // Make sure AutowiringEvents is in EventRegistry - assert(m_junctionBoxes.find(typeid(AutowiringEvents)) != m_junctionBoxes.end() - && "AutowiringEvents wasn't added to the event registry"); + m_junctionBoxes[typeid(AutowiringEvents)] = std::make_shared>(); // Always allow internal events m_junctionBoxes[typeid(AutowiringEvents)]->Initiate(); From 8e22638090e44c5d7b8aaf5dc14feefdc4a18ba8 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 19 Feb 2015 10:59:16 -0800 Subject: [PATCH 17/41] Simplification of deferrable strategy It should no longer be necessary to make use of a redundant slot argument in the update strategy. Other guarantees are being made right now to prevent a virtual function call from being made simultaneously with cancellation. --- autowiring/AutowirableSlot.h | 49 ++++++++++++---------------------- src/autowiring/CoreContext.cpp | 10 +++---- 2 files changed, 21 insertions(+), 38 deletions(-) diff --git a/autowiring/AutowirableSlot.h b/autowiring/AutowirableSlot.h index ec9ef1e3c..69d76ef92 100644 --- a/autowiring/AutowirableSlot.h +++ b/autowiring/AutowirableSlot.h @@ -11,25 +11,24 @@ class DeferrableAutowiring; class GlobalCoreContext; class CoreObject; -// Utility routine, for users who need a function that does nothing -template -void NullOp(T) {} - /// /// Strategy class for performing unsynchronized operations on an autowirable slot /// /// /// The DeferrableAutowiring base class' SatisfyAutowiring routine is guaranteed to be run in a /// synchronized context--IE, a call to CancelAutowiringNotification will block until the above -/// routines return. Unfortunately, this lock also excludes many other types of operations, such -/// as type search operations, which means that some of the work associated with cleaning up after -/// an autowiring has been satisfied will take place in an unsynchronized context. This means -/// that virtual function calls are generally unsafe on the member being autowired when they are -/// made without a lock being held. +/// routine returns. Unfortunately, this lock also excludes many other types of operations, such +/// as CoreContext::Inject and CoreContext::FindByType, which means that handing control to a user +/// specified callback is unsafe. /// -/// To mitigate this problem, instead of performing a virtual call through the original object, a -/// strategy type is provided by the DeferrableAutowiring while the lock is held, and then later the -/// strategy is employed to clean up the object, if necessary. +/// Exacerbating the problem is the fact that the original DeferrableAutowiring may refer to an +/// object on the stack or whose destruction cannot otherwise be delayed. As soon as the synchronized +/// context is exited, the object could already be in a teardown pathway, which means we can't invoke +/// any kind of virtual function call on the object. +/// +/// Thus, the Finalize operation is only supported on objects whose lifetimes can be externally +/// guaranteed. Currently, only AutowirableSlotFn supports this behavior, and it is accessable via +/// CoreContext::NotifyWhenAutowired and Autowired::NotifyWhenAutowired. /// class DeferrableUnsynchronizedStrategy { public: @@ -43,7 +42,7 @@ class DeferrableUnsynchronizedStrategy { /// outside of the context of a lock. Once this method returns, this object is guaranteed never /// to be referred to again by CoreContext. /// - virtual void Finalize(DeferrableAutowiring* pSlot) const = 0; + virtual void Finalize(void) = 0; }; /// @@ -103,7 +102,7 @@ class DeferrableAutowiring: /// /// If no custom strategy is required, this method may return null /// - virtual const DeferrableUnsynchronizedStrategy* GetStrategy(void) { return nullptr; } + virtual DeferrableUnsynchronizedStrategy* GetStrategy(void) { return nullptr; } /// /// @@ -204,24 +203,12 @@ class AutowirableSlot: /// template class AutowirableSlotFn: - public AutowirableSlot + public AutowirableSlot, + public DeferrableUnsynchronizedStrategy { static_assert(!std::is_same::value, "Do not attempt to autowire CoreContext. Instead, use AutoCurrentContext or AutoCreateContext"); static_assert(!std::is_same::value, "Do not attempt to autowire GlobalCoreContext. Instead, use AutoGlobalContext"); - class Strategy: - public DeferrableUnsynchronizedStrategy - { - public: - Strategy(void) {} - - void Finalize(DeferrableAutowiring* pfn) const override { - ((AutowirableSlotFn*) pfn)->Finalize(); - } - }; - - static const Strategy s_strategy; - public: AutowirableSlotFn(const std::shared_ptr& ctxt, Fn&& fn) : AutowirableSlot(ctxt), @@ -239,7 +226,7 @@ class AutowirableSlotFn: /// /// Finalization routine, called by our strategy /// - void Finalize(void) { + void Finalize(void) override { // Let the lambda execute as it sees fit: CallThroughObj(fn, &Fn::operator()); @@ -255,8 +242,6 @@ class AutowirableSlotFn: ); } - const DeferrableUnsynchronizedStrategy* GetStrategy(void) override { return &s_strategy; } + DeferrableUnsynchronizedStrategy* GetStrategy(void) override { return this; } }; -template -const typename AutowirableSlotFn::Strategy AutowirableSlotFn::s_strategy; diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index b3941a104..f0bb89c56 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -647,7 +647,7 @@ void CoreContext::CancelAutowiringNotification(DeferrableAutowiring* pDeferrable // Always finalize this entry: auto strategy = pDeferrable->GetStrategy(); if(strategy) - strategy->Finalize(pDeferrable); + strategy->Finalize(); // Stores the immediate predecessor of the node we will linearly scan for in our // linked list. @@ -735,7 +735,7 @@ void CoreContext::BroadcastContextCreationNotice(const std::type_info& sigil) co void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, const ObjectTraits& entry) { // Collection of satisfiable lists: - std::vector> satisfiable; + std::vector satisfiable; // Notify any autowired field whose autowiring was deferred. We do this by processing each entry // in the entire type memos collection. These entries are keyed on the type of the memo, and the @@ -790,9 +790,7 @@ void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, cons // are identified by an empty strategy, and we just skip them. auto strategy = pNext->GetStrategy(); if(strategy) - satisfiable.push_back( - std::make_pair(strategy, pNext) - ); + satisfiable.push_back(strategy); } } } @@ -816,7 +814,7 @@ void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, cons // Run through everything else and finalize it all: for(const auto& cur : satisfiable) - cur.first->Finalize(cur.second); + cur->Finalize(); } void CoreContext::AddEventReceiver(const JunctionBoxEntry& entry) { From 93688007b52e8a8d260be1a05e4e6b7e45d46cb1 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 19 Feb 2015 13:59:06 -0800 Subject: [PATCH 18/41] Correcting a flipped conditional This would cause a crash in cases where `m_queueUpdated` is spuriously signaled. --- src/autowiring/CoreThread.cpp | 4 +-- src/autowiring/test/CoreThreadTest.cpp | 40 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/autowiring/CoreThread.cpp b/src/autowiring/CoreThread.cpp index 85bad3001..2c857e1b2 100644 --- a/src/autowiring/CoreThread.cpp +++ b/src/autowiring/CoreThread.cpp @@ -84,8 +84,8 @@ bool CoreThread::WaitForEventUnsafe(std::unique_lock& lk, std::chron if(m_aborted) throw dispatch_aborted_exception(); - // Dispatch events if the queue is now non-empty: - if (!PromoteReadyDispatchersUnsafe()) + if (PromoteReadyDispatchersUnsafe()) + // Dispatcher is ready to run! Exit our loop and dispatch an event break; if(status == std::cv_status::timeout) diff --git a/src/autowiring/test/CoreThreadTest.cpp b/src/autowiring/test/CoreThreadTest.cpp index 78cfcf627..4807af208 100644 --- a/src/autowiring/test/CoreThreadTest.cpp +++ b/src/autowiring/test/CoreThreadTest.cpp @@ -516,4 +516,44 @@ TEST_F(CoreThreadTest, LightsOutPassiveCall) { // Verify no bad things happened: ASSERT_FALSE(mpc->bStartExcepted) << "Exception occurred during an attempt to elevate to synchronized level from CoreThread::OnStart"; ASSERT_FALSE(mpc->bStopExcepted) << "Exception occurred during an attempt to elevate to synchronized level from CoreThread::OnStop"; +} + +class CoreThreadExtraction: + public CoreThread +{ +public: + using CoreThread::m_queueUpdated; +}; + +TEST_F(CoreThreadTest, SpuriousWakeupTest) { + AutoCurrentContext()->Initiate(); + AutoRequired extraction; + + std::mutex lock; + std::condition_variable cv; + bool ready = false; + + auto wakeFn = [&] { + std::lock_guard lk(lock); + ready = true; + cv.notify_all(); + }; + + // Add a delayed lambda that we know won't launch and another one that should launch right away + *extraction += wakeFn; + *extraction += std::chrono::hours(1), [] {}; + + // Delay until the extraction lambda has run: + std::unique_lock lk(lock); + ASSERT_TRUE(cv.wait_for(lk, std::chrono::seconds(5), [&] { return ready; })); + + // Now force a spurious wakeup--this shouldn't technically be a problem + extraction->m_queueUpdated.notify_all(); + + // Delayed wake function, block for this to happen: + ready = false; + *extraction += std::chrono::milliseconds(1), wakeFn; + ASSERT_TRUE(cv.wait_for(lk, std::chrono::seconds(5), [&] { return ready; })); + + ASSERT_EQ(1UL, extraction->GetDispatchQueueLength()) << "Dispatch queue changed size under a spurious wakeup condition"; } \ No newline at end of file From 1cd65b64bc6122797e7ae07413a116ead4e9f851 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Thu, 19 Feb 2015 16:30:51 -0800 Subject: [PATCH 19/41] Provide a method to obtain the list of threads This can be handy for diagnostic cases where further information is desired about why a context didn't shut down properly. --- autowiring/CoreContext.h | 12 ++++++++++-- src/autowiring/CoreContext.cpp | 11 +++++++---- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index 3b8419c33..028e76cd1 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -253,8 +253,7 @@ class CoreContext: const std::shared_ptr m_junctionBoxManager; // Actual core threads: - typedef std::list t_threadList; - t_threadList m_threads; + std::list m_threads; // Clever use of shared pointer to expose the number of outstanding CoreRunnable instances. // Destructor does nothing; this is by design. @@ -517,6 +516,15 @@ class CoreContext: /// A shared reference to the parent context of this context. const std::shared_ptr& GetParentContext(void) const { return m_pParent; } + /// + /// Returns a copy of the list of runnables + /// + /// + /// This list is intended primarily for diagnostic purposes. The pointers are dumb pointers, and may be + /// invalidated if the caller is not careful to hold a shared pointer to the context. + /// + std::vector GetRunnables(void) const; + /// True if the sigil type of this CoreContext matches the specified sigil type. template bool Is(void) const { return GetSigilType() == typeid(Sigil); } diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index f0bb89c56..ce359275b 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -145,6 +145,11 @@ size_t CoreContext::GetChildCount(void) const { return m_children.size(); } +std::vector CoreContext::GetRunnables(void) const { + std::lock_guard lk(m_stateBlock->m_lock); + return std::vector(m_threads.begin(), m_threads.end()); +} + std::shared_ptr CoreContext::FirstChild(void) const { std::lock_guard lk(m_stateBlock->m_lock); @@ -381,8 +386,6 @@ void CoreContext::Initiate(void) { // Get the beginning of the thread list that we have at the time of lock acquisition // New threads are added to the front of the thread list, which means that objects // after this iterator are the ones that will need to be started - t_threadList::iterator beginning; - std::unique_lock lk(m_stateBlock->m_lock); // Now we can transition to initiated or running: @@ -431,7 +434,7 @@ void CoreContext::Initiate(void) { } // Now we can recover the first thread that will need to be started - beginning = m_threads.begin(); + auto beginning = m_threads.begin(); // Start our threads before starting any child contexts: lk.unlock(); @@ -452,7 +455,7 @@ void CoreContext::InitiateCoreThreads(void) { void CoreContext::SignalShutdown(bool wait, ShutdownMode shutdownMode) { // As we signal shutdown, there may be a CoreRunnable that is in the "running" state. If so, // then we will skip that thread as we signal the list of threads to shutdown. - t_threadList::iterator firstThreadToStop; + std::list::iterator firstThreadToStop; // Trivial return check if (IsShutdown()) From be2a4e20924d0fa48786c509345c2155f5e0bcb4 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 18 Feb 2015 07:59:57 -0800 Subject: [PATCH 20/41] Deprecate ContextCreator This type confusingly conflates the idea of context creation with context mapping. A separate type, ContextMap, already exists to function as a map between a key and a context, and should be preferred. Users should make use of `CoreContext::Create` or `AutoCreateContext` if they intend to create and manage a keyed set of contexts. --- autowiring/ContextCreator.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/autowiring/ContextCreator.h b/autowiring/ContextCreator.h index 78569027d..8a92aec5d 100644 --- a/autowiring/ContextCreator.h +++ b/autowiring/ContextCreator.h @@ -9,6 +9,7 @@ /// /// The sigil type that will be used for created contexts /// A key type used to identify this context +/// This class is obsolete, make use of instead /// /// This class helps manage the creation of contexts with global names. When the new child context /// is created, a notification is broadcast throughout the entire current context to any registered @@ -20,7 +21,7 @@ /// All static member functions are thread-safe, other members are not thread-safe. /// template -class ContextCreator: +class ContextCreator : public ContextCreatorBase { protected: @@ -32,6 +33,9 @@ class ContextCreator: typedef Key t_callbackHandle; public: + DEPRECATED(ContextCreator(void), "This type is deprecated, use manual context creation and ContextMap instead") + {} + // Accessor methods: size_t GetSize(void) const {return m_contexts.size();} From a91a843c158b8e6196c2c1d6ad4260ef006fce6d Mon Sep 17 00:00:00 2001 From: Jimmy Nguyen Date: Thu, 8 Jan 2015 08:40:20 -0800 Subject: [PATCH 21/41] Adding some documentation for AutoEnabled and CoreRunnable --- autowiring/Autowired.h | 3 ++- autowiring/CoreRunnable.h | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/autowiring/Autowired.h b/autowiring/Autowired.h index c0f0bb3ba..ec13671c7 100644 --- a/autowiring/Autowired.h +++ b/autowiring/Autowired.h @@ -338,7 +338,8 @@ class AutowiredFast: /// Enables the specified type to be "bolted" to the current context. /// /// -/// +/// Used to enable a boltable class in a context and can be used even if the context has not been created yet. In +/// this case, the class will be constructed and enabled when the context is created /// template class AutoEnable diff --git a/autowiring/CoreRunnable.h b/autowiring/CoreRunnable.h index 63ee78e3f..cff7d4ad2 100644 --- a/autowiring/CoreRunnable.h +++ b/autowiring/CoreRunnable.h @@ -7,7 +7,7 @@ class CoreObject; /// -/// Base class for objects that run threads. +/// Provides the interface for threads that should receive start and stop notifications in a context /// /// /// Users of Autowiring will typically use BasicThread or CoreThread instead of From 16317007adc8d0e07a9122ff2392a74f08cd2f40 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 05:13:17 -0800 Subject: [PATCH 22/41] Currently this feature only exists for Windows. Disable tests for other platforms until we get around to them. --- src/autowiring/test/CMakeLists.txt | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/autowiring/test/CMakeLists.txt b/src/autowiring/test/CMakeLists.txt index 02ac09258..46e191a83 100644 --- a/src/autowiring/test/CMakeLists.txt +++ b/src/autowiring/test/CMakeLists.txt @@ -5,7 +5,6 @@ set(AutowiringTest_SRCS AutoConstructTest.cpp AutoFilterCollapseRulesTest.cpp AutoFilterDiagnosticsTest.cpp - AutoFutureTest.cpp AutoInjectableTest.cpp AutoPacketTest.cpp AutoPacketFactoryTest.cpp @@ -61,6 +60,11 @@ set(AutowiringTest_SRCS UuidTest.cpp ) +add_windows_sources( + AutowiringTest_SRCS + AutoFutureTest.cpp +) + if(autowiring_USE_LIBCXX) set(AutowiringTest_SRCS ${AutowiringTest_SRCS} AutoFilterFunctionTest.cpp From 953946e558270598ec69cf12bb52ffedc25cdab1 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 05:22:06 -0800 Subject: [PATCH 23/41] Added missing copyright headers --- autowiring/auto_future.h | 1 + autowiring/auto_future_mac.h | 1 + autowiring/auto_future_win.h | 1 + src/autowiring/test/AutoFutureTest.cpp | 1 + 4 files changed, 4 insertions(+) diff --git a/autowiring/auto_future.h b/autowiring/auto_future.h index 928c10a08..e21d176cb 100644 --- a/autowiring/auto_future.h +++ b/autowiring/auto_future.h @@ -1,3 +1,4 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include diff --git a/autowiring/auto_future_mac.h b/autowiring/auto_future_mac.h index 6e1f5d240..2fe08af86 100644 --- a/autowiring/auto_future_mac.h +++ b/autowiring/auto_future_mac.h @@ -1,3 +1,4 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include #include diff --git a/autowiring/auto_future_win.h b/autowiring/auto_future_win.h index 02cb13d02..041d9d125 100644 --- a/autowiring/auto_future_win.h +++ b/autowiring/auto_future_win.h @@ -1,3 +1,4 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include #include diff --git a/src/autowiring/test/AutoFutureTest.cpp b/src/autowiring/test/AutoFutureTest.cpp index 26a567914..a0cadaa61 100644 --- a/src/autowiring/test/AutoFutureTest.cpp +++ b/src/autowiring/test/AutoFutureTest.cpp @@ -1,3 +1,4 @@ +// Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #include "stdafx.h" #include #include From 85d53e383a834d4f90492f5c9950211afe53d8e5 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 08:25:04 -0800 Subject: [PATCH 24/41] Adding a test to validate CoreThread::WaitFor --- src/autowiring/test/CoreThreadTest.cpp | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/autowiring/test/CoreThreadTest.cpp b/src/autowiring/test/CoreThreadTest.cpp index 4807af208..55a822938 100644 --- a/src/autowiring/test/CoreThreadTest.cpp +++ b/src/autowiring/test/CoreThreadTest.cpp @@ -556,4 +556,18 @@ TEST_F(CoreThreadTest, SpuriousWakeupTest) { ASSERT_TRUE(cv.wait_for(lk, std::chrono::seconds(5), [&] { return ready; })); ASSERT_EQ(1UL, extraction->GetDispatchQueueLength()) << "Dispatch queue changed size under a spurious wakeup condition"; +} + +TEST_F(CoreThreadTest, WaitForTimesOut) { + AutoCurrentContext ctxt; + ctxt->Initiate(); + + AutoCreateContext sub; + CurrentContextPusher pshr(sub); + AutoRequired ct; + ASSERT_FALSE(ct->WaitFor(std::chrono::microseconds(1))) << "A wait on a thread succeeded where it should have blocked indefinitely"; + + // Terminate outer context, thread should stop (eventually) + ctxt->SignalShutdown(); + ASSERT_TRUE(ct->WaitFor(std::chrono::seconds(5))) << "Outer scope termination did not correctly propagate to a child thread"; } \ No newline at end of file From 568417c3fffb61f582f40fed2375e280526488aa Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 08:42:36 -0800 Subject: [PATCH 25/41] Completion status moved into its own status block In the single context owner case, when the sole external holder of a `CoreContext` invokes `CoreContext::Wait`, that owner must hold the sole exclusive shared pointer to the context on return. This means that threads running in that context must not transition themselves to the "completed" state until after releasing all shared pointers to their enclosing context. In order to do this, the "completed" status flag has been moved to the context block, in order to allow us to update it even in the "lights out" countercase where the last holder of the context is a runnable in that same context. --- autowiring/BasicThread.h | 3 --- autowiring/BasicThreadStateBlock.h | 6 +++++- src/autowiring/BasicThread.cpp | 10 +++------- src/autowiring/BasicThreadStateBlock.cpp | 7 ++++++- 4 files changed, 14 insertions(+), 12 deletions(-) diff --git a/autowiring/BasicThread.h b/autowiring/BasicThread.h index a2a9927ac..26abe74e2 100644 --- a/autowiring/BasicThread.h +++ b/autowiring/BasicThread.h @@ -82,9 +82,6 @@ class BasicThread: // Run condition: bool m_running; - // Completion condition, true when this thread is no longer running and has run at least once - bool m_completed; - // The current thread priority ThreadPriority m_priority; diff --git a/autowiring/BasicThreadStateBlock.h b/autowiring/BasicThreadStateBlock.h index 05f801d69..44213d9f7 100644 --- a/autowiring/BasicThreadStateBlock.h +++ b/autowiring/BasicThreadStateBlock.h @@ -7,7 +7,8 @@ struct BasicThreadStateBlock: std::enable_shared_from_this { - ~BasicThreadStateBlock(); + BasicThreadStateBlock(void); + ~BasicThreadStateBlock(void); // General purpose thread lock and update condition for the lock std::mutex m_lock; @@ -15,4 +16,7 @@ struct BasicThreadStateBlock: // The current thread, if running std::thread m_thisThread; + + // Completion condition, true when this thread is no longer running and has run at least once + bool m_completed; }; diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index d7b859aed..40d3faa07 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -12,7 +12,6 @@ BasicThread::BasicThread(const char* pName): m_state(std::make_shared()), m_stop(false), m_running(false), - m_completed(false), m_priority(ThreadPriority::Default) {} @@ -70,10 +69,6 @@ void BasicThread::DoRunLoopCleanup(std::shared_ptr&& ctxt, std::sha m_stop = true; m_running = false; - // Need to ensure that "stop" and "running" are actually updated in memory before we mark "complete" - std::atomic_thread_fence(std::memory_order_release); - m_completed = true; - // Tell our CoreRunnable parent that we're done to ensure that our reference count will be cleared. Stop(false); @@ -97,6 +92,7 @@ void BasicThread::DoRunLoopCleanup(std::shared_ptr&& ctxt, std::sha // Notify other threads that we are done. At this point, any held references that might still exist // notification must happen from a synchronized level in order to ensure proper ordering. std::lock_guard lk(state->m_lock); + state->m_completed = true; state->m_stateCondition.notify_all(); } @@ -146,7 +142,7 @@ bool BasicThread::OnStart(void) { void BasicThread::OnStop(bool graceful) { // If we were never started, we need to set our completed flag to true if (!m_running) { - m_completed = true; + m_state->m_completed = true; } // Always invoke stop handler: @@ -158,7 +154,7 @@ void BasicThread::DoAdditionalWait(void) { std::unique_lock lk(m_state->m_lock); m_state->m_stateCondition.wait( lk, - [this] {return this->m_completed; } + [this] {return m_state->m_completed; } ); } diff --git a/src/autowiring/BasicThreadStateBlock.cpp b/src/autowiring/BasicThreadStateBlock.cpp index 1b42237ab..6b28dbe4c 100644 --- a/src/autowiring/BasicThreadStateBlock.cpp +++ b/src/autowiring/BasicThreadStateBlock.cpp @@ -2,4 +2,9 @@ #include "stdafx.h" #include "BasicThreadStateBlock.h" -BasicThreadStateBlock::~BasicThreadStateBlock(){} +BasicThreadStateBlock::BasicThreadStateBlock(void): + m_completed(false) +{} + +BasicThreadStateBlock::~BasicThreadStateBlock(void) +{} From b6a06badcb045ad5e0661bef43decbae5b7c740b Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 11:26:48 -0800 Subject: [PATCH 26/41] Adding a test to catch a factory new deduction error on Windows This issue has to do with the way that a string literal is type inducted in VC++. Attempting to do our nullptr-cast-to-ptr-dereference trick on decltype("string") causes problems because this decltype is understood to be const char (&)[], rather than const char*. --- autowiring/has_static_new.h | 6 +++++- src/autowiring/test/FactoryTest.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/autowiring/has_static_new.h b/autowiring/has_static_new.h index f5a99a473..b32302571 100644 --- a/autowiring/has_static_new.h +++ b/autowiring/has_static_new.h @@ -11,7 +11,11 @@ template struct has_well_formed_static_new { static const bool value = std::is_convertible< - decltype(T::New(std::forward(*(typename std::remove_reference::type*)nullptr)...)), + decltype( + T::New( + std::declval()... + ) + ), T* >::value; }; diff --git a/src/autowiring/test/FactoryTest.cpp b/src/autowiring/test/FactoryTest.cpp index 70f6efdf1..db43199be 100644 --- a/src/autowiring/test/FactoryTest.cpp +++ b/src/autowiring/test/FactoryTest.cpp @@ -155,3 +155,29 @@ TEST_F(FactoryTest, VerifyCanAutowireActualType) { ASSERT_TRUE(concrete.IsAutowired()) << "Failed to find the concrete derived type in a factory new construction in a case where it is known to exist"; } + +class AcceptsString: + public Object +{ +public: + AcceptsString(const char* str) : + str(str) + {} + + static AcceptsString* New(const char* pstrNamespace) { + return new AcceptsString(pstrNamespace); + } + + const char* const str; +}; + +TEST_F(FactoryTest, StringLiteralCheck) { + // This case can sometimes fail to compile because of the way we check for static new. + // String literals are interpreted to be a reference to an array, and taking the address of such a type can + // cause problems. + AutoCurrentContext()->Inject("Abcd"); + Autowired as; + + ASSERT_TRUE(as.IsAutowired()) << "Standard injection with an argument did not correctly forward the argument"; + ASSERT_STREQ("Abcd", as->str) << "Constructed type was not constructed properly"; +} \ No newline at end of file From 5b4680e98329324aa44594c68e0de9094b02ab21 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 09:28:11 -0800 Subject: [PATCH 27/41] Adding timed DoAdditionalWait variant Though the documentation does state that `DoAdditionalWait` should not block for long periods of time, we nevertheless have implementors that _do_ block. In order to prevent such cases from wreaking havoc and leaking out through our interfaces, a timed version of DoAdditionalWait is necessary. --- autowiring/AutoPacketFactory.h | 1 + autowiring/BasicThread.h | 6 +++- autowiring/CoreRunnable.h | 17 +++++++--- src/autowiring/AutoPacketFactory.cpp | 11 ++++++ src/autowiring/BasicThread.cpp | 17 ++++++++-- src/autowiring/CoreRunnable.cpp | 15 ++++----- src/autowiring/test/CoreThreadTest.cpp | 46 ++++++++++++++++++++++++++ 7 files changed, 96 insertions(+), 17 deletions(-) diff --git a/autowiring/AutoPacketFactory.h b/autowiring/AutoPacketFactory.h index 07fa3af59..e40b30042 100644 --- a/autowiring/AutoPacketFactory.h +++ b/autowiring/AutoPacketFactory.h @@ -75,6 +75,7 @@ class AutoPacketFactory: bool OnStart(void) override; void OnStop(bool graceful) override; void DoAdditionalWait(void) override; + bool DoAdditionalWait(std::chrono::nanoseconds timeout) override; /// /// Causes this AutoPacketFactory to release all of its packet subscribers diff --git a/autowiring/BasicThread.h b/autowiring/BasicThread.h index 26abe74e2..2f2b4b584 100644 --- a/autowiring/BasicThread.h +++ b/autowiring/BasicThread.h @@ -76,6 +76,9 @@ class BasicThread: // we're actually signalling this event after we free ourselves. const std::shared_ptr m_state; + // Flag indicating that this thread was started at some point + bool m_wasStarted; + // Flag indicating that we need to stop right now bool m_stop; @@ -217,7 +220,8 @@ class BasicThread: void OnStop(bool graceful) override; - void DoAdditionalWait() override; + void DoAdditionalWait(void) override; + bool DoAdditionalWait(std::chrono::nanoseconds timeout) override; public: /// diff --git a/autowiring/CoreRunnable.h b/autowiring/CoreRunnable.h index 63ee78e3f..1442043a9 100644 --- a/autowiring/CoreRunnable.h +++ b/autowiring/CoreRunnable.h @@ -62,13 +62,22 @@ class CoreRunnable { virtual void OnStop(bool graceful) {}; /// - /// Invoked when a Wait() call has been made. Override this method to perform - /// any actions required before this runnable enters the waiting state. + /// Invoked just before control is returned to the user. /// + /// The maximum amount of time to wait + /// True if the wait succeeded, false if a timeout occurred /// - /// This call should not block for an extended period of time. + /// This virtual method provides implementors with a way to add further constraints to the wait operation + /// beyond the condition variable held internally by this CoreRunnable. + /// + /// This method must return true if the timeout is indefinite. /// - virtual void DoAdditionalWait() {}; + virtual bool DoAdditionalWait(std::chrono::nanoseconds timeout) { return true; } + + /// + /// Untimed variant of DoAdditionalWait + /// + virtual void DoAdditionalWait(void) { } public: // Accessor methods: diff --git a/src/autowiring/AutoPacketFactory.cpp b/src/autowiring/AutoPacketFactory.cpp index 584fa4e8f..54994596e 100644 --- a/src/autowiring/AutoPacketFactory.cpp +++ b/src/autowiring/AutoPacketFactory.cpp @@ -128,6 +128,17 @@ void AutoPacketFactory::DoAdditionalWait(void) { ); } +bool AutoPacketFactory::DoAdditionalWait(std::chrono::nanoseconds timeout) { + std::unique_lock lk(m_lock); + return m_stateCondition.wait_for( + lk, + timeout, + [this]{ + return ShouldStop() && m_outstandingInternal.expired(); + } + ); +} + void AutoPacketFactory::Clear(void) { // Simple handoff to Stop is sufficient Stop(false); diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index 40d3faa07..c264b13e8 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -10,6 +10,7 @@ BasicThread::BasicThread(const char* pName): ContextMember(pName), m_state(std::make_shared()), + m_wasStarted(false), m_stop(false), m_running(false), m_priority(ThreadPriority::Default) @@ -23,6 +24,7 @@ std::mutex& BasicThread::GetLock(void) { void BasicThread::DoRun(std::shared_ptr&& refTracker) { assert(m_running); + m_wasStarted = true; // Make our own session current before we do anything else: CurrentContextPusher pusher(GetContext()); @@ -141,9 +143,8 @@ bool BasicThread::OnStart(void) { void BasicThread::OnStop(bool graceful) { // If we were never started, we need to set our completed flag to true - if (!m_running) { + if (!m_wasStarted) m_state->m_completed = true; - } // Always invoke stop handler: OnStop(); @@ -154,7 +155,17 @@ void BasicThread::DoAdditionalWait(void) { std::unique_lock lk(m_state->m_lock); m_state->m_stateCondition.wait( lk, - [this] {return m_state->m_completed; } + [this] { return m_state->m_completed; } + ); +} + +bool BasicThread::DoAdditionalWait(std::chrono::nanoseconds timeout) { + // Wait for the run loop cleanup to happen in DoRunLoopCleanup + std::unique_lock lk(m_state->m_lock); + return m_state->m_stateCondition.wait_for( + lk, + timeout, + [this] { return m_state->m_completed; } ); } diff --git a/src/autowiring/CoreRunnable.cpp b/src/autowiring/CoreRunnable.cpp index dbcb42b6f..581c878fd 100644 --- a/src/autowiring/CoreRunnable.cpp +++ b/src/autowiring/CoreRunnable.cpp @@ -59,25 +59,22 @@ void CoreRunnable::Stop(bool graceful) { void CoreRunnable::Wait(void) { std::unique_lock lk(m_lock); - m_cv.wait(lk, [this](){ return ShouldStop() && !IsRunning(); }); + m_cv.wait(lk, [this] { return ShouldStop() && !IsRunning(); }); DoAdditionalWait(); } bool CoreRunnable::WaitFor(std::chrono::nanoseconds timeout) { std::unique_lock lk(m_lock); - if (m_cv.wait_for(lk, timeout, [this](){ return ShouldStop() && !IsRunning(); })) { - DoAdditionalWait(); - return true; - } + + if (m_cv.wait_for(lk, timeout, [this] { return ShouldStop() && !IsRunning(); })) + return DoAdditionalWait(timeout); return false; } template bool CoreRunnable::WaitUntil(TimeType timepoint) { std::unique_lock lk(m_lock); - if (m_cv.wait_until(lk, timepoint, [this](){ return ShouldStop() && !IsRunning(); })) { - DoAdditionalWait(); - return true; - } + if (m_cv.wait_until(lk, timepoint, [this] { return ShouldStop() && !IsRunning(); })) + return DoAdditionalWait(timepoint - TimeType::now()); return false; } diff --git a/src/autowiring/test/CoreThreadTest.cpp b/src/autowiring/test/CoreThreadTest.cpp index 4807af208..c276bd7cb 100644 --- a/src/autowiring/test/CoreThreadTest.cpp +++ b/src/autowiring/test/CoreThreadTest.cpp @@ -556,4 +556,50 @@ TEST_F(CoreThreadTest, SpuriousWakeupTest) { ASSERT_TRUE(cv.wait_for(lk, std::chrono::seconds(5), [&] { return ready; })); ASSERT_EQ(1UL, extraction->GetDispatchQueueLength()) << "Dispatch queue changed size under a spurious wakeup condition"; +} +class BlocksInOnStop: + public CoreThread +{ +public: + bool is_waiting = false; + bool signal = false; + + void Run(void) { + // Let the run loop return. This triggers cleanup operations and ultimately causes OnStop + // to get called in our own thread context. + } + + bool Block(std::chrono::nanoseconds timeout) { + std::unique_lock lk(m_lock); + return m_cv.wait_for(lk, timeout, [this] {return is_waiting; }); + } + + void Continue(void) { + std::lock_guard lk(m_lock); + signal = true; + m_cv.notify_all(); + } + + void OnStop(void) override { + std::unique_lock lk(this->m_lock); + is_waiting = true; + m_cv.notify_all(); + m_cv.wait_for(lk, std::chrono::seconds(5), [this] { return signal; }); + } +}; + +TEST_F(CoreThreadTest, ContextWaitTimesOutInOnStop) { + AutoCurrentContext ctxt; + AutoRequired bios; + ctxt->Initiate(); + + // Wait for our pathological case to be waiting before we try shutting down the context + ASSERT_TRUE(bios->Block(std::chrono::seconds(5))) << "Blocking class failed to enter a blocked condition as expected"; + + ctxt->SignalShutdown(); + ASSERT_FALSE(bios->WaitFor(std::chrono::milliseconds(1))) << "Timed wait on a misbehaving CoreThread did not time out as expected"; + ASSERT_FALSE(ctxt->Wait(std::chrono::milliseconds(1))) << "Context wait routine did not timeout as expected"; + + // Let BIOS back out now: + bios->Continue(); } \ No newline at end of file From 0f4793af25554017439a1e3518aac2f04ef9d462 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 14:13:10 -0800 Subject: [PATCH 28/41] Added missing DoAdditionalWait override to CoreJob --- autowiring/CoreJob.h | 1 + src/autowiring/CoreJob.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/autowiring/CoreJob.h b/autowiring/CoreJob.h index 8e4f9576c..b61ece362 100644 --- a/autowiring/CoreJob.h +++ b/autowiring/CoreJob.h @@ -43,4 +43,5 @@ class CoreJob: bool OnStart(void) override; void OnStop(bool graceful) override; void DoAdditionalWait(void) override; + bool DoAdditionalWait(std::chrono::nanoseconds timeout) override; }; diff --git a/src/autowiring/CoreJob.cpp b/src/autowiring/CoreJob.cpp index 6b884ce5c..d84f21c66 100644 --- a/src/autowiring/CoreJob.cpp +++ b/src/autowiring/CoreJob.cpp @@ -118,3 +118,14 @@ void CoreJob::DoAdditionalWait(void) { m_curEvent = nullptr; } } + +bool CoreJob::DoAdditionalWait(std::chrono::nanoseconds timeout) { + if (!m_curEvent) + return true; + + std::future* ptr = static_cast*>(m_curEvent); + auto status = ptr->wait_for(timeout); + delete ptr; + m_curEvent = nullptr; + return status == std::future_status::ready; +} \ No newline at end of file From e5fde8d7f20ddb9cd1cc8dcbf05dfc2e95012ea3 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 14:36:09 -0800 Subject: [PATCH 29/41] Unit test behaviors extended --- src/autowiring/test/ContextCleanupTest.cpp | 2 ++ src/autowiring/test/DtorCorrectnessTest.cpp | 4 ++-- src/autowiring/test/ObjectPoolTest.cpp | 2 ++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/autowiring/test/ContextCleanupTest.cpp b/src/autowiring/test/ContextCleanupTest.cpp index 675a2a913..b80141957 100644 --- a/src/autowiring/test/ContextCleanupTest.cpp +++ b/src/autowiring/test/ContextCleanupTest.cpp @@ -153,7 +153,9 @@ TEST_F(ContextCleanupTest, VerifyGracefulThreadCleanup) { *ct += [called] { *called = true; }; // Verify that a graceful shutdown ensures both lambdas are called: + ASSERT_FALSE(ctxt->IsShutdown()) << "Context shut down prematurely"; ctxt->SignalShutdown(true, ShutdownMode::Graceful); + ASSERT_FALSE(ct->IsRunning()) << "Thread still reported as running even after a wait concluded"; ASSERT_TRUE(*called) << "Graceful shutdown did not correctly run down all lambdas"; } diff --git a/src/autowiring/test/DtorCorrectnessTest.cpp b/src/autowiring/test/DtorCorrectnessTest.cpp index adf4049db..6e30b5316 100644 --- a/src/autowiring/test/DtorCorrectnessTest.cpp +++ b/src/autowiring/test/DtorCorrectnessTest.cpp @@ -114,8 +114,8 @@ TEST_F(DtorCorrectnessTest, VerifyDeferringDtors) { listener2->Wait(); // Verify that we actually hit something: - EXPECT_TRUE(listener1->m_hitDeferred) << "Failed to hit a listener's deferred call"; - EXPECT_TRUE(listener2->m_hitDeferred) << "Failed to hit a listener's deferred call"; + EXPECT_TRUE(listener1->m_hitDeferred) << "Failed to hit listener #1's deferred call"; + EXPECT_TRUE(listener2->m_hitDeferred) << "Failed to hit listener #2's deferred call"; // Release all of our pointers: listener1.reset(); diff --git a/src/autowiring/test/ObjectPoolTest.cpp b/src/autowiring/test/ObjectPoolTest.cpp index fcf7f45f6..6f01c815a 100644 --- a/src/autowiring/test/ObjectPoolTest.cpp +++ b/src/autowiring/test/ObjectPoolTest.cpp @@ -367,6 +367,8 @@ TEST_F(ObjectPoolTest, MovableObjectPoolAysnc) { *AutoRequired() += [objs] {}; } + ASSERT_EQ(0, from.GetCached()) << "Initial pool received cached entities unexpectedly"; + // Kick off threads, then immediately and asynchronously move the pool: AutoCurrentContext()->Initiate(); ObjectPool to = std::move(from); From 7023dda7e127392f5e52f72e975a620a88cf6c35 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 14:45:39 -0800 Subject: [PATCH 30/41] DoAdditionalWait should not be called from a synchronous level --- src/autowiring/CoreRunnable.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/autowiring/CoreRunnable.cpp b/src/autowiring/CoreRunnable.cpp index 581c878fd..730d360f6 100644 --- a/src/autowiring/CoreRunnable.cpp +++ b/src/autowiring/CoreRunnable.cpp @@ -58,23 +58,28 @@ void CoreRunnable::Stop(bool graceful) { } void CoreRunnable::Wait(void) { - std::unique_lock lk(m_lock); - m_cv.wait(lk, [this] { return ShouldStop() && !IsRunning(); }); + { + std::unique_lock lk(m_lock); + m_cv.wait(lk, [this] { return ShouldStop() && !IsRunning(); }); + } DoAdditionalWait(); } bool CoreRunnable::WaitFor(std::chrono::nanoseconds timeout) { - std::unique_lock lk(m_lock); - - if (m_cv.wait_for(lk, timeout, [this] { return ShouldStop() && !IsRunning(); })) - return DoAdditionalWait(timeout); - return false; + { + std::unique_lock lk(m_lock); + if (!m_cv.wait_for(lk, timeout, [this] { return ShouldStop() && !IsRunning(); })) + return false; + } + return DoAdditionalWait(timeout); } template bool CoreRunnable::WaitUntil(TimeType timepoint) { - std::unique_lock lk(m_lock); - if (m_cv.wait_until(lk, timepoint, [this] { return ShouldStop() && !IsRunning(); })) - return DoAdditionalWait(timepoint - TimeType::now()); - return false; + { + std::unique_lock lk(m_lock); + if (!m_cv.wait_until(lk, timepoint, [this] { return ShouldStop() && !IsRunning(); })) + return false; + } + return DoAdditionalWait(timepoint - TimeType::now());; } From 12f2e3bbd6f9676b444c694c9254395b4196f66a Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 14:57:40 -0800 Subject: [PATCH 31/41] wasStarted should be set from the lambda, not in a potentially overrideable virtual function --- src/autowiring/BasicThread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index c264b13e8..866644adc 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -24,7 +24,6 @@ std::mutex& BasicThread::GetLock(void) { void BasicThread::DoRun(std::shared_ptr&& refTracker) { assert(m_running); - m_wasStarted = true; // Make our own session current before we do anything else: CurrentContextPusher pusher(GetContext()); @@ -126,8 +125,9 @@ bool BasicThread::OnStart(void) { if(!context) return false; - // Currently running: + // Currently running and started: m_running = true; + m_wasStarted = true; // Place the new thread entity directly in the space where it goes to avoid // any kind of races arising from asynchronous access to this space From 5709cc089356aefd987656178014db3ce07fb25e Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Fri, 20 Feb 2015 16:04:02 -0800 Subject: [PATCH 32/41] Add a "m_completed" reference bool for compatibility reasons This field is protected and some downstream consumers still make use of it. It needs to be left present until the next major version bump. --- autowiring/BasicThread.h | 8 ++++++++ src/autowiring/BasicThread.cpp | 5 +++++ 2 files changed, 13 insertions(+) diff --git a/autowiring/BasicThread.h b/autowiring/BasicThread.h index 2f2b4b584..5b2ec67c8 100644 --- a/autowiring/BasicThread.h +++ b/autowiring/BasicThread.h @@ -85,6 +85,9 @@ class BasicThread: // Run condition: bool m_running; + // Legacy field, some clients still refer to this + bool& DEPRECATED(m_completed, "Use IsCompleted instead"); + // The current thread priority ThreadPriority m_priority; @@ -224,6 +227,11 @@ class BasicThread: bool DoAdditionalWait(std::chrono::nanoseconds timeout) override; public: + /// + /// True if this thread has transitioned to a completed state + /// + bool IsCompleted(void) const; + /// /// Begins thread execution. /// diff --git a/src/autowiring/BasicThread.cpp b/src/autowiring/BasicThread.cpp index 866644adc..687c0c936 100644 --- a/src/autowiring/BasicThread.cpp +++ b/src/autowiring/BasicThread.cpp @@ -13,6 +13,7 @@ BasicThread::BasicThread(const char* pName): m_wasStarted(false), m_stop(false), m_running(false), + m_completed(m_state->m_completed), m_priority(ThreadPriority::Default) {} @@ -169,6 +170,10 @@ bool BasicThread::DoAdditionalWait(std::chrono::nanoseconds timeout) { ); } +bool BasicThread::IsCompleted(void) const { + return m_state->m_completed; +} + void BasicThread::ForceCoreThreadReidentify(void) { for(const auto& ctxt : ContextEnumerator(AutoGlobalContext())) { for(const auto& thread : ctxt->CopyBasicThreadList()) From caffeacffc49fab168d175272db62df3b49c1ca8 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Tue, 24 Feb 2015 15:31:45 -0800 Subject: [PATCH 33/41] Correcting invalid macro specification on Windows --- autowiring/BasicThread.h | 2 +- autowiring/C++11/cpp11.h | 10 ++-------- 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/autowiring/BasicThread.h b/autowiring/BasicThread.h index 5b2ec67c8..d9a11a686 100644 --- a/autowiring/BasicThread.h +++ b/autowiring/BasicThread.h @@ -86,7 +86,7 @@ class BasicThread: bool m_running; // Legacy field, some clients still refer to this - bool& DEPRECATED(m_completed, "Use IsCompleted instead"); + bool& DEPRECATED_MEMBER(m_completed, "Use IsCompleted instead"); // The current thread priority ThreadPriority m_priority; diff --git a/autowiring/C++11/cpp11.h b/autowiring/C++11/cpp11.h index 007bd67b4..089cd1270 100644 --- a/autowiring/C++11/cpp11.h +++ b/autowiring/C++11/cpp11.h @@ -325,20 +325,14 @@ /********************* * Deprecation convenience macro *********************/ -#ifndef _DEBUG - #ifdef _MSC_VER #define DEPRECATED(signature, msg) __declspec(deprecated(msg)) signature #define DEPRECATED_CLASS(classname, msg) __declspec(deprecated(msg)) classname +#define DEPRECATED_MEMBER(member, msg) member #else #define DEPRECATED(signature, msg) signature __attribute__((deprecated)) #define DEPRECATED_CLASS(classname, msg) -#endif - -#else -// In release mode we don't bother to deprecate -#define DEPRECATED(signature, msg) signature -#define DEPRECATED_CLASS(classname, msg) +#define DEPRECATED_MEMBER(member, msg) DEPRECATED(member, msg) #endif /********************* From ab3d3f7312addd84173c01f6d35deac7fbdc1a80 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Tue, 24 Feb 2015 15:33:12 -0800 Subject: [PATCH 34/41] Add default case is switch statements --- src/autowiring/AutoPacket.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/autowiring/AutoPacket.cpp b/src/autowiring/AutoPacket.cpp index 7b2c7b1c5..d14543acb 100644 --- a/src/autowiring/AutoPacket.cpp +++ b/src/autowiring/AutoPacket.cpp @@ -95,6 +95,8 @@ void AutoPacket::AddSatCounter(SatCounter& satCounter) { case DispositionState::Unsatisfiable: satCounter.Decrement(); break; + default: + break; } } if (pCur->is_output) { @@ -240,6 +242,8 @@ void AutoPacket::DecorateNoPriors(const AnySharedPointer& ptr, DecorationKey key throw std::runtime_error(ss.str()); } break; + default: + break; } // Decoration attaches here From e8e3f4aff16de1bbd7e4b6e334a1380e65a16b80 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Tue, 24 Feb 2015 16:23:10 -0800 Subject: [PATCH 35/41] Fix libstdc++ problems --- autowiring/C++11/boost_type_traits.h | 2 ++ autowiring/has_static_new.h | 1 + src/autowiring/AutoPacket.cpp | 4 ++-- src/autowiring/AutoPacketInternal.cpp | 5 +++++ 4 files changed, 10 insertions(+), 2 deletions(-) diff --git a/autowiring/C++11/boost_type_traits.h b/autowiring/C++11/boost_type_traits.h index dc0aa2467..540caccea 100644 --- a/autowiring/C++11/boost_type_traits.h +++ b/autowiring/C++11/boost_type_traits.h @@ -19,10 +19,12 @@ #include #include #include +#include namespace std { using autoboost::conditional; using autoboost::decay; + using autoboost::declval; using autoboost::false_type; using autoboost::has_trivial_constructor; using autoboost::integral_constant; diff --git a/autowiring/has_static_new.h b/autowiring/has_static_new.h index b32302571..f597c4db9 100644 --- a/autowiring/has_static_new.h +++ b/autowiring/has_static_new.h @@ -1,6 +1,7 @@ // Copyright (C) 2012-2015 Leap Motion, Inc. All rights reserved. #pragma once #include TYPE_TRAITS_HEADER +#include RVALUE_HEADER /// /// Utility helper structure for types which have a factory New routine diff --git a/src/autowiring/AutoPacket.cpp b/src/autowiring/AutoPacket.cpp index d14543acb..c2b8ab395 100644 --- a/src/autowiring/AutoPacket.cpp +++ b/src/autowiring/AutoPacket.cpp @@ -10,7 +10,7 @@ #include "SatCounter.h" #include #include -#include +#include RVALUE_HEADER using namespace autowiring; @@ -340,7 +340,7 @@ void AutoPacket::ForwardAll(std::shared_ptr recipient) const { // Copy decorations into an internal decorations maintenance collection. The values // in this collection are guaranteed to be stable in memory, and there are stable states // that can be relied upon without synchronization. - std::vector dd; + std::vector> dd; { std::lock_guard lk(m_lock); for (const auto& decoration : m_decorations) diff --git a/src/autowiring/AutoPacketInternal.cpp b/src/autowiring/AutoPacketInternal.cpp index a3e6a8183..525aaddcb 100644 --- a/src/autowiring/AutoPacketInternal.cpp +++ b/src/autowiring/AutoPacketInternal.cpp @@ -43,7 +43,12 @@ void AutoPacketInternal::Initialize(bool isFirstPacket) { call->GetCall()(call->GetAutoFilter(), *this); // First-call indicated by argumument type AutoPacket&: +#if autowiring_USE_LIBCXX for (bool is_shared : {false, true}) { +#else + for (int num = 0; num < 2; ++num) { + bool is_shared = (bool)num; +#endif std::unique_lock lk(m_lock); // Don't modify the decorations set if nobody expects an AutoPacket input From 05025e088753cd9e165ab1807504967b22a31303 Mon Sep 17 00:00:00 2001 From: Graham Tremper Date: Tue, 24 Feb 2015 16:57:12 -0800 Subject: [PATCH 36/41] Fix arm-linux problem Arm-linux has std::chrono, but doesn't have std::future. Functions in autoboost::future expect autoboost::chrono, so we need to convert on this platform --- src/autowiring/CoreJob.cpp | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/src/autowiring/CoreJob.cpp b/src/autowiring/CoreJob.cpp index d84f21c66..619e08874 100644 --- a/src/autowiring/CoreJob.cpp +++ b/src/autowiring/CoreJob.cpp @@ -4,6 +4,18 @@ #include "CoreContext.h" #include FUTURE_HEADER +// Arm doesn't have std::future, but does have std::chrono. We need to convert from std::chrono +// to autoboost::chrono when passing arguments to "std::future"(alias to autoboost::future) on arm. +#if autowiring_BUILD_ARM +autoboost::chrono::nanoseconds NanosecondsForFutureWait(const std::chrono::nanoseconds& time) { + return autoboost::chrono::nanoseconds(time.count()); +} +#else +std::chrono::nanoseconds NanosecondsForFutureWait(const std::chrono::nanoseconds& time) { + return time; +} +#endif + CoreJob::CoreJob(const char* name) : ContextMember(name), m_running(false), @@ -124,7 +136,7 @@ bool CoreJob::DoAdditionalWait(std::chrono::nanoseconds timeout) { return true; std::future* ptr = static_cast*>(m_curEvent); - auto status = ptr->wait_for(timeout); + auto status = ptr->wait_for(NanosecondsForFutureWait(timeout)); delete ptr; m_curEvent = nullptr; return status == std::future_status::ready; From 3a26ff64e4acdaf4ac6f5f77bd91688f6b68b5fd Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Tue, 24 Feb 2015 17:14:11 -0800 Subject: [PATCH 37/41] Adjust DEPRECATED macro to replicate original behavior If we don't do this, then downstream consumers which presently have "warnings as errors" turned on for their debug builds will have all of their code break when they try to run debug builds after minor version updates. --- autowiring/C++11/cpp11.h | 21 ++++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/autowiring/C++11/cpp11.h b/autowiring/C++11/cpp11.h index 089cd1270..8efa94198 100644 --- a/autowiring/C++11/cpp11.h +++ b/autowiring/C++11/cpp11.h @@ -325,16 +325,23 @@ /********************* * Deprecation convenience macro *********************/ -#ifdef _MSC_VER -#define DEPRECATED(signature, msg) __declspec(deprecated(msg)) signature -#define DEPRECATED_CLASS(classname, msg) __declspec(deprecated(msg)) classname -#define DEPRECATED_MEMBER(member, msg) member +#ifndef _DEBUG + #ifdef _MSC_VER + #define DEPRECATED(signature, msg) __declspec(deprecated(msg)) signature + #define DEPRECATED_CLASS(classname, msg) __declspec(deprecated(msg)) classname + #define DEPRECATED_MEMBER(member, msg) member + #else + #define DEPRECATED(signature, msg) signature __attribute__((deprecated)) + #define DEPRECATED_CLASS(classname, msg) classname + #define DEPRECATED_MEMBER(member, msg) DEPRECATED(member, msg) + #endif #else -#define DEPRECATED(signature, msg) signature __attribute__((deprecated)) -#define DEPRECATED_CLASS(classname, msg) -#define DEPRECATED_MEMBER(member, msg) DEPRECATED(member, msg) + #define DEPRECATED(signature, msg) signature + #define DEPRECATED_CLASS(classname, msg) classname + #define DEPRECATED_MEMBER(member, msg) member #endif + /********************* * Enum forward declaration convenience macro - VS2010 has a bad implementation *********************/ From a99546c0ecb30805213d48caf37ac9221a0b13e0 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Tue, 24 Feb 2015 17:48:55 -0800 Subject: [PATCH 38/41] Fixing a corner case in a self-reflexive cast initializer --- autowiring/fast_pointer_cast.h | 8 +++++++- src/autowiring/test/AnySharedPointerTest.cpp | 17 +++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/autowiring/fast_pointer_cast.h b/autowiring/fast_pointer_cast.h index d012f49ac..2e86c2519 100644 --- a/autowiring/fast_pointer_cast.h +++ b/autowiring/fast_pointer_cast.h @@ -79,7 +79,7 @@ namespace autowiring { /// template struct fast_pointer_cast_blind { - std::shared_ptr cast(const std::shared_ptr& rhs) { + static std::shared_ptr cast(const std::shared_ptr& rhs) { return rhs; } }; @@ -108,6 +108,12 @@ namespace autowiring { static const fast_pointer_cast_initializer sc_init; }; + // Trivial case specialization + template + struct fast_pointer_cast_initializer { + static const fast_pointer_cast_initializer sc_init; + }; + template const fast_pointer_cast_initializer fast_pointer_cast_initializer::sc_init; } diff --git a/src/autowiring/test/AnySharedPointerTest.cpp b/src/autowiring/test/AnySharedPointerTest.cpp index b54d12184..ac74e4e1b 100644 --- a/src/autowiring/test/AnySharedPointerTest.cpp +++ b/src/autowiring/test/AnySharedPointerTest.cpp @@ -191,3 +191,20 @@ TEST_F(AnySharedPointerTest, VoidReturnExpected) { // Validate equivalence of the void operator: ASSERT_EQ(v.get(), slot->ptr()) << "Shared pointer slot did not return a void* with an expected value"; } + +TEST_F(AnySharedPointerTest, CanHoldCoreObject) { + auto co = std::make_shared(); + + AnySharedPointer x = co; + ASSERT_EQ(co, x) << "Held CoreObject was not equivalent to constructed instance"; +} + +TEST_F(AnySharedPointerTest, CanFastCastToSelf) { + autowiring::fast_pointer_cast_initializer::sc_init; + + auto co = std::make_shared(); + ASSERT_EQ( + co, + autowiring::fast_pointer_cast(co) + ) << "Could not cast a CoreObject instance to itself"; +} \ No newline at end of file From a3c730f192d4fe86a54fd31bf0ab030bd4626b67 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 25 Feb 2015 08:22:48 -0800 Subject: [PATCH 39/41] ObjectTraits is now CoreObjectDescriptor We are preparing to make `CoreObjectDescriptor` a public interface, thus changing the name of this type is necessary. --- autowiring/AutoPacketGraph.h | 2 +- autowiring/AutowiringEvents.h | 4 ++-- autowiring/CoreContext.h | 18 +++++++++--------- .../{ObjectTraits.h => CoreObjectDescriptor.h} | 6 +++--- nuget/tools/autowiring.natvis | 4 ++-- src/autonet/AutoNetServerImpl.cpp | 4 ++-- src/autonet/AutoNetServerImpl.hpp | 4 ++-- src/autowiring/AutoPacketGraph.cpp | 2 +- src/autowiring/CMakeLists.txt | 2 +- src/autowiring/CoreContext.cpp | 10 +++++----- src/autowiring/test/ContextCleanupTest.cpp | 2 +- 11 files changed, 29 insertions(+), 29 deletions(-) rename autowiring/{ObjectTraits.h => CoreObjectDescriptor.h} (97%) diff --git a/autowiring/AutoPacketGraph.h b/autowiring/AutoPacketGraph.h index 1d97767e5..345b330a4 100644 --- a/autowiring/AutoPacketGraph.h +++ b/autowiring/AutoPacketGraph.h @@ -98,7 +98,7 @@ class AutoPacketGraph: /// AutowiringEvents overrides virtual void NewContext(CoreContext&) override {} virtual void ExpiredContext(CoreContext&) override {} - virtual void NewObject(CoreContext&, const ObjectTraits&) override; + virtual void NewObject(CoreContext&, const CoreObjectDescriptor&) override; /// CoreRunnable overrides virtual bool OnStart(void) override; diff --git a/autowiring/AutowiringEvents.h b/autowiring/AutowiringEvents.h index d1edba9ce..91f223242 100644 --- a/autowiring/AutowiringEvents.h +++ b/autowiring/AutowiringEvents.h @@ -3,7 +3,7 @@ #include "EventRegistry.h" #include TYPE_INDEX_HEADER -struct ObjectTraits; +struct CoreObjectDescriptor; class CoreContext; /// @@ -17,7 +17,7 @@ class AutowiringEvents { virtual void NewContext(CoreContext&)=0; virtual void ExpiredContext(CoreContext&)=0; - virtual void NewObject(CoreContext&, const ObjectTraits&)=0; + virtual void NewObject(CoreContext&, const CoreObjectDescriptor&)=0; }; // Extern explicit template instantiation declarations added to prevent diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index 028e76cd1..12dc0643e 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -16,7 +16,7 @@ #include "InvokeRelay.h" #include "JunctionBoxManager.h" #include "member_new_type.h" -#include "ObjectTraits.h" +#include "CoreObjectDescriptor.h" #include "result_or_default.h" #include "TeardownNotifier.h" #include "TypeRegistry.h" @@ -162,7 +162,7 @@ class CoreContext: DeferrableAutowiring* pFirst; // A back reference to the concrete type from which this memo was generated: - const ObjectTraits* pObjTraits; + const CoreObjectDescriptor* pObjTraits; // Once this memo entry is satisfied, this will contain the AnySharedPointer instance that performs // the satisfaction @@ -230,7 +230,7 @@ class CoreContext: class AutoFactoryFn; // This is a list of concrete types, indexed by the true type of each element. - std::list m_concreteTypes; + std::list m_concreteTypes; // This is a memoization map used to memoize any already-detected interfaces. mutable std::unordered_map m_typeMemos; @@ -327,7 +327,7 @@ class CoreContext: /// /// Invokes all deferred autowiring fields, generally called after a new member has been added /// - void UpdateDeferredElements(std::unique_lock&& lk, const ObjectTraits& entry); + void UpdateDeferredElements(std::unique_lock&& lk, const CoreObjectDescriptor& entry); /// \internal /// @@ -398,7 +398,7 @@ class CoreContext: /// /// Internal type introduction routine /// - void AddInternal(const ObjectTraits& traits); + void AddInternal(const CoreObjectDescriptor& traits); /// \internal /// @@ -454,7 +454,7 @@ class CoreContext: /// /// Forwarding routine, only removes from this context /// - void UnsnoopAutoPacket(const ObjectTraits& traits); + void UnsnoopAutoPacket(const CoreObjectDescriptor& traits); /// \internal /// @@ -671,7 +671,7 @@ class CoreContext: try { // Pass control to the insertion routine, which will handle injection from this point: - AddInternal(ObjectTraits(retVal, (T*)nullptr)); + AddInternal(CoreObjectDescriptor(retVal, (T*)nullptr)); } catch(autowiring_error&) { // We know why this exception occurred. It's because, while we were constructing our @@ -950,7 +950,7 @@ class CoreContext: /// template void Snoop(const std::shared_ptr& pSnooper) { - const ObjectTraits traits(pSnooper, (T*)nullptr); + const CoreObjectDescriptor traits(pSnooper, (T*)nullptr); // Add to collections of snoopers InsertSnooper(pSnooper); @@ -980,7 +980,7 @@ class CoreContext: /// template void Unsnoop(const std::shared_ptr& pSnooper) { - const ObjectTraits traits(pSnooper, (T*)nullptr); + const CoreObjectDescriptor traits(pSnooper, (T*)nullptr); RemoveSnooper(pSnooper); diff --git a/autowiring/ObjectTraits.h b/autowiring/CoreObjectDescriptor.h similarity index 97% rename from autowiring/ObjectTraits.h rename to autowiring/CoreObjectDescriptor.h index a0d3499e4..d6119fdf8 100644 --- a/autowiring/ObjectTraits.h +++ b/autowiring/CoreObjectDescriptor.h @@ -15,9 +15,9 @@ /// /// Mapping and extraction structure used to provide a runtime version of an Object-implementing shared pointer /// -struct ObjectTraits { +struct CoreObjectDescriptor { template - ObjectTraits(const std::shared_ptr& value, T*) : + CoreObjectDescriptor(const std::shared_ptr& value, T*) : type(typeid(T)), actual_type(typeid(*value)), stump(SlotInformationStump::s_stump), @@ -51,7 +51,7 @@ struct ObjectTraits { // The type of the passed pointer const std::type_info& type; - // The "actual type" used by Autowiring. This type may differ from ObjectTraits::type in cases + // The "actual type" used by Autowiring. This type may differ from CoreObjectDescriptor::type in cases // where a type unifier is used, or if the concrete type is defined in an external module--for // instance, by a class factory. const std::type_info& actual_type; diff --git a/nuget/tools/autowiring.natvis b/nuget/tools/autowiring.natvis index bbc235cd8..9f55a963c 100644 --- a/nuget/tools/autowiring.natvis +++ b/nuget/tools/autowiring.natvis @@ -40,7 +40,7 @@ - + [size = {_Mylast - _Myfirst}] _Mylast - _Myfirst @@ -51,7 +51,7 @@ - + [{((char*)type._M_data + 6 + (*(char*)type._M_data == 's')),sb}, uses = {((std::shared_ptr<Object>*)((SharedPointerSlot*)value.m_space)->m_space)->_Rep->_Uses}] ? diff --git a/src/autonet/AutoNetServerImpl.cpp b/src/autonet/AutoNetServerImpl.cpp index fddf4e81f..123f9ebbc 100644 --- a/src/autonet/AutoNetServerImpl.cpp +++ b/src/autonet/AutoNetServerImpl.cpp @@ -5,7 +5,7 @@ #include "autowiring.h" #include "AutoNetTransportHttp.hpp" #include "demangle.h" -#include "ObjectTraits.h" +#include "CoreObjectDescriptor.h" #include "EventRegistry.h" #include "TypeRegistry.h" #include @@ -170,7 +170,7 @@ void AutoNetServerImpl::ExpiredContext(CoreContext& oldCtxt){ }; } -void AutoNetServerImpl::NewObject(CoreContext& ctxt, const ObjectTraits& object){ +void AutoNetServerImpl::NewObject(CoreContext& ctxt, const CoreObjectDescriptor& object){ int contextID = ResolveContextID(&ctxt); *this += [this, object, contextID]{ diff --git a/src/autonet/AutoNetServerImpl.hpp b/src/autonet/AutoNetServerImpl.hpp index d4a0488d8..132a0dc85 100644 --- a/src/autonet/AutoNetServerImpl.hpp +++ b/src/autonet/AutoNetServerImpl.hpp @@ -14,7 +14,7 @@ #define BOOST_NO_CXX11_HDR_INITIALIZER_LIST #endif -struct ObjectTraits; +struct CoreObjectDescriptor; struct TypeIdentifierBase; // Protocol layer for AutoNet @@ -70,7 +70,7 @@ class AutoNetServerImpl: /// /// Context containing the object /// The object - virtual void NewObject(CoreContext& ctxt, const ObjectTraits& obj) override; + virtual void NewObject(CoreContext& ctxt, const CoreObjectDescriptor& obj) override; protected: /// diff --git a/src/autowiring/AutoPacketGraph.cpp b/src/autowiring/AutoPacketGraph.cpp index 3259bf791..087b7cd95 100644 --- a/src/autowiring/AutoPacketGraph.cpp +++ b/src/autowiring/AutoPacketGraph.cpp @@ -67,7 +67,7 @@ void AutoPacketGraph::RecordDelivery(const std::type_info* ti, const AutoFilterD itr->second++; } -void AutoPacketGraph::NewObject(CoreContext&, const ObjectTraits&) { +void AutoPacketGraph::NewObject(CoreContext&, const CoreObjectDescriptor&) { LoadEdges(); } diff --git a/src/autowiring/CMakeLists.txt b/src/autowiring/CMakeLists.txt index 0d625bd5f..f502fdc18 100644 --- a/src/autowiring/CMakeLists.txt +++ b/src/autowiring/CMakeLists.txt @@ -133,7 +133,7 @@ set(Autowiring_SRCS ObjectPool.h ObjectPoolMonitor.cpp ObjectPoolMonitor.h - ObjectTraits.h + CoreObjectDescriptor.h SatCounter.h SatCounter.cpp SharedPointerSlot.h diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index ce359275b..f126882e2 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -194,7 +194,7 @@ const std::type_info& CoreContext::GetAutoTypeId(const AnySharedPointer& ptr) co if (q == m_typeMemos.end() || !q->second.pObjTraits) throw autowiring_error("Attempted to obtain the true type of a shared pointer that was not a member of this context"); - const ObjectTraits* pObjTraits = q->second.pObjTraits; + const CoreObjectDescriptor* pObjTraits = q->second.pObjTraits; return pObjTraits->type; } @@ -237,7 +237,7 @@ std::shared_ptr CoreContext::IncrementOutstandingThreadCount(void) { return retVal; } -void CoreContext::AddInternal(const ObjectTraits& traits) { +void CoreContext::AddInternal(const CoreObjectDescriptor& traits) { { std::unique_lock lk(m_stateBlock->m_lock); @@ -324,7 +324,7 @@ CoreContext::MemoEntry& CoreContext::FindByTypeUnsafe(AnySharedPointer& referenc } // Resolve based on iterated dynamic casts for each concrete type: - const ObjectTraits* pObjTraits = nullptr; + const CoreObjectDescriptor* pObjTraits = nullptr; for(const auto& type : m_concreteTypes) { if(!reference->try_assign(*type.value)) // No match, try the next entry @@ -736,7 +736,7 @@ void CoreContext::BroadcastContextCreationNotice(const std::type_info& sigil) co m_pParent->BroadcastContextCreationNotice(sigil); } -void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, const ObjectTraits& entry) { +void CoreContext::UpdateDeferredElements(std::unique_lock&& lk, const CoreObjectDescriptor& entry) { // Collection of satisfiable lists: std::vector satisfiable; @@ -1045,7 +1045,7 @@ void CoreContext::TryTransitionChildrenState(void) { lk.unlock(); } -void CoreContext::UnsnoopAutoPacket(const ObjectTraits& traits) { +void CoreContext::UnsnoopAutoPacket(const CoreObjectDescriptor& traits) { { std::lock_guard lk(m_stateBlock->m_lock); diff --git a/src/autowiring/test/ContextCleanupTest.cpp b/src/autowiring/test/ContextCleanupTest.cpp index b80141957..f796121f5 100644 --- a/src/autowiring/test/ContextCleanupTest.cpp +++ b/src/autowiring/test/ContextCleanupTest.cpp @@ -74,7 +74,7 @@ TEST_F(ContextCleanupTest, VerifyContextDtor) { AutoRequired simple; objVerifier = simple; - // Each ObjectTraits instance holds 2 strong references to SimpleObject, as CoreObject type and as ContextMember type. + // Each CoreObjectDescriptor instance holds 2 strong references to SimpleObject, as CoreObject type and as ContextMember type. // One instance is held in CoreContext::m_concreteTypes and the other in the CoreContext::m_typeMemos. // Finally, once more reference is held by the shared_ptr inheritance of simple. EXPECT_EQ(5, objVerifier.use_count()) << "Unexpected number of references to a newly constructed object"; From f7b803b1173cf698e9df47520a5b9cd15f316459 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 25 Feb 2015 08:21:55 -0800 Subject: [PATCH 40/41] Add runtime variants to Snoop and Unsnoop This feature allows users to describe members to be added to a context in a runtime way. Lists of `CoreObjectDescriptor` instances may therefore be used to cross module boundaries and enable objects defined and implemented in other modules to snoop remotely defined contexts. --- autowiring/CoreContext.h | 59 +++++++++++++++---------------- autowiring/CoreObjectDescriptor.h | 16 ++++++++- src/autowiring/CoreContext.cpp | 41 ++++++++++++++++----- src/autowiring/test/SnoopTest.cpp | 24 ++++++++++++- 4 files changed, 99 insertions(+), 41 deletions(-) diff --git a/autowiring/CoreContext.h b/autowiring/CoreContext.h index 12dc0643e..9cb862fba 100644 --- a/autowiring/CoreContext.h +++ b/autowiring/CoreContext.h @@ -247,7 +247,7 @@ class CoreContext: t_rcvrSet m_delayedEventReceivers; // Context members from other contexts that have snooped this context - std::set m_snoopers; + std::set m_snoopers; // Manages events for this context. One JunctionBoxManager is shared between peer contexts const std::shared_ptr m_junctionBoxManager; @@ -432,13 +432,13 @@ class CoreContext: /// /// Adds a snooper to the snoopers set /// - void InsertSnooper(std::shared_ptr snooper); + void InsertSnooper(const AnySharedPointer& snooper); /// \internal /// /// Removes a snooper to the snoopers set /// - void RemoveSnooper(std::shared_ptr snooper); + void RemoveSnooper(const AnySharedPointer& snooper); /// \internal /// @@ -448,7 +448,7 @@ class CoreContext: /// This method has no effect if the passed value is presently a snooper in this context; the /// snooper collection must therefore be updated prior to the call to this method. /// - void UnsnoopEvents(CoreObject* snooper, const JunctionBoxEntry& traits); + void UnsnoopEvents(const AnySharedPointer& snooper, const JunctionBoxEntry& traits); /// \internal /// @@ -937,6 +937,11 @@ class CoreContext: /// The recipient of the event void FilterFiringException(const JunctionBoxBase* pProxy, CoreObject* pRecipient); + /// + /// Runtime version of Snoop + /// + void Snoop(const CoreObjectDescriptor& traits); + /// /// Registers the specified event receiver to receive messages broadcast within this context. /// @@ -950,18 +955,7 @@ class CoreContext: /// template void Snoop(const std::shared_ptr& pSnooper) { - const CoreObjectDescriptor traits(pSnooper, (T*)nullptr); - - // Add to collections of snoopers - InsertSnooper(pSnooper); - - // Add EventReceiver - if(traits.receivesEvents) - AddEventReceiver(JunctionBoxEntry(this, traits.pCoreObject)); - - // Add PacketSubscriber; - if(!traits.subscriber.empty()) - AddPacketSubscriber(traits.subscriber); + Snoop(CoreObjectDescriptor(pSnooper, (T*)nullptr)); } /// @@ -969,9 +963,19 @@ class CoreContext: /// template void Snoop(const Autowired& snooper) { - return Snoop(static_cast&>(snooper)); + Snoop( + CoreObjectDescriptor( + static_cast&>(snooper), + (T*)nullptr + ) + ); } + /// + /// Runtime version of Unsnoop + /// + void Unsnoop(const CoreObjectDescriptor& traits); + /// /// Unregisters an event receiver previously registered to receive snooped events /// @@ -980,19 +984,7 @@ class CoreContext: /// template void Unsnoop(const std::shared_ptr& pSnooper) { - const CoreObjectDescriptor traits(pSnooper, (T*)nullptr); - - RemoveSnooper(pSnooper); - - auto oSnooper = std::static_pointer_cast(pSnooper); - - // Cleanup if its an EventReceiver - if(traits.receivesEvents) - UnsnoopEvents(oSnooper.get(), JunctionBoxEntry(this, traits.pCoreObject)); - - // Cleanup if its a packet listener - if(!traits.subscriber.empty()) - UnsnoopAutoPacket(traits); + Unsnoop(CoreObjectDescriptor(pSnooper, (T*)nullptr)); } /// @@ -1000,7 +992,12 @@ class CoreContext: /// template void Unsnoop(const Autowired& snooper) { - return Unsnoop(static_cast&>(snooper)); + Unsnoop( + CoreObjectDescriptor( + static_cast&>(snooper), + (T*)nullptr + ) + ); } /// diff --git a/autowiring/CoreObjectDescriptor.h b/autowiring/CoreObjectDescriptor.h index d6119fdf8..b5629d179 100644 --- a/autowiring/CoreObjectDescriptor.h +++ b/autowiring/CoreObjectDescriptor.h @@ -15,8 +15,14 @@ /// /// Mapping and extraction structure used to provide a runtime version of an Object-implementing shared pointer /// +/// +/// This type is used to describe an object that is a member of a context. This descriptor structure performs +/// all of the type rule induction work that CoreContext performs, and provides users with a way to provide a +/// runtime description of a type. This is critical in cases where a type is not available at compile time-- +/// for instance, if that type is provided in another module that is dynamically linked. +/// struct CoreObjectDescriptor { - template + template CoreObjectDescriptor(const std::shared_ptr& value, T*) : type(typeid(T)), actual_type(typeid(*value)), @@ -48,6 +54,14 @@ struct CoreObjectDescriptor { (void) autowiring::fast_pointer_cast_initializer::sc_init; } + /// + /// Special case where the declared type is also the true type + /// + template + CoreObjectDescriptor(const std::shared_ptr& value) : + CoreObjectDescriptor(value, static_cast(nullptr)) + {} + // The type of the passed pointer const std::type_info& type; diff --git a/src/autowiring/CoreContext.cpp b/src/autowiring/CoreContext.cpp index f126882e2..651dd54fa 100644 --- a/src/autowiring/CoreContext.cpp +++ b/src/autowiring/CoreContext.cpp @@ -871,13 +871,13 @@ void CoreContext::RemoveEventReceivers(const t_rcvrSet& receivers) { m_pParent->RemoveEventReceivers(receivers); } -void CoreContext::UnsnoopEvents(CoreObject* oSnooper, const JunctionBoxEntry& receiver) { +void CoreContext::UnsnoopEvents(const AnySharedPointer& snooper, const JunctionBoxEntry& receiver) { { std::lock_guard lk(m_stateBlock->m_lock); if( // If the passed value is currently a snooper, then the caller has snooped a context and also // one of its parents. End here. - m_snoopers.count(oSnooper) || + m_snoopers.count(snooper) || // If we are an event receiver in this context, then the caller has snooped a context and // is also a proper EventReceiver in one of that context's ancestors--this is a fairly @@ -894,7 +894,7 @@ void CoreContext::UnsnoopEvents(CoreObject* oSnooper, const JunctionBoxEntryUnsnoopEvents(oSnooper, receiver); + m_pParent->UnsnoopEvents(snooper, receiver); } void CoreContext::FilterException(void) { @@ -938,6 +938,31 @@ void CoreContext::FilterFiringException(const JunctionBoxBase* pProxy, CoreObjec } } +void CoreContext::Snoop(const CoreObjectDescriptor& traits) { + // Add to collections of snoopers + InsertSnooper(traits.value); + + // Add EventReceiver + if (traits.receivesEvents) + AddEventReceiver(JunctionBoxEntry(this, traits.pCoreObject)); + + // Add PacketSubscriber; + if (!traits.subscriber.empty()) + AddPacketSubscriber(traits.subscriber); +} + +void CoreContext::Unsnoop(const CoreObjectDescriptor& traits) { + RemoveSnooper(traits.value); + + // Cleanup if its an EventReceiver + if (traits.receivesEvents) + UnsnoopEvents(traits.value, JunctionBoxEntry(this, traits.pCoreObject)); + + // Cleanup if its a packet listener + if (!traits.subscriber.empty()) + UnsnoopAutoPacket(traits); +} + void CoreContext::AddDeferredUnsafe(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 @@ -956,14 +981,14 @@ void CoreContext::AddDeferredUnsafe(DeferrableAutowiring* deferrable) { entry.pFirst = deferrable; } -void CoreContext::InsertSnooper(std::shared_ptr snooper) { +void CoreContext::InsertSnooper(const AnySharedPointer& snooper) { (std::lock_guard)m_stateBlock->m_lock, - m_snoopers.insert(snooper.get()); + m_snoopers.insert(snooper); } -void CoreContext::RemoveSnooper(std::shared_ptr snooper) { +void CoreContext::RemoveSnooper(const AnySharedPointer& snooper) { (std::lock_guard)m_stateBlock->m_lock, - m_snoopers.erase(snooper.get()); + m_snoopers.erase(snooper); } void CoreContext::AddContextMember(const std::shared_ptr& ptr) { @@ -1051,7 +1076,7 @@ void CoreContext::UnsnoopAutoPacket(const CoreObjectDescriptor& traits) { // If the passed value is currently a snooper, then the caller has snooped a context and also // one of its parents. End here. - if ( m_snoopers.count(traits.pCoreObject.get()) ) + if (m_snoopers.count(traits.value)) return; } diff --git a/src/autowiring/test/SnoopTest.cpp b/src/autowiring/test/SnoopTest.cpp index 8e17c4102..473019e97 100644 --- a/src/autowiring/test/SnoopTest.cpp +++ b/src/autowiring/test/SnoopTest.cpp @@ -319,7 +319,29 @@ TEST_F(SnoopTest, CanSnoopAutowired) { AutoCurrentContext ctxt; ctxt->Inject(); - // Now autowire what we injeced and verify we can snoop this directly + // Now autowire what we injected and verify we can snoop this directly Autowired so; ctxt->Snoop(so); } + +TEST_F(SnoopTest, RuntimeSnoopCall) { + AutoCurrentContext ctxt; + ctxt->Initiate(); + + auto x = std::make_shared(); + CoreObjectDescriptor traits(x); + + // Try to snoop and verify that the snooped member gets an event: + ctxt->Snoop(x); + + AutoFired ubl; + ubl(&UpBroadcastListener::SimpleCall)(); + + ASSERT_TRUE(x->m_simpleCall) << "Runtime snoop call did not correctly register a child context member"; + ASSERT_EQ(1UL, x->m_callCount) << "Call count to a child member was incorrect"; + + // And the alternative variant next: + ctxt->Unsnoop(x); + ubl(&UpBroadcastListener::SimpleCall)(); + ASSERT_EQ(1UL, x->m_callCount) << "Snoop method was invoked after the related type was removed"; +} \ No newline at end of file From 7fc68a451bade4e9303a91af4f37dfceff44e094 Mon Sep 17 00:00:00 2001 From: Jason Lokerson Date: Wed, 25 Feb 2015 12:58:21 -0800 Subject: [PATCH 41/41] Add a cmakedefine01 for autowiring_BUILD_ARM --- AutowiringConfig.h.in | 3 +++ 1 file changed, 3 insertions(+) diff --git a/AutowiringConfig.h.in b/AutowiringConfig.h.in index 30dcab4bc..43d230fc6 100644 --- a/AutowiringConfig.h.in +++ b/AutowiringConfig.h.in @@ -8,6 +8,9 @@ // Are we building autonet? #cmakedefine01 AUTOWIRING_BUILD_AUTONET +// Building for ARM? +#cmakedefine01 autowiring_BUILD_ARM + // Are we linking with C++11 STL? #cmakedefine01 autowiring_USE_LIBCXX #if autowiring_USE_LIBCXX