Skip to content

Conversation

@kkofler
Copy link
Contributor

@kkofler kkofler commented Nov 6, 2025

This fixes solveConcurrent crashing with a race condition when custom expression handlers are used.

src/cppad/core/atomic_base.hpp: Include <mutex> and <shared_mutex>. Do not include <cppad/utility/thread_alloc.hpp> because CPPAD_ASSERT_FIRST_CALL_NOT_PARALLEL is no longer used.
(CppAD::atomic_base::index_): Make this member non-const, because const members must be initialized in the member initializer, but the initialization is now in the constructor body because it needs to be within the mutex. (Logically, this member is still a constant.)
(CppAD::atomic_base::vector_mutex): New private static variable of type std::shared_mutex. A global read-write mutex that protects accesses to the class_object() and class_name() vectors.
(CppAD::atomic_base::class_object(), CppAD::atomic_base::class_name()): Remove no longer necessary CPPAD_ASSERT_FIRST_CALL_NOT_PARALLEL assertions. Instead, assume that these private static methods are called from within proper mutexes. Note that the first call can only reasonably be from the constructor, which holds an exclusive unique_lock. All the reading functions with shared locks require a valid index within the arrays, and such a valid index can only exist after at least one object has been constructed through the constructor. (Failing that, the assertion in the reading functions will trigger.) So the lazy initialization will always happen in an exclusive unique_lock, which is safe.
(CppAD::atomic_base::afun_name): Hold a std::shared_lock (read lock) on vector_mutex for this whole function.
(CppAD::atomic_base constructor): Remove the non-parallel restriction from the documentation and the non-parallel assertion from the code. Do not initialize index_ in the member initializer. Instead, wrap the initialization of index_ and the accesses to class_object() and class_name() in a std::unique_lock (write lock) on vector_mutex.
(CppAD::atomic_base destructor): Hold a std::unique_lock (write lock) on vector_mutex for this whole function.
(CppAD::atomic_base::class_object(std::size_t), CppAD::atomic_base::class_name(std::size_t)): Hold a std::shared_lock (read lock) on vector_mutex for this whole function.
(CppAD::atomic_base::clear): Remove the non-parallel restriction from the documentation and the non-parallel assertion from the code. Hold a std::unique_lock (write lock) on vector_mutex for this whole function.

This fixes solveConcurrent crashing with a race condition when custom expression handlers are used. src/cppad/core/atomic_base.hpp: Include <mutex> and <shared_mutex>. Do not include <cppad/utility/thread_alloc.hpp> because CPPAD_ASSERT_FIRST_CALL_NOT_PARALLEL is no longer used. (CppAD::atomic_base::index_): Make this member non-const, because const members must be initialized in the member initializer, but the initialization is now in the constructor body because it needs to be within the mutex. (Logically, this member is still a constant.) (CppAD::atomic_base::vector_mutex): New private static variable of type std::shared_mutex. A global read-write mutex that protects accesses to the class_object() and class_name() vectors. (CppAD::atomic_base::class_object(), CppAD::atomic_base::class_name()): Remove no longer necessary CPPAD_ASSERT_FIRST_CALL_NOT_PARALLEL assertions. Instead, assume that these private static methods are called from within proper mutexes. Note that the first call can only reasonably be from the constructor, which holds an exclusive unique_lock. All the reading functions with shared locks require a valid index within the arrays, and such a valid index can only exist after at least one object has been constructed through the constructor. (Failing that, the assertion in the reading functions will trigger.) So the lazy initialization will always happen in an exclusive unique_lock, which is safe. (CppAD::atomic_base::afun_name): Hold a std::shared_lock (read lock) on vector_mutex for this whole function. (CppAD::atomic_base constructor): Remove the non-parallel restriction from the documentation and the non-parallel assertion from the code. Do not initialize index_ in the member initializer. Instead, wrap the initialization of index_ and the accesses to class_object() and class_name() in a std::unique_lock (write lock) on vector_mutex. (CppAD::atomic_base destructor): Hold a std::unique_lock (write lock) on vector_mutex for this whole function. (CppAD::atomic_base::class_object(std::size_t), CppAD::atomic_base::class_name(std::size_t)): Hold a std::shared_lock (read lock) on vector_mutex for this whole function. (CppAD::atomic_base::clear): Remove the non-parallel restriction from the documentation and the non-parallel assertion from the code. Hold a std::unique_lock (write lock) on vector_mutex for this whole function.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant