Task: Fix rate_engine concurrent update/rates crash (SIGSEGV/SIGABRT)

Table of Contents

This page documents a task in the Cross-rates matrix (CRM) story. It captures the goal, current status, acceptance, and any notes or results.

Goal

rate_engine::update() and rate_engine::rates()/rate() run correctly under the documented single-producer/multi-reader concurrency contract with no crash, verified by a stress run of the existing 'concurrent updates and batched reads never crash or hang' test (many iterations, under ctest and standalone) plus ideally a TSAN build.

Status

Field Value
State DONE
Parent story Cross-rates matrix (CRM)
Now Nothing.
Waiting on Nothing.
Next Nothing.
Last touched 2026-07-12

Acceptance

  • Root cause identified; fix applied; the concurrency test passes reliably across repeated runs (both standalone with the xml reporter and under ctest -R with parallel scheduling); existing single-threaded rate_engine/topology_builder/risk_recentering tests still pass.

Plan

  1. Reproduce: confirmed 5/5 with the full test binary + xml reporter (ores.analytics.quant.tests -r xml::out=...), matching the original report exactly.
  2. Build a standalone ThreadSanitizer binary (TSAN is not wired into any CMake preset – compiled rate_engine.cpp=/=topology_builder.cpp=/ =risk_recentering.cpp + the test .cpp files directly with clang++ -fsanitize=thread, linked against the vcpkg static libCatch2.a): first just the concurrency test in isolation (no race reported, 15/15 clean – consistent with the original investigation's own failure to reproduce in isolation), then the full three-test-file suite with the xml reporter (still no race reported, 3/3 clean) – TSAN saw no data race in rate_engine=/ =crm_topology=/=immer itself under the exact conditions that reliably crash the real binary.
  3. That result redirected the investigation: if TSAN instruments everything in the race and still finds nothing, the race is most likely in code TSAN's instrumentation doesn't cover – e.g. the vcpkg-prebuilt libCatch2.a, which was not compiled with -fsanitize=thread. That pointed at Catch2's own assertion machinery, which is documented upstream as not thread-safe: calling REQUIRE=/=CHECK from any thread other than the one running the TEST_CASE mutates shared, non-atomic per-test result-capture state.
  4. Found exactly that: the concurrency test's 4 reader threads called REQUIRE(...) directly inside their thread lambdas (rate_engine_tests.cpp, the rates() result loop). This alone explains every reported symptom – reproduces only with the xml reporter (more shared reporter state touched per assertion, more surface for the race), does not reproduce running the test in isolation with the default reporter (less contention around Catch2's internals), and the SIGABRT was literally Catch2's own re-entrancy assertion (OutputRedirect::activate) firing when two threads hit its capture machinery at once.
  5. Fix: reader threads now only count failures into a plain std::atomic<int>; the main thread asserts once, after every thread has joined – the standard safe pattern for multi-threaded Catch2 tests. No production code (rate_engine.cpp, crm_topology.hpp, rate_snapshot.*) changed at all.
  6. Verified: 20/20 clean runs of the full binary with the xml reporter (the exact condition that reproduced 5/5 before the fix), 10/10 clean ctest -R runs.

Notes

How it was found

Discovered while raising a PR for an unrelated story (Extract stochastic process math from ores.synthetic into ores.analytics.quant), running the mandatory pre-PR full build + ctest on branch feature/move-istochasticprocess-interface (environment: jolly_knuth, preset linux-clang-debug-make). The new commits on that branch touch only ores.analytics.quant's process/domain-validation code (new files under service/processes/, service/process_factory.*, domain/process_parameter_validation.*) and never touch rate_engine.*, crm_topology.hpp, rate_snapshot.*, or rate_engine_tests.cpp — this is pre-existing behaviour in code from the earlier CRM story (PRs #1510, #1511, #1512), not something introduced by that branch.

