Skip to content

Oscillators: Fixed the data race on the static juce::Random shared by start() - #406

Merged
drowaudio merged 1 commit into
developfrom
bugfix/issue_400_oscillator_static_random
Sep 2, 2026
Merged

Oscillators: Fixed the data race on the static juce::Random shared by start()#406
drowaudio merged 1 commit into
developfrom
bugfix/issue_400_oscillator_static_random

Conversation

@drowaudio

Copy link
Copy Markdown
Contributor

Summary

Oscillator::start() and MultiVoiceOscillator::start() each drew their random start phase from a function-local static juce::Random. Each oscillator now owns a juce::Random member instead, so starting a note only touches state belonging to that instance. Behaviour is unchanged: every note still gets a random start phase, and juce::Random's default constructor seeds itself, so instances don't share a sequence.

Root cause

The static generator was shared by every oscillator in the process and is entered from the audio thread: FourOscVoice::noteStarted() calls MultiVoiceOscillator::start() on each of its four oscillators from within renderNextBlock, on new notes and on retrigger. With more than one audio worker thread (the default is the CPU count), two 4OSC instances on different tracks starting notes at the same time race on juce::Random's seed, which is not thread safe. Oscillator::start() is not called from within the engine but is public API and had the same construction, so it gets the same fix.

Cost is one juce::Random (16 bytes) per Oscillator and per MultiVoiceOscillator.

Regression test

New tracktion_Oscillators.test.cpp (enabled via ENGINE_UNIT_TESTS_OSCILLATORS):

  • "start() on separate instances from concurrent threads": eight threads each own a MultiVoiceOscillator and an Oscillator and call start() repeatedly at the same time. Nothing should be shared between them. The race is silent without ThreadSanitizer, so this case is aimed at the TSan CI job. Verified locally with a -fsanitize=thread Debug build: before the fix TSan reports 4 data races in juce::Random::nextInt() via both start() overloads and the run exits 134; after the fix the run is clean.
  • "start() picks a random phase": checks that repeated start() calls on one instance, and start() on freshly constructed instances, produce different first samples. This guards against regressing to a fixed or default-seeded generator (e.g. reusing the std::default_random_engine the noise path already owns, which would give every instance the same phase sequence).

Full Debug TestRunner run locally: all JUCE unit tests pass; the only doctest failures are two pre-existing ClipLauncher audio-clip cases that fail identically on a develop baseline build on this machine while develop CI is green.

Fixes #400

…very start() call (fixes #400)

Oscillator::start() and MultiVoiceOscillator::start() drew their random start
phase from a function-local static juce::Random, so voices started at the same
time on different audio threads (e.g. two 4OSC instances on separate tracks)
raced on its seed. Each oscillator now owns its own juce::Random, which seeds
itself on construction, so behaviour is unchanged but nothing is shared between
instances.

Adds a doctest case that starts oscillators concurrently from several threads
(caught by the ThreadSanitizer CI job) and a check that start() draws a fresh
phase per call and per instance.
@codecov

codecov Bot commented Sep 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.74%. Comparing base (0e02f70) to head (6eec63e).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #406      +/-   ##
===========================================
+ Coverage    58.71%   58.74%   +0.03%     
===========================================
  Files          563      564       +1     
  Lines        78823    78879      +56     
  Branches     12325    12332       +7     
===========================================
+ Hits         46279    46337      +58     
+ Misses       32544    32542       -2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@drowaudio
drowaudio merged commit b88a6ee into develop Sep 2, 2026
39 checks passed
@drowaudio
drowaudio deleted the bugfix/issue_400_oscillator_static_random branch September 2, 2026 19:15
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.

[Bug]: shared static juce::Random in MultiVoiceOscillator::start() is a data race between audio threads

1 participant