diff --git a/src/audio/CMakeLists.txt b/src/audio/CMakeLists.txt index f20a6a1..e2306bd 100644 --- a/src/audio/CMakeLists.txt +++ b/src/audio/CMakeLists.txt @@ -12,6 +12,7 @@ set(AUDIO_SOURCES engine/SourcePool.cpp engine/ExtraInputs.cpp addon/AddonContext.cpp + addon/ChainOps.cpp SourceChain.cpp SignalChain.cpp VSTHost.cpp diff --git a/src/audio/NodeAddon.cpp b/src/audio/NodeAddon.cpp index 766da8a..46fdb48 100644 --- a/src/audio/NodeAddon.cpp +++ b/src/audio/NodeAddon.cpp @@ -28,6 +28,7 @@ #include "addon/AddonContext.h" #include "addon/NapiHelpers.h" +#include "addon/ChainOps.h" // Lifetime/threading moved to addon/AddonContext (TLC phase 6); the usings // keep the 100+ existing binding bodies unchanged. @@ -2064,6 +2065,9 @@ public: void Execute() override { + // Serialize the FULL mutation (TLC deep-read 1): overlapping chain + // workers on the libuv pool must not interleave clear()/addProcessor(). + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); // Snapshot engine + vstHost through their mutex-protected helpers so // shutdown's reset on the message thread can't race the worker's // dereferences below. The shared_ptr locals keep both objects alive @@ -2250,6 +2254,9 @@ public: void Execute() override { + // Serialize the FULL mutation (TLC deep-read 1): overlapping chain + // workers on the libuv pool must not interleave clear()/addProcessor(). + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); auto liveEngine = snapshotEngine(); if (!liveEngine) { slotId_ = -1; return; } @@ -2298,6 +2305,9 @@ public: void Execute() override { + // Serialize the FULL mutation (TLC deep-read 1): overlapping chain + // workers on the libuv pool must not interleave clear()/addProcessor(). + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); auto liveEngine = snapshotEngine(); if (!liveEngine) { slotId_ = -1; return; } @@ -2357,6 +2367,9 @@ public: void Execute() override { + // Serialize the FULL mutation (TLC deep-read 1): overlapping chain + // workers on the libuv pool must not interleave clear()/addProcessor(). + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); auto liveEngine = snapshotEngine(); if (!liveEngine) { ok_ = false; return; } @@ -2414,7 +2427,11 @@ static Napi::Value RemoveProcessor(const Napi::CallbackInfo& info) auto liveEngine = snapshotEngine(); const auto slotId = slopsmith::addon::argSlotId(info, 0); if (liveEngine && slotId) + { + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); liveEngine->getSignalChain().removeProcessor(*slotId); + slopsmith::addon::bumpChainGeneration(); + } return info.Env().Undefined(); } @@ -2424,7 +2441,11 @@ static Napi::Value MoveProcessor(const Napi::CallbackInfo& info) const auto from = slopsmith::addon::argSlotId(info, 0); const auto to = slopsmith::addon::argSlotId(info, 1); if (liveEngine && from && to) + { + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); liveEngine->getSignalChain().moveProcessor(*from, *to); + slopsmith::addon::bumpChainGeneration(); + } return info.Env().Undefined(); } @@ -2451,7 +2472,15 @@ static Napi::Value ClearChain(const Napi::CallbackInfo& info) { // Tear editors down before their processors are freed just below (#56). closeAllPluginEditorWindows(); - if (auto liveEngine = snapshotEngine()) liveEngine->getSignalChain().clear(); + if (auto liveEngine = snapshotEngine()) + { + // Serialized with the async chain workers (deep-read 1). May block + // briefly behind an in-flight preset/VST load -- that wait IS the fix + // for the interleaved clear-vs-rebuild corruption. + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); + liveEngine->getSignalChain().clear(); + slopsmith::addon::bumpChainGeneration(); + } return info.Env().Undefined(); } @@ -2507,6 +2536,14 @@ static Napi::Value SetBranchSrc(const Napi::CallbackInfo& info) // ── Chain State ─────────────────────────────────────────────────────────────── +// Monotonic chain-mutation counter (TLC phase 7): JS-side chain owners (the +// audio-effects executor) compare this against the generation their load +// returned to detect that another writer changed the chain under them. +static Napi::Value GetChainGeneration(const Napi::CallbackInfo& info) +{ + return Napi::Number::New(info.Env(), (double) slopsmith::addon::currentChainGeneration()); +} + static Napi::Value GetChainState(const Napi::CallbackInfo& info) { auto env = info.Env(); @@ -3077,6 +3114,9 @@ public: void Execute() override { + // Serialize the FULL mutation (TLC deep-read 1): overlapping chain + // workers on the libuv pool must not interleave clear()/addProcessor(). + std::lock_guard chainLock(slopsmith::addon::chainMutationMutex()); auto liveEngine = snapshotEngine(); if (!liveEngine) { success_ = false; error_ = "No engine"; return; } @@ -3188,8 +3228,9 @@ public: juce::MemoryBlock state; if (decodeStateBlob(stateB64, state, allowStandard)) { - auto* slot = const_cast(liveEngine->getSignalChain().getSlot(slotId)); - if (slot) slot->setState(state); + // Through the class's own synchronized API (deep-read 9) -- + // no more const_cast around setSlotState's locking. + liveEngine->getSignalChain().setSlotState(slotId, state); } } @@ -3197,6 +3238,7 @@ public: } success_ = true; + generation_ = slopsmith::addon::bumpChainGeneration(); // still under chainLock } void OnOK() override @@ -3204,6 +3246,7 @@ public: auto obj = Napi::Object::New(Env()); obj.Set("success", success_); obj.Set("slotsLoaded", slotsLoaded_); + obj.Set("chainGeneration", (double) generation_); if (!success_) obj.Set("error", error_); deferred_.Resolve(obj); } @@ -3212,6 +3255,7 @@ public: private: Napi::Promise::Deferred deferred_; std::string presetJson_; + uint64_t generation_ = 0; bool success_ = false; std::string error_; int slotsLoaded_ = 0; @@ -3458,6 +3502,7 @@ static Napi::Object InitModule(Napi::Env env, Napi::Object exports) exports.Set("setBranchSrc", Napi::Function::New(env, SetBranchSrc)); exports.Set("clearChain", Napi::Function::New(env, ClearChain)); exports.Set("getChainState", Napi::Function::New(env, GetChainState)); + exports.Set("getChainGeneration", Napi::Function::New(env, GetChainGeneration)); exports.Set("openPluginEditor", Napi::Function::New(env, OpenPluginEditor)); exports.Set("closePluginEditor", Napi::Function::New(env, ClosePluginEditor)); diff --git a/src/audio/addon/ChainOps.cpp b/src/audio/addon/ChainOps.cpp new file mode 100644 index 0000000..3dbe091 --- /dev/null +++ b/src/audio/addon/ChainOps.cpp @@ -0,0 +1,25 @@ +#include "ChainOps.h" + +#include + +namespace slopsmith::addon { + +std::mutex& chainMutationMutex() +{ + static std::mutex m; + return m; +} + +static std::atomic chainGeneration{0}; + +uint64_t bumpChainGeneration() +{ + return chainGeneration.fetch_add(1, std::memory_order_acq_rel) + 1; +} + +uint64_t currentChainGeneration() +{ + return chainGeneration.load(std::memory_order_acquire); +} + +} // namespace slopsmith::addon diff --git a/src/audio/addon/ChainOps.h b/src/audio/addon/ChainOps.h new file mode 100644 index 0000000..207636b --- /dev/null +++ b/src/audio/addon/ChainOps.h @@ -0,0 +1,44 @@ +#pragma once + +// ChainOps — the native chain-mutation serialization point (TLC plan phase 7 +// / §3.3, deep-read §1). +// +// The five chain-mutating async workers (LoadPreset/LoadVST/LoadNAM/LoadIR/ +// ReplaceIR) queue on the libuv threadpool with no mutual exclusion, and +// SignalChain locks per-operation only — so two overlapping loadPreset calls +// could interleave clear()/addProcessor() and merge both presets into +// garbage (the documented rig_builder-vs-bundle "~1ms later" race). One +// mutex held across each worker's FULL Execute() — and across the +// synchronous mutators (clearChain / remove / move) — converts that +// corruption into last-writer-wins. +// +// chainGeneration is bumped on every completed mutation and returned in the +// load results (and via getChainGeneration), so JS-side owners (the +// audio-effects executor's stageSlots map) can detect that another writer +// changed the chain under them and re-sync instead of flipping bypass/params +// on the wrong slots. +// +// The full worker bodies migrate into this unit with the phase-7 binding +// split; the serializer lands first so the storm gate flips. + +#include +#include + +namespace slopsmith::addon { + +// Held for the FULL clear+rebuild (or single-slot mutation). Control/worker +// threads only — never the audio thread. +std::mutex& chainMutationMutex(); + +// Monotonic, bumped AFTER a completed mutation (under the mutex). 0 = never +// mutated. +uint64_t bumpChainGeneration(); +uint64_t currentChainGeneration(); + +// Usage in a mutator: +// std::lock_guard chainLock(chainMutationMutex()); +// ... clear/rebuild/add ... +// const uint64_t gen = bumpChainGeneration(); // still under the lock +// (return gen in the result object) + +} // namespace slopsmith::addon diff --git a/tests/chain-mutation-storm.test.js b/tests/chain-mutation-storm.test.js index cb6c547..e6301c7 100644 --- a/tests/chain-mutation-storm.test.js +++ b/tests/chain-mutation-storm.test.js @@ -46,7 +46,7 @@ function irPreset(irFile, slotCount) { }); } -test('concurrent loadPreset calls end with exactly one caller\'s chain', { skip: !ENABLED && 'quarantined — set CHAIN_STORM=1 (expected-fail until ChainOps serializer)' }, async () => { +test('concurrent loadPreset calls end with exactly one caller\'s chain', { skip: !ENABLED && 'needs built addon — set CHAIN_STORM=1 (hard gate since the phase-7 serializer)' }, async () => { assert.ok(fs.existsSync(ADDON), 'addon must be built (npm run build:audio)'); const audio = require(ADDON); audio.init(); // returns undefined; loadPreset fails "No engine" if it didn't take diff --git a/tests/contracts/addon-exports.json b/tests/contracts/addon-exports.json index c1a6da0..aac4fa5 100644 --- a/tests/contracts/addon-exports.json +++ b/tests/contracts/addon-exports.json @@ -10,6 +10,7 @@ "getBackingLevel", "getBackingPosition", "getBufferSizes", + "getChainGeneration", "getChainState", "getCurrentDevice", "getDeviceMetrics",