From 1a9ecfe20f60f5d3a63f25e266767d9322700247 Mon Sep 17 00:00:00 2001 From: K-ballo Date: Tue, 19 May 2015 17:03:01 -0500 Subject: [PATCH] Make sure sync primitives are not copyable nor movable Fixes #1471 --- hpx/config/emulate_deleted.hpp | 10 ++-- hpx/lcos/local/mutex.hpp | 6 ++- hpx/lcos/local/once.hpp | 3 +- hpx/lcos/local/spinlock.hpp | 25 +--------- hpx/lcos/local/spinlock_no_backoff.hpp | 24 +--------- hpx/lcos/local/spinlock_pool.hpp | 3 +- hpx/util/spinlock.hpp | 12 +++-- hpx/util/spinlock_pool.hpp | 11 +++-- .../performance/local/spinlock_overhead1.cpp | 5 +- .../performance/local/spinlock_overhead2.cpp | 5 +- tests/unit/lcos/CMakeLists.txt | 46 +++++++++++++++++++ tests/unit/lcos/fail_compile_mutex_move.cpp | 19 ++++++++ .../unit/lcos/fail_compile_spinlock_move.cpp | 19 ++++++++ .../fail_compile_spinlock_no_backoff_move.cpp | 19 ++++++++ 14 files changed, 141 insertions(+), 66 deletions(-) create mode 100644 tests/unit/lcos/fail_compile_mutex_move.cpp create mode 100644 tests/unit/lcos/fail_compile_spinlock_move.cpp create mode 100644 tests/unit/lcos/fail_compile_spinlock_no_backoff_move.cpp diff --git a/hpx/config/emulate_deleted.hpp b/hpx/config/emulate_deleted.hpp index 882beffc237a..83c44f6717f3 100644 --- a/hpx/config/emulate_deleted.hpp +++ b/hpx/config/emulate_deleted.hpp @@ -6,9 +6,9 @@ #if !defined(HPX_CONFIG_EMULATE_DELETED_JAN_06_2013_0919PM) #define HPX_CONFIG_EMULATE_DELETED_JAN_06_2013_0919PM -#include +#include -#if !defined(BOOST_NO_CXX11_DELETED_FUNCTIONS) +#ifdef HPX_HAVE_CXX11_DELETED_FUNCTIONS #define HPX_DELETE_COPY_CTOR(cls) \ cls(cls const&) = delete; \ @@ -32,15 +32,17 @@ public: \ /**/ -#endif // BOOST_NO_CXX11_DELETED_FUNCTIONS +#endif // HPX_HAVE_CXX11_DELETED_FUNCTIONS #define HPX_NON_COPYABLE(cls) \ HPX_DELETE_COPY_CTOR(cls) \ HPX_DELETE_COPY_ASSIGN(cls) \ /**/ +#include + #if !defined(BOOST_DELETED_FUNCTION) -#if !defined(BOOST_NO_CXX11_DELETED_FUNCTIONS) +#if defined(HPX_HAVE_CXX11_DELETED_FUNCTIONS) # define BOOST_DELETED_FUNCTION(fun) fun = delete; #else # define BOOST_DELETED_FUNCTION(fun) private: fun; diff --git a/hpx/lcos/local/mutex.hpp b/hpx/lcos/local/mutex.hpp index ac142f6ad257..99f47e60b479 100644 --- a/hpx/lcos/local/mutex.hpp +++ b/hpx/lcos/local/mutex.hpp @@ -8,6 +8,7 @@ #define HPX_LCOS_MUTEX_JUN_23_2008_0530PM #include +#include #include #include #include @@ -18,13 +19,12 @@ #include #include -#include #include /////////////////////////////////////////////////////////////////////////////// namespace hpx { namespace lcos { namespace local { - class mutex : boost::noncopyable + class mutex { private: typedef lcos::local::spinlock mutex_type; @@ -41,6 +41,8 @@ namespace hpx { namespace lcos { namespace local HPX_ITT_SYNC_RENAME(this, "lcos::local::mutex"); } + HPX_NON_COPYABLE(mutex); + ~mutex() { HPX_ITT_SYNC_DESTROY(this); diff --git a/hpx/lcos/local/once.hpp b/hpx/lcos/local/once.hpp index 8f9e566dd060..15d20ef65eb8 100644 --- a/hpx/lcos/local/once.hpp +++ b/hpx/lcos/local/once.hpp @@ -11,14 +11,13 @@ #define HPX_LCOS_LOCAL_ONCE_JAN_03_2013_0810PM #include +#include #include #include -#include #include #include #include -#include namespace hpx { namespace lcos { namespace local { diff --git a/hpx/lcos/local/spinlock.hpp b/hpx/lcos/local/spinlock.hpp index 2df7cc40c660..61efec32db43 100644 --- a/hpx/lcos/local/spinlock.hpp +++ b/hpx/lcos/local/spinlock.hpp @@ -13,7 +13,7 @@ #define HPX_B3A83B49_92E0_4150_A551_488F9F5E1113 #include - +#include #include #include #include @@ -45,8 +45,6 @@ namespace hpx { namespace lcos { namespace local boost::uint64_t v_; #endif - HPX_MOVABLE_BUT_NOT_COPYABLE(spinlock) - public: /////////////////////////////////////////////////////////////////////// static void yield(std::size_t k) @@ -112,32 +110,13 @@ namespace hpx { namespace lcos { namespace local HPX_ITT_SYNC_CREATE(this, desc, ""); } - spinlock(spinlock && rhs) -#if defined(BOOST_WINDOWS) - : v_(BOOST_INTERLOCKED_EXCHANGE(&rhs.v_, 0)) -#else - : v_(__sync_lock_test_and_set(&rhs.v_, 0)) -#endif - {} + HPX_NON_COPYABLE(spinlock); ~spinlock() { HPX_ITT_SYNC_DESTROY(this); } - spinlock& operator=(spinlock && rhs) - { - if (this != &rhs) { - unlock(); -#if defined(BOOST_WINDOWS) - v_ = BOOST_INTERLOCKED_EXCHANGE(&rhs.v_, 0); -#else - v_ = __sync_lock_test_and_set(&rhs.v_, 0); -#endif - } - return *this; - } - void lock() { HPX_ITT_SYNC_PREPARE(this); diff --git a/hpx/lcos/local/spinlock_no_backoff.hpp b/hpx/lcos/local/spinlock_no_backoff.hpp index e052c7883230..cf71f9613028 100644 --- a/hpx/lcos/local/spinlock_no_backoff.hpp +++ b/hpx/lcos/local/spinlock_no_backoff.hpp @@ -10,6 +10,7 @@ #if !defined(HPX_LCOS_LOCAL_SPINLOCK_NO_BACKOFF) #define HPX_LCOS_LOCAL_SPINLOCK_NO_BACKOFF +#include #include #include #include @@ -36,40 +37,19 @@ namespace hpx { namespace lcos { namespace local private: boost::uint64_t v_; - HPX_MOVABLE_BUT_NOT_COPYABLE(spinlock_no_backoff) - public: spinlock_no_backoff() : v_(0) { HPX_ITT_SYNC_CREATE(this, "hpx::lcos::local::spinlock_no_backoff", ""); } - spinlock_no_backoff(spinlock_no_backoff && rhs) -#if defined(BOOST_WINDOWS) - : v_(BOOST_INTERLOCKED_EXCHANGE(&rhs.v_, 0)) -#else - : v_(__sync_lock_test_and_set(&rhs.v_, 0)) -#endif - {} + HPX_NON_COPYABLE(spinlock_no_backoff); ~spinlock_no_backoff() { HPX_ITT_SYNC_DESTROY(this); } - spinlock_no_backoff& operator=(spinlock_no_backoff && rhs) - { - if (this != &rhs) { - unlock(); -#if defined(BOOST_WINDOWS) - v_ = BOOST_INTERLOCKED_EXCHANGE(&rhs.v_, 0); -#else - v_ = __sync_lock_test_and_set(&rhs.v_, 0); -#endif - } - return *this; - } - void lock() { HPX_ITT_SYNC_PREPARE(this); diff --git a/hpx/lcos/local/spinlock_pool.hpp b/hpx/lcos/local/spinlock_pool.hpp index c4324a38b912..69bc22e21d8d 100644 --- a/hpx/lcos/local/spinlock_pool.hpp +++ b/hpx/lcos/local/spinlock_pool.hpp @@ -13,6 +13,7 @@ #ifndef HPX_LCOS_LOCAL_SPINLOCK_POOL_HPP #define HPX_LCOS_LOCAL_SPINLOCK_POOL_HPP +#include #include #include @@ -52,7 +53,7 @@ namespace hpx { namespace lcos { namespace local private: hpx::lcos::local::spinlock & sp_; - HPX_MOVABLE_BUT_NOT_COPYABLE(scoped_lock); + HPX_NON_COPYABLE(scoped_lock); public: explicit scoped_lock(void const * pv) diff --git a/hpx/util/spinlock.hpp b/hpx/util/spinlock.hpp index a04f346888e9..c1a20362321c 100644 --- a/hpx/util/spinlock.hpp +++ b/hpx/util/spinlock.hpp @@ -8,18 +8,18 @@ #if !defined(HPX_DF595582_FEBC_4EE0_A606_A1EEB171D770) #define HPX_DF595582_FEBC_4EE0_A606_A1EEB171D770 -#include -#include -#include - +#include #include #include +#include +#include + namespace hpx { namespace util { /// boost::mutex-compatible spinlock class -struct spinlock : boost::noncopyable +struct spinlock { private: boost::detail::spinlock m; @@ -33,6 +33,8 @@ struct spinlock : boost::noncopyable m = l; } + HPX_NON_COPYABLE(spinlock); + ~spinlock() { HPX_ITT_SYNC_DESTROY(this); diff --git a/hpx/util/spinlock_pool.hpp b/hpx/util/spinlock_pool.hpp index 828d9d66ef76..ae1ad8729a23 100644 --- a/hpx/util/spinlock_pool.hpp +++ b/hpx/util/spinlock_pool.hpp @@ -18,14 +18,15 @@ # pragma once #endif +#include +#include +#include + #include #include #include #include -#include -#include - namespace hpx { namespace util { #if HPX_HAVE_ITTNOTIFY != 0 @@ -62,9 +63,9 @@ namespace hpx { namespace util private: boost::detail::spinlock & sp_; - HPX_MOVABLE_BUT_NOT_COPYABLE(scoped_lock); - public: + HPX_NON_COPYABLE(scoped_lock); + public: explicit scoped_lock( void const * pv ): sp_( spinlock_for( pv ) ) { lock(); diff --git a/tests/performance/local/spinlock_overhead1.cpp b/tests/performance/local/spinlock_overhead1.cpp index 860c0151cd91..70c91125278d 100644 --- a/tests/performance/local/spinlock_overhead1.cpp +++ b/tests/performance/local/spinlock_overhead1.cpp @@ -4,6 +4,7 @@ // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +#include #include #include #include @@ -53,8 +54,10 @@ std::size_t k2 = 0; namespace test { - struct local_spinlock : boost::noncopyable + struct local_spinlock { + HPX_NON_COPYABLE(local_spinlock); + private: boost::uint64_t v_; diff --git a/tests/performance/local/spinlock_overhead2.cpp b/tests/performance/local/spinlock_overhead2.cpp index 2c340860ed47..da4b107421c5 100644 --- a/tests/performance/local/spinlock_overhead2.cpp +++ b/tests/performance/local/spinlock_overhead2.cpp @@ -4,6 +4,7 @@ // Distributed under the Boost Software License, Version 1.0. (See accompanying // file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) +#include #include #include #include @@ -55,8 +56,10 @@ std::size_t k3 = 0; namespace test { - struct local_spinlock : boost::noncopyable + struct local_spinlock { + HPX_NON_COPYABLE(local_spinlock); + private: boost::uint64_t v_; diff --git a/tests/unit/lcos/CMakeLists.txt b/tests/unit/lcos/CMakeLists.txt index b7839b13b6cb..e1ff180b0529 100644 --- a/tests/unit/lcos/CMakeLists.txt +++ b/tests/unit/lcos/CMakeLists.txt @@ -101,3 +101,49 @@ foreach(test ${tests}) ${test}_test_exe) endforeach() +if(HPX_WITH_COMPILE_ONLY_TESTS) + # add compile time tests + set(compile_tests + fail_compile_mutex_move + fail_compile_spinlock_move + fail_compile_spinlock_no_backoff_move + ) + + set(fail_compile_mutex_move_FLAGS FAILURE_EXPECTED) + set(fail_compile_spinlock_move_FLAGS FAILURE_EXPECTED) + set(fail_compile_spinlock_no_backoff_move_FLAGS FAILURE_EXPECTED) + + foreach(compile_test ${compile_tests}) + set(sources + ${compile_test}.cpp) + + source_group("Source Files" FILES ${sources}) + + if(MSVC) + # add dummy library for MSVC to generate a project in VS + add_hpx_library(${compile_test}_compile_test + SOURCES ${sources} + EXCLUDE_FROM_ALL + FOLDER "Tests/Unit/LCOs/CompileOnly") + endif() + + add_hpx_unit_compile_test( + "lcos" + ${compile_test} + SOURCES ${sources} + ${${compile_test}_FLAGS} + FOLDER "Tests/Unit/LCOs/CompileOnly") + + # add a custom target for this example + add_hpx_pseudo_target(tests.unit.lcos.${compile_test}) + + # make pseudo-targets depend on master pseudo-target + add_hpx_pseudo_dependencies(tests.unit.lcos + tests.unit.lcos.${compile_test}) + + # add dependencies to pseudo-target + add_hpx_pseudo_dependencies(tests.unit.lcos.${compile_test} + ${compile_test}_test_exe) + endforeach() +endif() + diff --git a/tests/unit/lcos/fail_compile_mutex_move.cpp b/tests/unit/lcos/fail_compile_mutex_move.cpp new file mode 100644 index 000000000000..3960106c3b0d --- /dev/null +++ b/tests/unit/lcos/fail_compile_mutex_move.cpp @@ -0,0 +1,19 @@ +// Copyright (c) 2015 Agustin Berge +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +// This must fail compiling + +#include +#include + +#include + +/////////////////////////////////////////////////////////////////////////////// +int main() +{ + using hpx::lcos::local::mutex; + mutex m1, m2(std::move(m1)); + return 0; +} diff --git a/tests/unit/lcos/fail_compile_spinlock_move.cpp b/tests/unit/lcos/fail_compile_spinlock_move.cpp new file mode 100644 index 000000000000..579f0b51a365 --- /dev/null +++ b/tests/unit/lcos/fail_compile_spinlock_move.cpp @@ -0,0 +1,19 @@ +// Copyright (c) 2015 Agustin Berge +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +// This must fail compiling + +#include +#include + +#include + +/////////////////////////////////////////////////////////////////////////////// +int main() +{ + using hpx::lcos::local::spinlock; + spinlock m1, m2(std::move(m1)); + return 0; +} diff --git a/tests/unit/lcos/fail_compile_spinlock_no_backoff_move.cpp b/tests/unit/lcos/fail_compile_spinlock_no_backoff_move.cpp new file mode 100644 index 000000000000..bd1a794a1d16 --- /dev/null +++ b/tests/unit/lcos/fail_compile_spinlock_no_backoff_move.cpp @@ -0,0 +1,19 @@ +// Copyright (c) 2015 Agustin Berge +// +// Distributed under the Boost Software License, Version 1.0. (See accompanying +// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt) + +// This must fail compiling + +#include +#include + +#include + +/////////////////////////////////////////////////////////////////////////////// +int main() +{ + using hpx::lcos::local::spinlock_no_backoff; + spinlock_no_backoff m1, m2(std::move(m1)); + return 0; +}