From eb5c00c1ed8593db8be372f87865ae894f153f30 Mon Sep 17 00:00:00 2001 From: Ben Swift Date: Mon, 20 Apr 2026 01:47:31 +1000 Subject: [PATCH] fix(concurrency): atomic UNIV::TIME, UNIV::DEVICE_TIME, m_libsLoaded TSan flagged three data races on the cpp-modernisation-followup branch (all pre-existing, not introduced by that PR): - UNIV::TIME (plain volatile uint64_t) is written by the scheduler thread in TaskScheduler::timeSlice and read from every other thread; the volatile keyword prevents compiler reordering but does nothing for atomicity across threads. - UNIV::DEVICE_TIME has the same shape, written by the audio callback and read from other threads. - SchemeProcess::m_libsLoaded is a bool set once by the task thread after runtime libs finish loading and spin-waited on by the server thread in serverImpl(). Promote all three to std::atomic. std::atomic is lock-free on every tier-1 platform (added a static_assert) and has the same size/alignment as uint64_t, so the xtlang JIT's plain `load i64, i64* @TIME` in runtime/bitcode.ll remains layout-compatible. A couple of callers had to change: - AudioDevice.cpp's `UNIV::TIME = UNIV::DEVICE_TIME` is now explicit load/store to avoid the deleted atomic copy assignment. - Three `auto time(UNIV::TIME)` sites become `uint64_t time(UNIV::TIME)` for the same reason (auto would deduce atomic, which isn't copyable). Re-ran the full libs-core + cpp-unit + compiler-unit suite under TSan: the four previously-reported unique races are down to one (the SchemeTask deque iterator race, which belongs with TASK-042). --- include/SchemeProcess.h | 6 +++++- include/UNIV.h | 12 ++++++++++-- src/AudioDevice.cpp | 4 ++-- src/SchemeProcess.cpp | 4 ++-- src/UNIV.cpp | 7 +++++-- 5 files changed, 24 insertions(+), 9 deletions(-) diff --git a/include/SchemeProcess.h b/include/SchemeProcess.h index 1f690818..79e80425 100644 --- a/include/SchemeProcess.h +++ b/include/SchemeProcess.h @@ -41,6 +41,7 @@ #include #include "Task.h" #include "EXTThread.h" +#include #include #include #include @@ -92,7 +93,10 @@ class SchemeProcess { int16_t m_serverPort; bool m_banner; std::string m_initExpr; - bool m_libsLoaded; + // Set once by the task thread after the runtime libs finish loading. + // Read by the server thread spin-waiting in serverImpl(); must be + // synchronised. + std::atomic m_libsLoaded; std::recursive_mutex m_guardMutex; std::condition_variable_any m_guardCond; bool m_running; diff --git a/include/UNIV.h b/include/UNIV.h index 7f31c4e7..6bb86686 100644 --- a/include/UNIV.h +++ b/include/UNIV.h @@ -36,6 +36,7 @@ #ifndef UNIV_H #define UNIV_H +#include #include #include @@ -108,8 +109,15 @@ extern std::string SHARE_DIR; EXPORT uint32_t CHANNELS; EXPORT uint32_t IN_CHANNELS; EXPORT uint32_t SAMPLE_RATE; -EXPORT volatile uint64_t TIME; -extern uint64_t DEVICE_TIME; +// TIME is written by the scheduler thread (TaskScheduler::timeSlice) and +// read from every other thread; atomic gives a lock-free load/ +// store on all tier-1 platforms. DEVICE_TIME is written by the audio +// callback and read from other threads, same story. The xtlang JIT +// accesses TIME via @TIME in runtime/bitcode.ll as a plain i64 load — OK +// because the layout of std::atomic is a single uint64_t on +// all supported compilers. +EXPORT std::atomic TIME; +extern std::atomic DEVICE_TIME; extern double AUDIO_CLOCK_BASE; extern double AUDIO_CLOCK_NOW; extern uint64_t TIME_DIVISION; diff --git a/src/AudioDevice.cpp b/src/AudioDevice.cpp index 6e042b8c..157e3950 100644 --- a/src/AudioDevice.cpp +++ b/src/AudioDevice.cpp @@ -289,7 +289,7 @@ int audioCallback(const void* InputBuffer, void* OutputBuffer, unsigned long Fra auto sched(reinterpret_cast(UserData)); UNIV::DEVICE_TIME += FramesPerBuffer; if (likely(UNIV::TIME_DIVISION == 1)) { - UNIV::TIME = UNIV::DEVICE_TIME; + UNIV::TIME.store(UNIV::DEVICE_TIME.load()); } if (unlikely(AudioDevice::CLOCKBASE < 1.0)) { AudioDevice::CLOCKBASE = getRealTime(); @@ -320,7 +320,7 @@ int audioCallback(const void* InputBuffer, void* OutputBuffer, unsigned long Fra llvm_zone_t* zone = extemp::EXTZones::llvm_peek_zone_stack(); auto dat(reinterpret_cast(OutputBuffer)); auto in(reinterpret_cast(InputBuffer)); - auto time(UNIV::DEVICE_TIME); + uint64_t time(UNIV::DEVICE_TIME); if (likely(!UNIV::IN_CHANNELS)) { float dummy(0.0); for (uint64_t i = 0; i < FramesPerBuffer; ++i, ++time) { diff --git a/src/SchemeProcess.cpp b/src/SchemeProcess.cpp index c07f25ae..4457f7db 100644 --- a/src/SchemeProcess.cpp +++ b/src/SchemeProcess.cpp @@ -392,7 +392,7 @@ void* SchemeProcess::taskImpl() resetOutportString(); } else { if (m_banner) { - auto time(UNIV::TIME); + uint64_t time(UNIV::TIME); auto hours(time / UNIV::HOUR()); time -= hours * UNIV::HOUR(); auto minutes(time / UNIV::MINUTE()); @@ -520,7 +520,7 @@ void* SchemeProcess::serverImpl() std::string outString; if (m_banner) { outString += sm_banner; - auto time(UNIV::TIME); + uint64_t time(UNIV::TIME); auto hours(time / UNIV::HOUR()); time -= hours * UNIV::HOUR(); auto minutes(time / UNIV::MINUTE()); diff --git a/src/UNIV.cpp b/src/UNIV.cpp index f20a9d4d..6cf1a5ba 100644 --- a/src/UNIV.cpp +++ b/src/UNIV.cpp @@ -393,8 +393,11 @@ uint32_t NUM_FRAMES = 1024; uint32_t CHANNELS = 2; uint32_t IN_CHANNELS = 0; uint32_t SAMPLE_RATE = 44100; -volatile uint64_t TIME = 0l; -uint64_t DEVICE_TIME = 0l; +std::atomic TIME{0}; +std::atomic DEVICE_TIME{0}; +static_assert(std::atomic::is_always_lock_free, + "std::atomic must be lock-free so the xtlang JIT's " + "plain i64 loads of @TIME remain compatible"); double AUDIO_CLOCK_NOW = 0.0; double AUDIO_CLOCK_BASE = 0.0; uint64_t TIME_DIVISION = 1;