Opt-in lock-free engagement checks: empty( std::memory_order )

include/psi/functionoid/detail/callable_base.hpp

@@ -27,6 +27,7 @@
 #include <boost/config.hpp>
 #include <psi/functionoid/detail/function_equal.hpp>
 
+#include <atomic>
 #include <cstdint>
 #include <cstring>
 #include <memory>
@@ -845,7 +846,7 @@ class callable_base : public callable_tag
 					empty_handler_traits::allowsSmallObjectOptimization
 				);
 				empty_handler_manager::assign( EmptyHandler(), p_function_->functor_, std::allocator<EmptyHandler>() );
-				p_function_->p_vtable_ = &empty_handler_vtable_;
+				p_function_->store_vtable( &empty_handler_vtable_ );
 			}
 		}
 
@@ -880,7 +881,7 @@ class callable_base : public callable_tag
     callable_base( no_eh_state_construction_trick_tag, Constructor const constructor, Args && ... args ) noexcept( noexcept( constructor( std::declval<callable_base &>(), std::forward<Args>( args )... ) ) )
     {
         auto const & vtable( constructor( *this, std::forward<Args>( args )... ) );
-        BOOST_ASSUME( p_vtable_ == &vtable );
+        BOOST_ASSUME( load_vtable( std::memory_order_relaxed ) == &vtable );
     }
 
 	// destructor = trivial promises no target ever needs destroying (and the
@@ -896,14 +897,31 @@ class callable_base : public callable_tag
 protected:
     bool empty( void const * const p_empty_handler_vtable ) const noexcept { return get_vtable().is_empty_handler_vtable( p_empty_handler_vtable ); }
 
-    /// \todo Add atomic vtable accessors that would enable lock-free operation
-    /// for basic functionality (such as empty(), clear() and operator()()) w/o
-    /// requiring an additional std::atomic<bool> is_my_functionoid_set-like
-    /// variable.
-    /// Making the vtable pointer a std::atomic<vtable const *> is not an
-    /// option currently because even with std::memory_order_relaxed access the
-    /// variable is accessed 'like a volatile' which produces bad codegen (e.g.
-    /// it is reread from memory for every access).
+    /// Engagement probe for a reader that may race the thread which assigns to
+    /// this callable. Only the vtable pointer load is ordered - what it orders
+    /// is the target buffer written before it (see store_vtable()).
+    bool empty( void const * const p_empty_handler_vtable, std::memory_order const order ) const noexcept
+        requires ( Traits::concurrent_reads )
+    {
+        auto const p_vtable{ load_vtable( order ) };
+        BOOST_ASSUME( p_vtable );
+        return p_vtable->is_empty_handler_vtable( p_empty_handler_vtable );
+    }
+
+    ////////////////////////////////////////////////////////////////////////////
+    // Atomic vtable accessors: the vtable pointer _is_ the engagement state, so
+    // lock-free empty()/operator bool() need no additional
+    // std::atomic<bool> is_my_functionoid_set-like member (which would also
+    // cost the genuinely trivial special members and the trivially copyable
+    // guarantee). Making the member itself a std::atomic<vtable const *> is
+    // still not an option: even memory_order_relaxed access is then compiled
+    // 'like a volatile' (rereading from memory for every access), and it is not
+    // movable. std::atomic_ref is the way out - the member stays a plain
+    // pointer, so every Traits without concurrent_reads keeps the existing
+    // codegen exactly, and only the opted-in instantiation pays.
+    ///   Both sides must go through these accessors: one plain store racing one
+    /// atomic load is still a data race, and no amount of atomicity on the
+    /// reader alone closes it.
     /// Atomic operations on non-atomic data:
     /// http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4013.html
     /// Making std::function safe for concurrency:
@@ -912,10 +930,30 @@ class callable_base : public callable_tag
     /// http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4058.pdf
     /// Overloaded and qualified std::function:
     /// (for 'automatic' atomic vtable access through volatile member function
-    /// overloads)
+    /// overloads - rejected: a volatile-qualified overload set duplicates the
+    /// entire API surface, a single memory_order overload does not)
     /// http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0045r0.pdf
     ///                                       (08.11.2016.) (Domagoj Saric)
