diff --git a/CHANGELOG.md b/CHANGELOG.md index 6e8c760f..aa11f720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Version 149: * Protect calls from macros * pausation always allocates * Don't copy completion handlers +* handler_ptr is move-only -------------------------------------------------------------------------------- diff --git a/include/boost/beast/core/handler_ptr.hpp b/include/boost/beast/core/handler_ptr.hpp index 12b03da6..dfce6cce 100644 --- a/include/boost/beast/core/handler_ptr.hpp +++ b/include/boost/beast/core/handler_ptr.hpp @@ -10,10 +10,9 @@ #ifndef BOOST_BEAST_HANDLER_PTR_HPP #define BOOST_BEAST_HANDLER_PTR_HPP +#include #include #include -#include -#include #include #include @@ -22,73 +21,63 @@ namespace beast { /** A smart pointer container with associated completion handler. - This is a smart pointer that retains shared ownership of an - object through a pointer. Memory is managed using the allocation - and deallocation functions associated with a completion handler, - which is also stored in the object. The managed object is - destroyed and its memory deallocated when one of the following - happens: + This is a smart pointer that retains unique ownership of an + object through a pointer. Memory is managed using the allocator + associated with a completion handler stored in the object. The + managed object is destroyed and its memory deallocated when one + of the following occurs: @li The function @ref invoke is called. @li The function @ref release_handler is called. - @li The last remaining container owning the object is destroyed. + @li The container is destroyed. - Objects of this type are used in the implementation of - composed operations. Typically the composed operation's shared - state is managed by the @ref handler_ptr and an allocator - associated with the final handler is used to create the managed - object. + Objects of this type are used in the implementation of composed + operations with states that are expensive or impossible to move. + This container manages that non-trivial state on behalf of the + composed operation. @par Thread Safety @e Distinct @e objects: Safe.@n @e Shared @e objects: Unsafe. - @note The reference count is stored using a 16 bit unsigned - integer. Making more than 2^16 copies of one object results - in undefined behavior. - - @tparam T The type of the owned object. + @tparam T The type of the owned object. Must be noexcept destructible. @tparam Handler The type of the completion handler. */ template class handler_ptr { - struct P - { - T* t; - std::atomic n; + T* t_ = nullptr; + Handler h_; - // There's no way to put the handler anywhere else - // without exposing ourselves to race conditions - // and all sorts of ugliness. - // See: - // https://github.com/boostorg/beast/issues/215 - Handler handler; - - template - P(DeducedHandler&& handler, Args&&... args); - }; - - P* p_; + void clear(); public: - /// The type of element this object stores + static_assert(std::is_nothrow_destructible::value, + "T must be nothrow destructible"); + + /// The type of element stored using element_type = T; - /// The type of handler this object stores + /// The type of handler stored using handler_type = Handler; - /// Copy assignment (disallowed). + /// Default constructor (deleted). + handler_ptr() = delete; + + /// Copy assignment (deleted). handler_ptr& operator=(handler_ptr const&) = delete; - /** Destructs the owned object if no more @ref handler_ptr link to it. + /// Move assignment (deleted). + handler_ptr& operator=(handler_ptr &&) = delete; - If `*this` owns an object and it is the last @ref handler_ptr - owning it, the object is destroyed and the memory deallocated - using the associated deallocator. + /** Destructor + + If `*this` owns an object the object is destroyed and + the memory deallocated using the allocator associated + with the handler. */ ~handler_ptr(); @@ -99,54 +88,42 @@ public: */ handler_ptr(handler_ptr&& other); - /// Copy constructor - handler_ptr(handler_ptr const& other); + /// Copy constructor (deleted). + handler_ptr(handler_ptr const& other) = delete; - /** Construct a new @ref handler_ptr + /** Constructor - This creates a new @ref handler_ptr with an owned object - of type `T`. The allocator associated with the handler will - be used to allocate memory for the owned object. The constructor - for the owned object will be called thusly: + This creates a new container with an owned object of + type `T`. The allocator associated with the handler will + be used to allocate memory for the owned object. The + constructor for the owned object will be called with the + following equivalent signature: @code - T(handler, std::forward(args)...) + T::T(Handler&, Args&&...) @endcode @param handler The handler to associate with the owned - object. The argument will be moved. + object. The argument will be moved if it is an xvalue. @param args Optional arguments forwarded to the owned object's constructor. */ - template - handler_ptr(Handler&& handler, Args&&... args); + template + explicit handler_ptr(DeducedHandler&& handler, Args&&... args); - /** Construct a new @ref handler_ptr - - This creates a new @ref handler_ptr with an owned object - of type `T`. The allocator associated with the handler will - be used to allocate memory for the owned object. The constructor - for the owned object will be called thusly: - - @code - T(handler, std::forward(args)...) - @endcode - - @param handler The handler to associate with the owned - object. The argument will be copied. - - @param args Optional arguments forwarded to - the owned object's constructor. - */ - template - handler_ptr(Handler const& handler, Args&&... args); + /// Returns a const reference to the handler + handler_type const& + handler() const + { + return h_; + } /// Returns a reference to the handler handler_type& - handler() const + handler() { - return p_->handler; + return h_; } /** Returns a pointer to the owned object. @@ -154,32 +131,30 @@ public: T* get() const { - return p_->t; + return t_; } /// Return a reference to the owned object. T& operator*() const { - return *p_->t; + return *t_; } /// Return a pointer to the owned object. T* operator->() const { - return p_->t; + return t_; } /** Release ownership of the handler Requires: `*this` owns an object - Before this function returns, - the owned object is destroyed, satisfying the - deallocation-before-invocation Asio guarantee. All - instances of @ref handler_ptr which refer to the - same owned object will be reset, including this instance. + Before this function returns, the owned object is + destroyed, satisfying the deallocation-before-invocation + Asio guarantee. @return The released handler. */ @@ -191,9 +166,7 @@ public: This function invokes the handler in the owned object with a forwarded argument list. Before the invocation, the owned object is destroyed, satisfying the - deallocation-before-invocation Asio guarantee. All - instances of @ref handler_ptr which refer to the - same owned object will be reset, including this instance. + deallocation-before-invocation Asio guarantee. @note Care must be taken when the arguments are themselves stored in the owned object. Such arguments must first be diff --git a/include/boost/beast/core/impl/handler_ptr.ipp b/include/boost/beast/core/impl/handler_ptr.ipp index 15a159a8..8a03e1cf 100644 --- a/include/boost/beast/core/impl/handler_ptr.ipp +++ b/include/boost/beast/core/impl/handler_ptr.ipp @@ -18,87 +18,63 @@ namespace boost { namespace beast { template -template -inline -handler_ptr::P:: -P(DeducedHandler&& h, Args&&... args) - : n(1) - , handler(std::forward(h)) +void +handler_ptr:: +clear() { - typename std::allocator_traits< - boost::asio::associated_allocator_t>:: - template rebind_alloc alloc{ - boost::asio::get_associated_allocator(handler)}; - t = std::allocator_traits::allocate(alloc, 1); - try - { - t = new(t) T{handler, - std::forward(args)...}; - } - catch(...) - { - std::allocator_traits< - decltype(alloc)>::deallocate(alloc, t, 1); - throw; - } + typename beast::detail::allocator_traits< + boost::asio::associated_allocator_t< + Handler>>::template rebind_alloc alloc{ + boost::asio::get_associated_allocator(h_)}; + beast::detail::allocator_traits< + decltype(alloc)>::destroy(alloc, t_); + beast::detail::allocator_traits< + decltype(alloc)>::deallocate(alloc, t_, 1); + t_ = nullptr; } template handler_ptr:: ~handler_ptr() { - if(! p_) - return; - if(--p_->n) - return; - if(p_->t) - { - p_->t->~T(); - typename std::allocator_traits< - boost::asio::associated_allocator_t>:: - template rebind_alloc alloc{ - boost::asio::get_associated_allocator( - p_->handler)}; - std::allocator_traits< - decltype(alloc)>::deallocate(alloc, p_->t, 1); - } - delete p_; + if(t_) + clear(); } template handler_ptr:: handler_ptr(handler_ptr&& other) - : p_(other.p_) + : t_(other.t_) + , h_(std::move(other.h_)) { - other.p_ = nullptr; + other.t_ = nullptr; } template +template handler_ptr:: -handler_ptr(handler_ptr const& other) - : p_(other.p_) +handler_ptr(DeducedHandler&& handler, Args&&... args) + : t_([&] + { + BOOST_STATIC_ASSERT(! std::is_array::value); + typename beast::detail::allocator_traits< + boost::asio::associated_allocator_t< + Handler>>::template rebind_alloc alloc{ + boost::asio::get_associated_allocator(handler)}; + using A = decltype(alloc); + auto const d = + [&alloc](T* p) + { + beast::detail::allocator_traits::deallocate(alloc, p, 1); + }; + std::unique_ptr p{ + beast::detail::allocator_traits::allocate(alloc, 1), d}; + beast::detail::allocator_traits::construct( + alloc, p.get(), handler, std::forward(args)...); + return p.release(); + }()) + , h_(std::forward(handler)) { - if(p_) - ++p_->n; -} - -template -template -handler_ptr:: -handler_ptr(Handler&& handler, Args&&... args) - : p_(new P{std::move(handler), - std::forward(args)...}) -{ - BOOST_STATIC_ASSERT(! std::is_array::value); -} - -template -template -handler_ptr:: -handler_ptr(Handler const& handler, Args&&... args) - : p_(new P{handler, std::forward(args)...}) -{ - BOOST_STATIC_ASSERT(! std::is_array::value); } template @@ -107,18 +83,9 @@ handler_ptr:: release_handler() -> handler_type { - BOOST_ASSERT(p_); - BOOST_ASSERT(p_->t); - p_->t->~T(); - typename std::allocator_traits< - boost::asio::associated_allocator_t>:: - template rebind_alloc alloc{ - boost::asio::get_associated_allocator( - p_->handler)}; - std::allocator_traits< - decltype(alloc)>::deallocate(alloc, p_->t, 1); - p_->t = nullptr; - return std::move(p_->handler); + BOOST_ASSERT(t_); + clear(); + return std::move(h_); } template @@ -127,18 +94,9 @@ void handler_ptr:: invoke(Args&&... args) { - BOOST_ASSERT(p_); - BOOST_ASSERT(p_->t); - p_->t->~T(); - typename std::allocator_traits< - boost::asio::associated_allocator_t>:: - template rebind_alloc alloc{ - boost::asio::get_associated_allocator( - p_->handler)}; - std::allocator_traits< - decltype(alloc)>::deallocate(alloc, p_->t, 1); - p_->t = nullptr; - p_->handler(std::forward(args)...); + BOOST_ASSERT(t_); + clear(); + h_(std::forward(args)...); } } // beast diff --git a/test/beast/core/handler_ptr.cpp b/test/beast/core/handler_ptr.cpp index 953bdb1f..768239b2 100644 --- a/test/beast/core/handler_ptr.cpp +++ b/test/beast/core/handler_ptr.cpp @@ -12,6 +12,7 @@ #include #include +#include #include namespace boost { @@ -22,8 +23,7 @@ class handler_ptr_test : public beast::unit_test::suite public: struct handler { - handler() = default; - handler(handler const&) = default; + std::unique_ptr ptr; void operator()(bool& b) const @@ -54,12 +54,10 @@ public: void run() override { - handler h; - handler_ptr p1{h}; - handler_ptr p2{p1}; + handler_ptr p1{handler{}}; try { - handler_ptr p3{h}; + handler_ptr p2{handler{}}; fail(); } catch(std::exception const&) @@ -70,9 +68,9 @@ public: { fail(); } - handler_ptr p4{std::move(h)}; + handler_ptr p3{handler{}}; bool b = false; - p4.invoke(std::ref(b)); + p3.invoke(std::ref(b)); BEAST_EXPECT(b); } };