Reproduction

  • Full suite via ctest --preset linux-clang-debug-make: intermittently fails ores.analytics.quant.tests with either SEGFAULT or SIGABRT (the SIGABRT case is a secondary failure – Catch2's Catch::OutputRedirect::activate() hits !m_redirectActive && "redirect is already active" because the crash happens mid-capture; the real fault is still the concurrency test).
  • Reliably reproduces standalone when the whole binary runs with the xml::out reporter that CI/ctest uses, e.g.: ores.analytics.quant.tests -r xml::out=.... Running the failing test case alone in isolation (ores.analytics.quant.tests "concurrent updates and batched reads never crash or hang") and running the full binary with the default compact reporter did not reproduce across several attempts – the crash is timing/scheduling-sensitive, consistent with a genuine data race rather than a deterministic logic bug.
  • Test in question: rate_engine_tests.cpp, TEST_CASE("concurrent updates and batched reads never crash or hang", "[rate_engine][thread]") (line ~219). One writer thread calls engine.update(...) in a loop; four reader threads call engine.rates(...) 2000 times each – this matches rate_engine's own documented contract in rate_engine.hpp (single producer, many concurrent readers), so the test is not misusing the API as far as could be determined.

Coredumps

Two coredumps captured via coredumpctl on this machine (environment jolly_knuth), both for ores.analytics.quant.tests:

PID Signal Timestamp (BST) Coredump path (zstd, use coredumpctl dump <PID> -o <file> to extract)
535290 SIGSEGV 2026-07-12 12:27:43 /var/lib/systemd/coredump/core.ores\x2eanalytics\x2e.1000.40e75b2c08a142ea85e12da39c72ab8f.535290.1783855662000000.zst
543053 SIGABRT 2026-07-12 12:33:29 /var/lib/systemd/coredump/core.ores\x2eanalytics\x2e.1000.40e75b2c08a142ea85e12da39c72ab8f.543053.1783856009000000.zst

Retrieve with e.g. coredumpctl info 535290 or coredumpctl gdb 535290 (systemd-coredump retention is time/space-bounded on this machine, so these may age out – reproduce fresh if they are gone; see Reproduction above).

PID 535290 (SIGSEGV) – three relevant threads

  • Writer thread (the single producer calling update()) crashed inside immer's refcounted node destructor, reached via rate_snapshot::transient() called from rate_engine::update: 

    immer::detail::rbts::node<...vertex_state...>::dec()
immer::detail::rbts::dec_visitor::visit_regular<...>
immer::detail::rbts::empty_regular_pos<...>::visit<dec_visitor>
immer::detail::rbts::rbtree<...>::traverse<dec_visitor>
immer::detail::rbts::rbtree<...>::dec()
immer::detail::rbts::rbtree<...>::~rbtree()
immer::vector<vertex_state,...>::transient() [inside rate_snapshot::transient()]
ores::analytics::quant::domain::rate_snapshot::transient()
ores::analytics::quant::service::rate_engine::update(driver_quote const&)

  • A reader thread crashed inside std::string::operator== called from std::unordered_map::find, reached via crm_topology::currency_id_for: 

    std::basic_string::max_size / size  [operating on a corrupt/garbage string]
std::operator==<char,...>
std::equal_to<basic_string<char>>::operator()
std::_Hashtable<...>::_M_key_equals
std::_Hashtable<...>::_M_locate
std::_Hashtable<...>::find
std::unordered_map<string, currency_id>::find
ores::analytics::quant::domain::crm_topology::currency_id_for
ores::analytics::quant::service::rate_engine::rate_from_snapshot
ores::analytics::quant::service::rate_engine::rates

    The garbage/corrupt std::string being compared inside the hashtable lookup (crashing in max_size()=/=size() before even reaching the byte comparison) is the strongest signal here: crm_topology is constructed once and never mutated (see its class comment in crm_topology.hpp), so a concurrent read-only lookup into its currency_index_ should never see a torn/corrupt key – unless something elsewhere has corrupted the heap (e.g. a use-after-free or buffer overrun in the immer-backed rate_snapshot=/=immer::atom path) and this is simply where the corruption was first observed.

  • A third thread was mid-__clone3 (thread spin-up), not directly relevant.

PID 543053 (SIGABRT) – secondary failure, same root cause

Same test, same call sites implicated (crm_topology::currency_id_for via rate_engine::update from the writer thread, and via rate_from_snapshot from a reader). The abort itself is Catch2's OutputRedirect::activate() re-entrancy assertion, which fires because the underlying SIGSEGV/heap corruption happens while Catch2's XML reporter has an output capture active – i.e. this is very likely the same underlying bug, just observed via the abort path instead of a raw segfault.

Working hypothesis (unconfirmed – needs an implementer's investigation)

  • rate_engine::update() is documented as the single producer, and rate()=/=rates() as safe for concurrent readers, via immer::atom<rate_snapshot> snapshot_ (see the class doc in rate_engine.hpp). The crash inside the writer's own call to rate_snapshot::transient() – i.e. destructing/reference-counting the immer::vector<vertex_state> it just loaded via snapshot_.load().get() in rate_engine.cpp line ~108 – suggests the refcount on the immer tree nodes is being corrupted by concurrent load() calls from the four reader threads racing with the single writer's load()=/=store(), despite immer's refcount_policy=/=spinlock_policy memory policy nominally being built for exactly this. Whether that is a genuine bug in how immer::atom is used here (e.g. a missing memory fence, an unsafe .get() on the loaded box, or an assumption violated by rate_snapshot's own copy/move semantics) or an upstream immer issue needs a focused investigation – ideally build and run ores.analytics.quant.tests under ThreadSanitizer (TSAN is not currently wired into any CMake preset for this component) to get a precise data-race report instead of reasoning from crash-site backtraces alone.
  • Check domain/rate_snapshot.hpp=/.cpp and =domain/vertex_state.hpp for how they wrap/copy the underlying immer::vector; check whether immer::atom::load() as used here (.load().get()) returns a properly refcounted owning copy or something that can be invalidated by a concurrent store().

Test Scenarios

Manual QA scenarios (scaffolded via compass add test_scenario, run through the QA Validation Runner panel) that verify this task. Link new ones here as they're created; the scenario doc itself links back via its "Verifies task" field.

Scenario State Notes
     

PRs

PR Title
#1524 [analytics.quant] Fix intermittent SIGSEGV/SIGABRT in rate_engine concurrency test

Review

Comment summary File Decision Notes
       

Result

Root cause was a test bug, not a production bug: the concurrency test's 4 reader threads called Catch2's REQUIRE() macro directly from worker threads. Catch2's assertion machinery is not thread-safe (mutates shared, non-atomic per-test-case result-capture state), so concurrent REQUIRE=/=CHECK calls from multiple threads is itself a data race – independent of anything rate_engine=/=crm_topology=/=immer::atom does. That explains every symptom in the original report: reproduces only with the full binary + xml reporter (more shared Catch2 reporter state touched per assertion), not in isolation (less threading contention around Catch2's internals), and the SIGABRT was Catch2's own OutputRedirect::activate() re-entrancy assertion firing.

Confirmed via a standalone ThreadSanitizer build (compiled directly with clang++ -fsanitize=thread, since TSAN isn't wired into any CMake preset) that found zero data races in the library code itself, under the exact conditions (full suite, xml reporter) that reliably crash the real binary – which is what redirected the investigation away from rate_engine=/=immer and towards the vcpkg-prebuilt libCatch2.a (not TSAN-instrumented) and its documented threading limitation.

Fix: rate_engine_tests.cpp's reader threads now collect failures into a plain std::atomic<int>; only the main thread asserts, after every thread has joined. rate_engine.cpp, crm_topology.hpp, rate_snapshot.*, and every other production file are unchanged.

Verified: 20/20 clean runs of the full test binary with the xml reporter (the exact condition that crashed 5/5 before the fix), 10/10 clean ctest -R runs, single-threaded topology_builder=/=rate_engine=/ =risk_recentering tests all still green (23 test cases total).

Emacs 29.3 (Org mode 9.6.15)