-	auto const & get_vtable() const noexcept { BOOST_ASSUME( p_vtable_ ); return *p_vtable_; }
+    vtable const * load_vtable( std::memory_order const order ) const noexcept
+    {
+        if constexpr ( Traits::concurrent_reads ) { return std::atomic_ref{ p_vtable_ }.load( order ); }
+        else                                      { return p_vtable_; }
+    }
+
+    ///   The publishing store. Every assignment path writes the target buffer
+    /// _first_ and the vtable pointer last, so this release store is exactly
+    /// the edge a reader's acquire load of it needs: observing a non-empty
+    /// vtable implies the target it describes is fully constructed.
+    ///   Deliberately not exposed with a memory_order parameter: a weaker
+    /// publish would defeat the mechanism, so there is no legitimate choice for
+    /// a caller to make - only a way to get it wrong.
+    void store_vtable( vtable const * const p_vtable ) noexcept
+    {
+        if constexpr ( Traits::concurrent_reads ) { std::atomic_ref{ p_vtable_ }.store( p_vtable, std::memory_order_release ); }
+        else                                      { p_vtable_ = p_vtable; }
+    }
+
+	auto const & get_vtable() const noexcept { auto const p_vtable{ load_vtable( std::memory_order_relaxed ) }; BOOST_ASSUME( p_vtable ); return *p_vtable; }
 
 	buffer & functor() const noexcept { return functor_; }
 
@@ -955,16 +993,16 @@ class callable_base : public callable_tag
         auto const same_traits{ std::is_convertible_v<std::decay_t<FunctionObj> *, callable_base const *> };
         if constexpr ( same_traits )
         {
-		    BOOST_ASSUME( &functor_vtable == f.p_vtable_ );
+		    BOOST_ASSUME( &functor_vtable == f.load_vtable( std::memory_order_relaxed ) );
         }
 		if constexpr ( direct )
 		{
 		    BOOST_ASSUME( &f != static_cast<callable_tag const *>( this ) );
 			BOOST_ASSERT
             (
-                ( this->p_vtable_ == &empty_handler_vtable ) ||
+                ( this->load_vtable( std::memory_order_relaxed ) == &empty_handler_vtable ) ||
                 // just being constructed/inside a no_eh_state_construction_trick constructor in a debug build:
-                ( this->p_vtable_ == invalid_ptr )
+                ( this->load_vtable( std::memory_order_relaxed ) == invalid_ptr )
             );
 			assign_functionoid_direct( std::forward<FunctionObj>( f ), empty_handler_vtable );
 		}
@@ -1004,10 +1042,10 @@ class callable_base : public callable_tag
 			// functionoid.hpp as to why a null vtable is allowed and expected
 			// here.
 			//                                    (02.11.2010.) (Domagoj Saric)
-			BOOST_ASSERT( this->p_vtable_ == &empty_handler_vtable || /*just being constructed/inside a no_eh_state_construction_trick constructor in a debug build:*/ this->p_vtable_ == invalid_ptr );
+			BOOST_ASSERT( this->load_vtable( std::memory_order_relaxed ) == &empty_handler_vtable || /*just being constructed/inside a no_eh_state_construction_trick constructor in a debug build:*/ this->load_vtable( std::memory_order_relaxed ) == invalid_ptr );
 			using functor_manager = detail::functor_manager<std::remove_reference_t<FunctionObj>, Allocator, buffer>;
 			functor_manager::assign( std::forward<FunctionObj>( f ), this->functor_, a );
-			this->p_vtable_ = &functor_vtable;
+			this->store_vtable( &functor_vtable );
 		}
 		else
 		{
@@ -1050,7 +1088,7 @@ class callable_base : public callable_tag
 		using functor_manager = functor_manager<F, Allocator, buffer>;
 		this->destroy();
 		functor_manager::assign( std::forward<F>( f ), this->functor_, a );
-		this->p_vtable_ = &functor_vtable;
+		this->store_vtable( &functor_vtable );
 	}
 
 	template <typename EmptyHandler, typename F, typename Allocator>
@@ -1069,7 +1107,7 @@ class callable_base : public callable_tag
 		using functor_manager = functor_manager<std::remove_reference_t<F>, Allocator, buffer>;
 		callable_base tmp( empty_handler_vtable, EmptyHandler() );
 		functor_manager::assign( std::forward<F>( f ), tmp.functor_, a );
-		tmp.p_vtable_ = &functor_vtable;
+		tmp.store_vtable( &functor_vtable );
 		this->swap<EmptyHandler>( tmp, empty_handler_vtable );
 	}
 
@@ -1078,14 +1116,14 @@ class callable_base : public callable_tag
 	{
         static_assert( Traits::copyable != support_level::na, "Callable not copyable" );
 		source.get_vtable().clone( source.functor_, this->functor_ );
-		p_vtable_ = &source.get_vtable();
+		store_vtable( &source.get_vtable() );
 	}
 
 	void assign_functionoid_direct( callable_base && source, vtable const & empty_handler_vtable ) noexcept( ( Traits::moveable >= support_level::nofail ) || ( Traits::moveable == support_level::na && Traits::copyable >= support_level::nofail ) )
 	{
         source.move_to( *this );
-		this ->p_vtable_ = &source.get_vtable();
-		source.p_vtable_ = &empty_handler_vtable;
+		this ->store_vtable( &source.get_vtable() );
+		source.store_vtable( &empty_handler_vtable );
 	}
 
     static constexpr bool compatible_vtable_function_entry( support_level const me, support_level const other ) noexcept
@@ -1156,7 +1194,7 @@ class callable_base : public callable_tag
 
         auto & source_vtable{ source.get_vtable() } ;
         static_assert( sizeof( *p_vtable_ ) == sizeof( source_vtable ) );
-		p_vtable_ = reinterpret_cast<vtable const *>( &source_vtable );
+		store_vtable( reinterpret_cast<vtable const *>( &source_vtable ) );
 	}
 
 	template <typename EmptyHandler, typename FunctionBaseRef>
@@ -1194,7 +1232,12 @@ private: template <typename OtherTraits> friend class callable_base;
 	class safe_mover_base;
 	template <class EmptyHandler> class safe_mover;
 
-			vtable const * __restrict p_vtable_;
+	// __restrict promises the pointer is not aliased - which is exactly what
+	// concurrent_reads says it is (and std::atomic_ref cannot bind to a
+	// restrict-qualified lvalue anyway), so the opt-in drops it.
+	// mutable: load_vtable() is const, and std::atomic_ref's const-T
+	// specialization is both unnecessary here and libc++-buggy.
+	mutable std::conditional_t<Traits::concurrent_reads, vtable const *, vtable const * __restrict> p_vtable_;
 	mutable buffer                    functor_ ;
 }; // class callable_base
 
@@ -1220,7 +1263,7 @@ class callable_base<Traits>::safe_mover_base
 		empty_function_to_move_to_{ empty_function_to_move_to              },
 		empty_handler_vtable_     { empty_function_to_move_to.get_vtable() }
 	{
-		BOOST_ASSERT( empty_function_to_move_to_.p_vtable_ == &empty_handler_vtable_ );
+		BOOST_ASSERT( empty_function_to_move_to_.load_vtable( std::memory_order_relaxed ) == &empty_handler_vtable_ );
 		move( function_to_guard, empty_function_to_move_to_, empty_handler_vtable_ );
 	}
 
@@ -1230,8 +1273,8 @@ class callable_base<Traits>::safe_mover_base
 	static void move( callable_base & source, callable_base & destination, vtable const & empty_handler_vtable ) noexcept
 	{
         source.move_to( destination );
-		destination.p_vtable_ = source.p_vtable_;
-		source     .p_vtable_ = &empty_handler_vtable;
+		destination.store_vtable( source.load_vtable( std::memory_order_relaxed ) );
+		source     .store_vtable( &empty_handler_vtable );
 	}
 
 protected:

include/psi/functionoid/functionoid.hpp

@@ -194,6 +194,21 @@ class callable<ReturnType(Arguments ...), Traits>
     /// Determine if the function is empty (i.e. has an empty target).
     bool empty() const noexcept { return function_base::empty( &empty_handler_vtable() ); }
 
+    ///   Same question, asked from a thread that may be racing the one which
+    /// assigns the target - available only for Traits::concurrent_reads (see
+    /// policies.hpp for what the opt-in costs and, more importantly, what it
+    /// does and does not make safe).
+    ///   memory_order_acquire is the ordering that makes the answer usable:
+    /// a false result then also orders the target itself, so reading it
+    /// afterwards - invoking it, target()-ing it - is race-free with respect to
+    /// the assignment that published it. memory_order_relaxed answers the
+    /// question alone and orders nothing else.
+    bool empty( std::memory_order const order ) const noexcept
+        requires ( Traits::concurrent_reads )
+    {
+        return function_base::empty( &empty_handler_vtable(), order );
+    }
+
     void swap( callable & other ) noexcept
     {
         static_assert( sizeof( callable ) == sizeof( function_base ), "Internal inconsistency" );

include/psi/functionoid/policies.hpp

@@ -89,6 +89,42 @@ struct std_traits
     static constexpr auto rtti                 = true;
     static constexpr auto dll_safe_empty_check = true;
 
+    /// Opt-in for callables whose engagement is probed - empty() /
+    /// operator bool() - from a thread that may concurrently race the one
+    /// assigning to them (the build-once / publish-once pattern: one thread
+    /// assigns the target, others poll until they observe it). It makes every
+    /// access to the internal vtable pointer atomic (std::atomic_ref, so the
+    /// member stays a plain pointer and the type stays trivially copyable when
+    /// its Traits say so) and adds the empty( std::memory_order ) overload.
+    ///   Off by default, and the cost is larger than "one extra store": an
+    /// atomic access is not elided even when the compiler can prove the object
+    /// never escapes the function. Neither clang 22 nor gcc 16 promotes such an
+    /// object to registers - the atomics pin it to the stack and block the
+    /// devirtualization that normally follows. Measured, x86-64 -O3 -DNDEBUG,
+    /// both compilers in agreement: a purely local callable constructed from a
+    /// lambda and invoked once folds to 2 instructions without the opt-in and
+    /// to ~40 with it. (The as-if rule would permit the elision - an object no
+    /// other thread can reference cannot observe the difference - but neither
+    /// compiler implements it.)
+    ///   For what this is actually for, a shared long-lived callable whose
+    /// address escapes anyway, there is no such scalar replacement to lose and
+    /// the cost is only the atomic accesses themselves: ~+15% instructions on
+    /// an assign-and-publish. Ordinary single-threaded use should still never
+    /// pay it, hence the opt-in.
+    ///   Note that this orders the vtable pointer only. It publishes the
+    /// target and makes 'is it engaged' race-free; it does not make assignment
+    /// itself, or a concurrent invocation of a target being reassigned, safe.
+    ///   What makes the ordered read useful is a property of the *writer*, not
+    /// of the type: every assignment path here writes the target buffer first
+    /// and the vtable pointer last, so that one store is a genuine publication
+    /// point and an acquire load of it orders everything before it. A reader
+    /// that observes an engaged callable and then invokes it is safe only for
+    /// as long as no one reassigns or clears it - clear() and a subsequent
+    /// assign() destroy the target the reader is about to call. This is a
+    /// publish-once mechanism; it does not make the callable's whole lifetime
+    /// concurrent.
+    static constexpr bool concurrent_reads     = false;
+
     static constexpr std::uint8_t sbo_size      = 4 * sizeof( void * );
     static constexpr std::uint8_t sbo_alignment = alignof( std::max_align_t );
 

test/CMakeLists.txt

@@ -12,6 +12,7 @@ add_executable( functionoid_smoke
     callable_invoke_test.cpp
     nested_callable_test.cpp
     vtable_attrs_test.cpp
+    concurrent_reads_test.cpp
 )
 target_link_libraries( functionoid_smoke PRIVATE GTest::gtest_main Psi::Functionoid )