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.
@svigerske svigerske self-assigned this Nov 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants