mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 22:14:10 +00:00
fix(audio): serialize chain mutations + chainGeneration (phase 7a)
The single highest-value fix of the TLC pass (deep-read §1): one native chain-mutation mutex (addon/ChainOps) held across the FULL Execute() of every chain worker (LoadPreset/LoadVST/LoadNAM/LoadIR/ReplaceIR) and the synchronous mutators (clearChain/removeProcessor/moveProcessor). Two overlapping loadPreset calls can no longer interleave clear()/addProcessor() into a merged-garbage chain — the plugin-vs-plugin fight becomes last-writer-wins. chainGeneration (monotonic, bumped under the mutex) is returned in loadPreset results and exposed as getChainGeneration (new export, snapshot regenerated), so the audio-effects executor can detect a foreign write invalidated its stageSlots map and re-sync instead of flipping bypass/params on wrong slots — the prerequisite for the single-chain-owner ownership track. Also rides here: LoadPresetWorker's slot-state restore goes through setSlotState() instead of const_cast (deep-read §9). The phase-0 storm test flips from expected-fail to a hard gate: 50 iterations of concurrent loadPreset now always end with exactly one caller's chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
d4e0bfc272
commit
db337eaf29
@@ -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
|
||||
|
||||
+48
-3
@@ -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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<std::mutex> 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<ProcessorSlot*>(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));
|
||||
|
||||
|
||||
@@ -0,0 +1,25 @@
|
||||
#include "ChainOps.h"
|
||||
|
||||
#include <atomic>
|
||||
|
||||
namespace slopsmith::addon {
|
||||
|
||||
std::mutex& chainMutationMutex()
|
||||
{
|
||||
static std::mutex m;
|
||||
return m;
|
||||
}
|
||||
|
||||
static std::atomic<uint64_t> 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
|
||||
@@ -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 <cstdint>
|
||||
#include <mutex>
|
||||
|
||||
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<std::mutex> chainLock(chainMutationMutex());
|
||||
// ... clear/rebuild/add ...
|
||||
// const uint64_t gen = bumpChainGeneration(); // still under the lock
|
||||
// (return gen in the result object)
|
||||
|
||||
} // namespace slopsmith::addon
|
||||
@@ -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
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
"getBackingLevel",
|
||||
"getBackingPosition",
|
||||
"getBufferSizes",
|
||||
"getChainGeneration",
|
||||
"getChainState",
|
||||
"getCurrentDevice",
|
||||
"getDeviceMetrics",
|
||||
|
||||
Reference in New Issue
Block a user