From 24f3f832a72c0d20de366527ace8b2573937b3ef Mon Sep 17 00:00:00 2001 From: "Billy O'Neal (VC LIBS)" Date: Tue, 26 Mar 2019 12:54:17 -0700 Subject: [PATCH 1/2] Fix data race, GitHub #1085 --- Release/src/pplx/pplx.cpp | 59 +++++++++++++++++---------------------- 1 file changed, 26 insertions(+), 33 deletions(-) diff --git a/Release/src/pplx/pplx.cpp b/Release/src/pplx/pplx.cpp index 5f9626edd8..10a833a54d 100644 --- a/Release/src/pplx/pplx.cpp +++ b/Release/src/pplx/pplx.cpp @@ -14,13 +14,8 @@ #include "stdafx.h" #if !defined(_WIN32) || CPPREST_FORCE_PPLX - #include "pplx/pplx.h" - -// Disable false alarm code analyze warning -#if defined(_MSC_VER) -#pragma warning(disable : 26165 26110) -#endif +#include namespace pplx { @@ -36,24 +31,16 @@ class _Spin_lock void lock() { - if (details::atomic_compare_exchange(_M_lock, 1l, 0l) != 0l) + while (_M_lock.test_and_set()) { - do - { - pplx::details::platform::YieldExecution(); - - } while (details::atomic_compare_exchange(_M_lock, 1l, 0l) != 0l); + pplx::details::platform::YieldExecution(); } } - void unlock() - { - // fence for release semantics - details::atomic_exchange(_M_lock, 0l); - } + void unlock() { _M_lock.clear(); } private: - atomic_long _M_lock; + std::atomic_flag _M_lock; }; typedef ::pplx::scoped_lock<_Spin_lock> _Scoped_spin_lock; @@ -63,44 +50,49 @@ static struct _pplx_g_sched_t { typedef std::shared_ptr sched_ptr; - _pplx_g_sched_t() { m_state = post_ctor; } + _pplx_g_sched_t() { m_state.store(post_ctor, memory_order_relaxed); } - ~_pplx_g_sched_t() { m_state = post_dtor; } + ~_pplx_g_sched_t() { m_state.store(post_dtor, memory_order_relaxed); } sched_ptr get_scheduler() { - switch (m_state) + sched_ptr result; + switch (m_state.load(memory_order_relaxed)) { case post_ctor: // This is the 99.9% case. - - if (!m_scheduler) { ::pplx::details::_Scoped_spin_lock lock(m_spinlock); if (!m_scheduler) { m_scheduler = std::make_shared<::pplx::default_scheduler_t>(); } - } - return m_scheduler; + result = m_scheduler; + } // unlock + + break; default: // This case means the global m_scheduler is not available. // We spin off an individual scheduler instead. - return std::make_shared<::pplx::default_scheduler_t>(); + result = std::make_shared<::pplx::default_scheduler_t>(); + break; } + + return result; } void set_scheduler(sched_ptr scheduler) { - if (m_state == pre_ctor || m_state == post_dtor) + const auto localState = m_state.load(memory_order_relaxed); + if (localState == pre_ctor || localState == post_dtor) { throw invalid_operation("Scheduler cannot be initialized now"); } ::pplx::details::_Scoped_spin_lock lock(m_spinlock); - if (m_scheduler != nullptr) + if (m_scheduler) { throw invalid_operation("Scheduler is already initialized"); } @@ -108,14 +100,15 @@ static struct _pplx_g_sched_t m_scheduler = std::move(scheduler); } - enum + enum m_state_values { - pre_ctor = 0, - post_ctor = 1, - post_dtor = 2 - } m_state; + pre_ctor, + post_ctor, + post_dtor + }; private: + atomic m_state; pplx::details::_Spin_lock m_spinlock; sched_ptr m_scheduler; } _pplx_g_sched; From e37b6979a85896c03e0d28dc3576fd6e6b0ff5fa Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Tue, 26 Mar 2019 13:52:45 -0700 Subject: [PATCH 2/2] Fix *nix typos. --- Release/src/pplx/pplx.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/Release/src/pplx/pplx.cpp b/Release/src/pplx/pplx.cpp index 10a833a54d..618c02b908 100644 --- a/Release/src/pplx/pplx.cpp +++ b/Release/src/pplx/pplx.cpp @@ -50,14 +50,14 @@ static struct _pplx_g_sched_t { typedef std::shared_ptr sched_ptr; - _pplx_g_sched_t() { m_state.store(post_ctor, memory_order_relaxed); } + _pplx_g_sched_t() { m_state.store(post_ctor, std::memory_order_relaxed); } - ~_pplx_g_sched_t() { m_state.store(post_dtor, memory_order_relaxed); } + ~_pplx_g_sched_t() { m_state.store(post_dtor, std::memory_order_relaxed); } sched_ptr get_scheduler() { sched_ptr result; - switch (m_state.load(memory_order_relaxed)) + switch (m_state.load(std::memory_order_relaxed)) { case post_ctor: // This is the 99.9% case. @@ -84,7 +84,7 @@ static struct _pplx_g_sched_t void set_scheduler(sched_ptr scheduler) { - const auto localState = m_state.load(memory_order_relaxed); + const auto localState = m_state.load(std::memory_order_relaxed); if (localState == pre_ctor || localState == post_dtor) { throw invalid_operation("Scheduler cannot be initialized now"); @@ -108,7 +108,7 @@ static struct _pplx_g_sched_t }; private: - atomic m_state; + std::atomic m_state; pplx::details::_Spin_lock m_spinlock; sched_ptr m_scheduler; } _pplx_g_sched;