diff --git a/src/audio/CMakeLists.txt b/src/audio/CMakeLists.txt index 1c44e19..f20a6a1 100644 --- a/src/audio/CMakeLists.txt +++ b/src/audio/CMakeLists.txt @@ -11,6 +11,7 @@ set(AUDIO_SOURCES engine/DeviceSetup.cpp engine/SourcePool.cpp engine/ExtraInputs.cpp + addon/AddonContext.cpp SourceChain.cpp SignalChain.cpp VSTHost.cpp diff --git a/src/audio/NodeAddon.cpp b/src/audio/NodeAddon.cpp index 358e193..766da8a 100644 --- a/src/audio/NodeAddon.cpp +++ b/src/audio/NodeAddon.cpp @@ -18,10 +18,6 @@ #include "AudioEngine.h" #include "VSTHost.h" -// Forward declaration — defined alongside loadVstSandboxAware further down. -// doShutdown (below) needs it to release any LoadVSTWorker / LoadPreset- -// Worker blocked on a pending async load before the message thread stops. -static void cancelAllPendingLoads(); #include "VSTTrace.h" #include "NAMProcessor.h" #include "IRLoader.h" @@ -30,29 +26,18 @@ static void cancelAllPendingLoads(); #include -// engine / vstHost — shared_ptr (not unique_ptr) so worker threads can take -// a stable snapshot that keeps the object alive for the duration of their -// work, even if the message thread reassigns the global mid-operation. This -// matters most for the async VST load: createPluginInstanceAsync's JUCE -// continuation must not have VSTHost / its formatManager torn out from -// under it mid-load. -// -// snapshotEngine() / snapshotVstHost() take the global under the matching -// mutex and return a private copy. The only code permitted to touch the -// bare `engine` / `vstHost` globals is the snapshot helpers below and the -// mutex-guarded writes in Init / doShutdown — the message thread mutates -// the globals there while worker threads and the napi handlers read them. -// -// Enforced rule: every *dereference* of engine / vstHost goes through a -// local snapshot. A napi handler takes that snapshot at the top, null- -// checks it, and uses only the local — either for the rest of its body, -// or (for the handlers that hand off to an AsyncWorker — LoadVST, -// LoadNAMModel, LoadIR, LoadPreset) purely as the availability guard -// before queuing, with the worker re-snapshotting on its own thread. -// Either way a concurrent doShutdown reset can never pull the object out -// from under an in-flight dereference. -static std::shared_ptr engine; -static std::mutex engineMutex; +#include "addon/AddonContext.h" +#include "addon/NapiHelpers.h" + +// Lifetime/threading moved to addon/AddonContext (TLC phase 6); the usings +// keep the 100+ existing binding bodies unchanged. +using slopsmith::addon::snapshotEngine; +using slopsmith::addon::snapshotVstHost; +using slopsmith::addon::dispatchOnMessageThread; +using slopsmith::addon::registerPendingLoad; +using slopsmith::addon::unregisterPendingLoad; +using slopsmith::addon::cancelAllPendingLoads; +using slopsmith::addon::doShutdown; // Decode a state blob that may be in EITHER base64 flavour. JUCE's // MemoryBlock::fromBase64Encoding only understands JUCE's own proprietary @@ -80,12 +65,6 @@ static bool decodeStateBlob(const juce::String& s, juce::MemoryBlock& mb, return juce::Base64::convertFromBase64(mo, s) && mb.getSize() > 0; } -static std::shared_ptr snapshotEngine() -{ - std::lock_guard lock(engineMutex); - return engine; -} - // Validate a JS source-id argument and return the live source, or nullptr if it is // missing / not a Number / not a FINITE INTEGER / out of range. The TS bridge already // validates, but the addon must fail soft on its own: Int32Value() silently coerces @@ -102,15 +81,6 @@ static SourceChain* getValidatedSource(AudioEngine* eng, const Napi::CallbackInf return eng->getSource((int) raw); } -static std::shared_ptr vstHost; -static std::mutex vstHostMutex; - -static std::shared_ptr snapshotVstHost() -{ - std::lock_guard lock(vstHostMutex); - return vstHost; -} - static double loadSafeSampleRate(const AudioEngine& eng) { const double sr = eng.getCurrentSampleRate(); @@ -123,79 +93,6 @@ static int loadSafeBlockSize(const AudioEngine& eng) return bs > 0 ? bs : 256; } -static std::thread juceMessageThread; -static std::atomic juceRunning{false}; -static std::atomic alreadyShutDown{false}; - -// ── JUCE Message Thread ─────────────────────────────────────────────────────── -// JUCE requires a message thread for plugin loading, audio device management, etc. -// We pump it in a dedicated thread. - -static void startJuceMessageThread() -{ - if (juceRunning.load()) return; - juceRunning.store(true); - -#if JUCE_MAC - // On macOS, JUCE's MessageManager::runDispatchLoopUntil internally calls - // `-[NSApplication _nextEventMatchingEventMask:...]`, which AppKit asserts - // must run on the true main thread. Node.js already owns the main thread - // (running libuv's event loop), so we can't spawn a second NS event pump - // without hitting `nextEventMatchingMask should only be called from the - // Main Thread!` and aborting. - // - // Workaround: designate Node's current thread as JUCE's message thread and - // skip the dispatch loop. callAsync()'d callbacks will still queue; we - // drain them from the Node thread via a libuv timer created below. - juce::MessageManager::getInstance(); -#else - juceMessageThread = std::thread([]() { - juce::MessageManager::getInstance(); - while (juceRunning.load()) - { - juce::MessageManager::getInstance()->runDispatchLoopUntil(50); - } - juce::MessageManager::deleteInstance(); - }); -#endif -} - -static void stopJuceMessageThread() -{ - juceRunning.store(false); -#if !JUCE_MAC - if (juceMessageThread.joinable()) - juceMessageThread.join(); -#else - juce::MessageManager::deleteInstance(); -#endif -} - -// ── Helper: dispatch on JUCE message thread ─────────────────────────────────── - -template -static void dispatchOnMessageThread(Func&& func) -{ -#if JUCE_MAC - // No background message thread on macOS — execute inline on caller thread. - // Audio device / NAM / IR init is thread-safe for our use; VST/AU plugin - // instantiation (which genuinely requires a message thread on macOS) is - // the one capability we give up until a proper libuv-based pump lands. - func(); -#else - // Heap-allocate the WaitableEvent and capture by value so the queued - // callAsync closure can outlive this stack frame. Without this, a 15 s - // timeout (rare, but possible during shutdown when the message thread is - // busy) leaves the lambda running on freed `done` storage — a real UAF. - auto done = std::make_shared(); - juce::MessageManager::callAsync([func = std::forward(func), done]() mutable { - func(); - done->signal(); - }); - done->wait(15000); -#endif -} - // Destroys every in-process plugin editor window. MUST be called on the message // thread (editorWindows holds JUCE GUI objects). Defined far below, after the // editorWindows map; forward-declared here so doShutdown — which already runs on @@ -207,99 +104,10 @@ static void destroyAllPluginEditorWindowsOnMessageThread(); static Napi::Value Init(const Napi::CallbackInfo& info) { - auto env = info.Env(); - - // Reset the shutdown latch so a JS-level init→shutdown→init cycle (e.g. - // a test harness recreating the engine) actually runs shutdown again - // instead of treating it as already-done. - alreadyShutDown.store(false, std::memory_order_release); - - // Start JUCE message thread first (no-op on macOS — see startJuceMessageThread) - startJuceMessageThread(); - -#if !JUCE_MAC - // Small delay to ensure message thread is pumping - std::this_thread::sleep_for(std::chrono::milliseconds(200)); -#endif - - // Create engine on the JUCE message thread (or inline on macOS) - dispatchOnMessageThread([]() { - std::shared_ptr liveEngine; - { - std::lock_guard lock(engineMutex); - engine = std::make_shared(); - liveEngine = engine; - } - { - std::lock_guard lock(vstHostMutex); - vstHost = std::make_shared(); - } - - auto types = liveEngine->getDeviceTypes(); - fprintf(stderr, "[audio-native] Init complete. Device types: %d\n", types.size()); - for (int i = 0; i < types.size(); ++i) - fprintf(stderr, "[audio-native] %s: %d inputs, %d outputs\n", - types[i].name.toRawUTF8(), - types[i].inputDevices.size(), - types[i].outputDevices.size()); - }); - - return env.Undefined(); -} - -static void doShutdown() -{ - // The latch is flipped at the TOP rather than the bottom so a - // re-entrant call (e.g. env-cleanup-hook firing while a JS-level - // shutdown is mid-flight) bails immediately rather than racing on - // the same teardown sequence. Assumed serialisation invariants: - // - dispatchOnMessageThread is single-writer to engine/vstHost - // (both unique_ptrs touched only here or from Init); - // - stopJuceMessageThread is idempotent and safe to call when the - // thread was never started (defensive checks inside). - // If a future caller mutates engine/vstHost between this latch and - // the dispatch (or the dispatch's 15s wait times out), THIS call's - // body may not finish before returning — but the re-entrant - // cleanup-hook will then no-op via the latch and the dispatch - // queue itself unwinds whatever's pending. Net result: at-most- - // once execution of the gated body, even under teardown races. - bool expected = false; - if (!alreadyShutDown.compare_exchange_strong(expected, true)) return; - - // Release any LoadVSTWorker / LoadPresetWorker currently blocked on a - // pending async load. Without this they'd wait forever on the - // WaitableEvent — the createPluginInstanceAsync callback can't fire - // once the message thread is gone. Forward-declared above; the - // implementation lives near loadVstSandboxAware. - cancelAllPendingLoads(); - - if (juceRunning.load() || snapshotEngine() || snapshotVstHost()) - { - dispatchOnMessageThread([]() { - // Editors reference their slot's processor; engine.reset() below - // frees the whole chain, so destroy the editor windows first (#56). - // Already on the message thread here — call the inline variant - // directly (closeAllPluginEditorWindows() would reach the same code - // via its message-thread branch; this just skips the thread check). - destroyAllPluginEditorWindowsOnMessageThread(); - if (auto liveEngine = snapshotEngine()) - liveEngine->stopAudio(); - { - std::lock_guard lock(engineMutex); - engine.reset(); - } - { - std::lock_guard lock(vstHostMutex); - vstHost.reset(); - } - }); - } - - stopJuceMessageThread(); - - // Restore the previous top-level exception filter — the addon (and thus our - // unhandledFilter's code) may be unloaded, so it must not stay installed. - slopsmith::sandbox::uninstallVstCrashAttribution(); + // Engine/vstHost creation + message-thread start live on AddonContext; + // the UI teardown hook runs at shutdown BEFORE engine.reset() (#56). + slopsmith::addon::initialize([] { destroyAllPluginEditorWindowsOnMessageThread(); }); + return info.Env().Undefined(); } static Napi::Value Shutdown(const Napi::CallbackInfo& info) @@ -628,8 +436,11 @@ static Napi::Value SetGain(const Napi::CallbackInfo& info) auto liveEngine = snapshotEngine(); if (!liveEngine || info.Length() < 2) return env.Undefined(); + if (!info[0].IsString()) return env.Undefined(); auto which = info[0].As().Utf8Value(); - float value = info[1].As().FloatValue(); + const auto valueOpt = slopsmith::addon::argFiniteFloat(info, 1); + if (!valueOpt) return env.Undefined(); // engine clamps range; NaN/Inf rejected here + const float value = *valueOpt; if (which == "input") liveEngine->setInputGain(value); else if (which == "output") liveEngine->setOutputGain(value); @@ -2028,28 +1839,6 @@ static Napi::Value SetVstCrashSentinelPath(const Napi::CallbackInfo& info) // signals them all so the workers unblock and return a clean "cancelled" // error instead of hanging forever when the JUCE message thread is about // to be stopped (and any unfired callback would never arrive). -static std::mutex pendingLoadsMutex; -static std::set> pendingLoads; - -static void registerPendingLoad(std::shared_ptr evt) -{ - std::lock_guard lock(pendingLoadsMutex); - pendingLoads.insert(std::move(evt)); -} - -static void unregisterPendingLoad(const std::shared_ptr& evt) -{ - std::lock_guard lock(pendingLoadsMutex); - pendingLoads.erase(evt); -} - -static void cancelAllPendingLoads() -{ - std::lock_guard lock(pendingLoadsMutex); - for (auto& evt : pendingLoads) evt->signal(); - pendingLoads.clear(); -} - // Load a VST3, routing it through the out-of-process sandbox when // shouldSandbox() says so (the filename pre-seed or the runtime crash // blocklist), otherwise loading it in-process. The in-process load uses @@ -2171,7 +1960,7 @@ static std::unique_ptr loadVstSandboxAware( // Check alreadyShutDown after registering to catch the inverse race // (shutdown ran before we registered): if it's already set, the // shutdown won't see this event and we must bail ourselves. - if (alreadyShutDown.load(std::memory_order_acquire)) + if (slopsmith::addon::isShuttingDown()) { unregisterPendingLoad(done); error = "shutdown in flight"; @@ -2195,7 +1984,7 @@ static std::unique_ptr loadVstSandboxAware( // lambda and the message thread picking it up. Bail before // kicking off another in-flight createPluginInstanceAsync // that the shutdown would otherwise have to wait on. - if (alreadyShutDown.load(std::memory_order_acquire)) + if (slopsmith::addon::isShuttingDown()) { *loadError = "shutdown in flight"; done->signal(); @@ -2282,7 +2071,7 @@ public: // mid-load. The atomic alreadyShutDown gate is the early-out: once // it's set, the dispatched reset is on its way and there's no point // continuing. - if (alreadyShutDown.load(std::memory_order_acquire)) + if (slopsmith::addon::isShuttingDown()) { error_ = "shutdown in flight"; return; @@ -2333,7 +2122,7 @@ public: // dispatched reset of engine/vstHost is on its way and any use of // the pointers from this worker thread is racy. The atomic check is // the authoritative "should I still be touching engine?" signal. - if (alreadyShutDown.load(std::memory_order_acquire)) + if (slopsmith::addon::isShuttingDown()) { error_ = "engine torn down during load"; return; @@ -2620,36 +2409,32 @@ static Napi::Value ReplaceIR(const Napi::CallbackInfo& info) static Napi::Value RemoveProcessor(const Napi::CallbackInfo& info) { + // Typed extractors (addon/NapiHelpers.h): NaN/Inf slot ids used to coerce + // to slot 0 and mutate the wrong slot (deep-read §2) — now a clean no-op. auto liveEngine = snapshotEngine(); - if (liveEngine && info.Length() > 0) - { - int slotId = info[0].As().Int32Value(); - liveEngine->getSignalChain().removeProcessor(slotId); - } + const auto slotId = slopsmith::addon::argSlotId(info, 0); + if (liveEngine && slotId) + liveEngine->getSignalChain().removeProcessor(*slotId); return info.Env().Undefined(); } static Napi::Value MoveProcessor(const Napi::CallbackInfo& info) { auto liveEngine = snapshotEngine(); - if (liveEngine && info.Length() >= 2) - { - int from = info[0].As().Int32Value(); - int to = info[1].As().Int32Value(); - liveEngine->getSignalChain().moveProcessor(from, to); - } + const auto from = slopsmith::addon::argSlotId(info, 0); + const auto to = slopsmith::addon::argSlotId(info, 1); + if (liveEngine && from && to) + liveEngine->getSignalChain().moveProcessor(*from, *to); return info.Env().Undefined(); } static Napi::Value SetBypass(const Napi::CallbackInfo& info) { auto liveEngine = snapshotEngine(); - if (liveEngine && info.Length() >= 2) - { - int slotId = info[0].As().Int32Value(); - bool bypassed = info[1].As().Value(); - liveEngine->getSignalChain().setBypass(slotId, bypassed); - } + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto bypassed = slopsmith::addon::argBool(info, 1); + if (liveEngine && slotId && bypassed) + liveEngine->getSignalChain().setBypass(*slotId, *bypassed); return info.Env().Undefined(); } @@ -2676,9 +2461,9 @@ static Napi::Value SetPan(const Napi::CallbackInfo& info) auto liveEngine = snapshotEngine(); if (liveEngine && info.Length() >= 2) { - int slotId = info[0].As().Int32Value(); - float pan = (float) info[1].As().DoubleValue(); - liveEngine->getSignalChain().setPan(slotId, pan); + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto pan = slopsmith::addon::argFiniteFloat(info, 1); + if (slotId && pan) liveEngine->getSignalChain().setPan(*slotId, *pan); } return info.Env().Undefined(); } @@ -2688,9 +2473,9 @@ static Napi::Value SetPostGain(const Napi::CallbackInfo& info) auto liveEngine = snapshotEngine(); if (liveEngine && info.Length() >= 2) { - int slotId = info[0].As().Int32Value(); - float gain = (float) info[1].As().DoubleValue(); - liveEngine->getSignalChain().setPostGain(slotId, gain); + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto gain = slopsmith::addon::argFiniteFloat(info, 1); + if (slotId && gain) liveEngine->getSignalChain().setPostGain(*slotId, *gain); } return info.Env().Undefined(); } @@ -2700,9 +2485,9 @@ static Napi::Value SetBranch(const Napi::CallbackInfo& info) auto liveEngine = snapshotEngine(); if (liveEngine && info.Length() >= 2) { - int slotId = info[0].As().Int32Value(); - int branch = info[1].As().Int32Value(); - liveEngine->getSignalChain().setBranch(slotId, branch); + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto branch = slopsmith::addon::argInt(info, 1); + if (slotId && branch) liveEngine->getSignalChain().setBranch(*slotId, *branch); } return info.Env().Undefined(); } @@ -2713,9 +2498,9 @@ static Napi::Value SetBranchSrc(const Napi::CallbackInfo& info) auto liveEngine = snapshotEngine(); if (liveEngine && info.Length() >= 2) { - int slotId = info[0].As().Int32Value(); - int src = info[1].As().Int32Value(); - liveEngine->getSignalChain().setBranchSrc(slotId, src); + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto branchSrc = slopsmith::addon::argInt(info, 1, 0, 2); + if (slotId && branchSrc) liveEngine->getSignalChain().setBranchSrc(*slotId, *branchSrc); } return info.Env().Undefined(); } @@ -3138,13 +2923,11 @@ static Napi::Value GetParameters(const Napi::CallbackInfo& info) static Napi::Value SetParameter(const Napi::CallbackInfo& info) { auto liveEngine = snapshotEngine(); - if (liveEngine && info.Length() >= 3) - { - int slotId = info[0].As().Int32Value(); - int paramIdx = info[1].As().Int32Value(); - float value = info[2].As().FloatValue(); - liveEngine->getSignalChain().setParameter(slotId, paramIdx, value); - } + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto paramIdx = slopsmith::addon::argSlotId(info, 1); + const auto value = slopsmith::addon::argFiniteFloat(info, 2); + if (liveEngine && slotId && paramIdx && value) + liveEngine->getSignalChain().setParameter(*slotId, *paramIdx, *value); return info.Env().Undefined(); } @@ -3178,26 +2961,30 @@ static Napi::Value SendMidiToSlot(const Napi::CallbackInfo& info) if (!liveEngine || info.Length() < 4) return Napi::Boolean::New(env, false); - int slotId = info[0].As().Int32Value(); - int msgType = info[1].As().Int32Value(); - int channel = info[2].As().Int32Value(); - - juce::MidiMessage midiMsg; - if (msgType == 0) // Program Change - { - int program = info[3].As().Int32Value(); - midiMsg = juce::MidiMessage::programChange(channel, program); - } - else if (msgType == 1) // Control Change - { - int controller = info[3].As().Int32Value(); - int value = info.Length() > 4 ? info[4].As().Int32Value() : 0; - midiMsg = juce::MidiMessage::controllerEvent(channel, controller, value); - } - else + // Typed + range-checked: unclamped channel/program used to trip JUCE + // assertions (deep-read §2). Out-of-range now returns false cleanly. + const auto slotId = slopsmith::addon::argSlotId(info, 0); + const auto msgType = slopsmith::addon::argInt(info, 1, 0, 1); + const auto channel = slopsmith::addon::argMidiChannel(info, 2); + if (!slotId || !msgType || !channel) return Napi::Boolean::New(env, false); - liveEngine->getSignalChain().queueMidiMessage(slotId, midiMsg); + juce::MidiMessage midiMsg; + if (*msgType == 0) // Program Change + { + const auto program = slopsmith::addon::argMidiByte(info, 3); + if (!program) return Napi::Boolean::New(env, false); + midiMsg = juce::MidiMessage::programChange(*channel, *program); + } + else // Control Change + { + const auto controller = slopsmith::addon::argMidiByte(info, 3); + if (!controller) return Napi::Boolean::New(env, false); + const auto value = slopsmith::addon::argMidiByte(info, 4); + midiMsg = juce::MidiMessage::controllerEvent(*channel, *controller, value.value_or(0)); + } + + liveEngine->getSignalChain().queueMidiMessage(*slotId, midiMsg); return Napi::Boolean::New(env, true); } @@ -3264,8 +3051,10 @@ static Napi::Value SetBackingSpeed(const Napi::CallbackInfo& info) .ThrowAsJavaScriptException(); return env.Undefined(); } - if (engine) - engine->setBackingSpeed(info[0].As().DoubleValue()); + // (Was a bare `engine` dereference — the one binding that dodged the + // file's own snapshot rule; surfaced by the phase-6 move.) + if (auto liveEngine = snapshotEngine()) + liveEngine->setBackingSpeed(info[0].As().DoubleValue()); return env.Undefined(); } @@ -3469,10 +3258,17 @@ static Napi::Value SetMultiBypass(const Napi::CallbackInfo& info) for (uint32_t i = 0; i < arr.Length(); i++) { - auto item = arr.Get(i).As(); - int slotId = item.Get("slotId").As().Int32Value(); - bool bypassed = item.Get("bypassed").As().Value(); - changes.add({ slotId, bypassed }); + // Per-item type guards (deep-read §2): a malformed entry is skipped + // instead of coercing NaN to slot 0. + auto itemVal = arr.Get(i); + if (!itemVal.IsObject()) continue; + auto item = itemVal.As(); + auto slotVal = item.Get("slotId"); + auto bypVal = item.Get("bypassed"); + if (!slotVal.IsNumber() || !bypVal.IsBoolean()) continue; + const double raw = slotVal.As().DoubleValue(); + if (!std::isfinite(raw) || raw != std::floor(raw) || raw < 0.0 || raw > 4096.0) continue; + changes.add({ (int) raw, bypVal.As().Value() }); } liveEngine->getSignalChain().setMultiBypass(changes); diff --git a/src/audio/addon/AddonContext.cpp b/src/audio/addon/AddonContext.cpp new file mode 100644 index 0000000..b2050e5 --- /dev/null +++ b/src/audio/addon/AddonContext.cpp @@ -0,0 +1,228 @@ +// AddonContext implementation — moved verbatim from NodeAddon.cpp (TLC plan +// phase 6 / §3.1). See AddonContext.h for the lifetime rules. + +#include "AddonContext.h" + +#include "../Sandbox/CrashAttribution.h" + +#include +#include +#include +#include + +namespace slopsmith::addon { + +static std::shared_ptr engine; +static std::mutex engineMutex; +static std::shared_ptr vstHost; +static std::mutex vstHostMutex; + +static std::thread juceMessageThread; +static std::atomic juceRunning{false}; +static std::atomic alreadyShutDown{false}; + +// Runs on the message thread at the start of shutdown, before engine.reset() +// frees the processors any editor windows point at (#56). Set by initialize(). +static std::function shutdownUiTeardown; + +std::shared_ptr snapshotEngine() +{ + std::lock_guard lock(engineMutex); + return engine; +} + +std::shared_ptr snapshotVstHost() +{ + std::lock_guard lock(vstHostMutex); + return vstHost; +} + +// ── JUCE Message Thread ────────────────────────────────────────────────────── +// JUCE requires a message thread for plugin loading, audio device management, +// etc. We pump it in a dedicated thread. + +static void startJuceMessageThread() +{ + if (juceRunning.load()) return; + juceRunning.store(true); + +#if JUCE_MAC + // On macOS, JUCE's MessageManager::runDispatchLoopUntil internally calls + // `-[NSApplication _nextEventMatchingEventMask:...]`, which AppKit asserts + // must run on the true main thread. Node.js already owns the main thread + // (running libuv's event loop), so we can't spawn a second NS event pump + // without hitting `nextEventMatchingMask should only be called from the + // Main Thread!` and aborting. + // + // Workaround: designate Node's current thread as JUCE's message thread and + // skip the dispatch loop. callAsync()'d callbacks will still queue; we + // drain them from the Node thread via a libuv timer created below. + juce::MessageManager::getInstance(); +#else + juceMessageThread = std::thread([]() { + juce::MessageManager::getInstance(); + while (juceRunning.load()) + { + juce::MessageManager::getInstance()->runDispatchLoopUntil(50); + } + juce::MessageManager::deleteInstance(); + }); +#endif +} + +static void stopJuceMessageThread() +{ + juceRunning.store(false); +#if !JUCE_MAC + if (juceMessageThread.joinable()) + juceMessageThread.join(); +#else + juce::MessageManager::deleteInstance(); +#endif +} + +void dispatchOnMessageThreadImpl(std::function func) +{ +#if JUCE_MAC + // No background message thread on macOS — execute inline on caller thread. + // Audio device / NAM / IR init is thread-safe for our use; VST/AU plugin + // instantiation (which genuinely requires a message thread on macOS) is + // the one capability we give up until a proper libuv-based pump lands. + func(); +#else + // Heap-allocate the WaitableEvent and capture by value so the queued + // callAsync closure can outlive this stack frame. Without this, a 15 s + // timeout (rare, but possible during shutdown when the message thread is + // busy) leaves the lambda running on freed `done` storage — a real UAF. + auto done = std::make_shared(); + juce::MessageManager::callAsync([func = std::move(func), done]() mutable { + func(); + done->signal(); + }); + done->wait(15000); +#endif +} + +// ── Pending async loads ────────────────────────────────────────────────────── + +static std::mutex pendingLoadsMutex; +static std::set> pendingLoads; + +bool isShuttingDown() +{ + return alreadyShutDown.load(std::memory_order_acquire); +} + +void registerPendingLoad(std::shared_ptr evt) +{ + std::lock_guard lock(pendingLoadsMutex); + pendingLoads.insert(std::move(evt)); +} + +void unregisterPendingLoad(const std::shared_ptr& evt) +{ + std::lock_guard lock(pendingLoadsMutex); + pendingLoads.erase(evt); +} + +void cancelAllPendingLoads() +{ + std::lock_guard lock(pendingLoadsMutex); + for (auto& evt : pendingLoads) evt->signal(); + pendingLoads.clear(); +} + +// ── Lifecycle ──────────────────────────────────────────────────────────────── + +void initialize(std::function uiTeardownHook) +{ + shutdownUiTeardown = std::move(uiTeardownHook); + + // Reset the shutdown latch so a JS-level init→shutdown→init cycle (e.g. + // a test harness recreating the engine) actually runs shutdown again + // instead of treating it as already-done. + alreadyShutDown.store(false, std::memory_order_release); + + // Start JUCE message thread first (no-op on macOS) + startJuceMessageThread(); + +#if !JUCE_MAC + // Small delay to ensure message thread is pumping + std::this_thread::sleep_for(std::chrono::milliseconds(200)); +#endif + + // Create engine on the JUCE message thread (or inline on macOS) + dispatchOnMessageThread([]() { + std::shared_ptr liveEngine; + { + std::lock_guard lock(engineMutex); + engine = std::make_shared(); + liveEngine = engine; + } + { + std::lock_guard lock(vstHostMutex); + vstHost = std::make_shared(); + } + + auto types = liveEngine->getDeviceTypes(); + fprintf(stderr, "[audio-native] Init complete. Device types: %d\n", types.size()); + for (int i = 0; i < types.size(); ++i) + fprintf(stderr, "[audio-native] %s: %d inputs, %d outputs\n", + types[i].name.toRawUTF8(), + types[i].inputDevices.size(), + types[i].outputDevices.size()); + }); +} + +void doShutdown() +{ + // The latch is flipped at the TOP rather than the bottom so a + // re-entrant call (e.g. env-cleanup-hook firing while a JS-level + // shutdown is mid-flight) bails immediately rather than racing on + // the same teardown sequence. Assumed serialisation invariants: + // - dispatchOnMessageThread is single-writer to engine/vstHost + // (both touched only here or from initialize); + // - stopJuceMessageThread is idempotent and safe to call when the + // thread was never started (defensive checks inside). + // If a future caller mutates engine/vstHost between this latch and + // the dispatch (or the dispatch's 15s wait times out), THIS call's + // body may not finish before returning — but the re-entrant + // cleanup-hook will then no-op via the latch and the dispatch + // queue itself unwinds whatever's pending. Net result: at-most- + // once execution of the gated body, even under teardown races. + bool expected = false; + if (!alreadyShutDown.compare_exchange_strong(expected, true)) return; + + // Release any LoadVSTWorker / LoadPresetWorker currently blocked on a + // pending async load. Without this they'd wait forever on the + // WaitableEvent — the createPluginInstanceAsync callback can't fire + // once the message thread is gone. + cancelAllPendingLoads(); + + if (juceRunning.load() || snapshotEngine() || snapshotVstHost()) + { + dispatchOnMessageThread([]() { + // Editors reference their slot's processor; engine.reset() below + // frees the whole chain, so destroy the editor windows first (#56). + if (shutdownUiTeardown) shutdownUiTeardown(); + if (auto liveEngine = snapshotEngine()) + liveEngine->stopAudio(); + { + std::lock_guard lock(engineMutex); + engine.reset(); + } + { + std::lock_guard lock(vstHostMutex); + vstHost.reset(); + } + }); + } + + stopJuceMessageThread(); + + // Restore the previous top-level exception filter — the addon (and thus our + // unhandledFilter's code) may be unloaded, so it must not stay installed. + slopsmith::sandbox::uninstallVstCrashAttribution(); +} + +} // namespace slopsmith::addon diff --git a/src/audio/addon/AddonContext.h b/src/audio/addon/AddonContext.h new file mode 100644 index 0000000..879da2a --- /dev/null +++ b/src/audio/addon/AddonContext.h @@ -0,0 +1,66 @@ +#pragma once + +// AddonContext — engine/vstHost lifetime, the JUCE message thread, the +// shutdown latch, and the pending-async-load registry (TLC plan phase 6 / +// §3.1). Moved verbatim from NodeAddon.cpp; this quarantines the JUCE_MAC +// platform fork (no dispatch loop — see startJuceMessageThread) into ONE +// file instead of a branch inside every load path. +// +// engine / vstHost — shared_ptr (not unique_ptr) so worker threads can take +// a stable snapshot that keeps the object alive for the duration of their +// work, even if the message thread reassigns the global mid-operation. This +// matters most for the async VST load: createPluginInstanceAsync's JUCE +// continuation must not have VSTHost / its formatManager torn out from +// under it mid-load. +// +// Enforced rule: every *dereference* of engine / vstHost goes through a +// local snapshot (snapshotEngine / snapshotVstHost). The only code touching +// the bare globals is the snapshot helpers and the mutex-guarded writes in +// initialize / doShutdown. + +#include "../AudioEngine.h" +#include "../VSTHost.h" + +#include + +#include +#include +#include +#include + +namespace slopsmith::addon { + +std::shared_ptr snapshotEngine(); +std::shared_ptr snapshotVstHost(); + +// Start the pump + create engine/vstHost on the message thread (inline on +// macOS). `uiTeardownHook` runs on the message thread at the START of +// shutdown, BEFORE engine.reset() frees the processors — NodeAddon points it +// at destroyAllPluginEditorWindowsOnMessageThread (use-after-free; #56). +void initialize(std::function uiTeardownHook); +void doShutdown(); + +// Dispatch `func` on the JUCE message thread and wait (bounded 15 s). +// macOS: executes inline on the caller thread — no background pump exists +// (AppKit owns the real main thread; see the fork note in the .cpp). +void dispatchOnMessageThreadImpl(std::function func); +template +inline void dispatchOnMessageThread(Func&& func) +{ + dispatchOnMessageThreadImpl(std::function(std::forward(func))); +} + +// Pending-async-load registry: LoadVSTWorker / LoadPresetWorker block on a +// WaitableEvent until the message-thread continuation fires; doShutdown +// signals every registered event so no worker waits forever once the pump +// is gone. +// Whether doShutdown has begun (acquire). The load workers gate on this +// after registering their pending event, catching the register-vs-shutdown +// race in both directions. +bool isShuttingDown(); + +void registerPendingLoad(std::shared_ptr evt); +void unregisterPendingLoad(const std::shared_ptr& evt); +void cancelAllPendingLoads(); + +} // namespace slopsmith::addon diff --git a/src/audio/addon/NapiHelpers.h b/src/audio/addon/NapiHelpers.h new file mode 100644 index 0000000..e52511d --- /dev/null +++ b/src/audio/addon/NapiHelpers.h @@ -0,0 +1,66 @@ +#pragma once + +// NapiHelpers — typed N-API argument extractors (TLC plan phase 6 / §3.2). +// Generalizes the getValidatedSource pattern so argument validation is +// structural, not per-binding: Int32Value() silently coerces NaN/Infinity +// into a valid index (NaN → 0), which let a malformed slot id hit a REAL +// slot (deep-read §2). Every extractor returns nullopt for a missing / +// non-Number / non-finite / out-of-range argument, and the binding no-ops — +// fail-soft, matching the addon's NAPI_DISABLE_CPP_EXCEPTIONS posture. +// +// New bindings should have no raw As() path to copy. + +#include + +#include +#include + +namespace slopsmith::addon { + +// Finite integer in [minV, maxV]. The 4096 default ceiling keeps the cast +// well-defined for id-shaped args (slot ids, source ids, indices). +inline std::optional argInt(const Napi::CallbackInfo& info, size_t i, + int minV = 0, int maxV = 4096) +{ + if (i >= info.Length() || ! info[i].IsNumber()) return std::nullopt; + const double raw = info[i].As().DoubleValue(); + if (! std::isfinite(raw) || raw != std::floor(raw)) return std::nullopt; + if (raw < (double) minV || raw > (double) maxV) return std::nullopt; + return (int) raw; +} + +// Slot / source / param-index ids: finite non-negative integers. +inline std::optional argSlotId(const Napi::CallbackInfo& info, size_t i) +{ + return argInt(info, i); +} + +// Finite float (parameter values, gains, pans). Range clamping stays with +// the engine-side sanitizers (GainSanitize.h) — this only rejects the +// NaN/Inf class that coercion would otherwise let through. +inline std::optional argFiniteFloat(const Napi::CallbackInfo& info, size_t i) +{ + if (i >= info.Length() || ! info[i].IsNumber()) return std::nullopt; + const double raw = info[i].As().DoubleValue(); + if (! std::isfinite(raw)) return std::nullopt; + return (float) raw; +} + +inline std::optional argBool(const Napi::CallbackInfo& info, size_t i) +{ + if (i >= info.Length() || ! info[i].IsBoolean()) return std::nullopt; + return info[i].As().Value(); +} + +// MIDI channel: JUCE expects 1..16 and asserts otherwise. +inline std::optional argMidiChannel(const Napi::CallbackInfo& info, size_t i) +{ + return argInt(info, i, 1, 16); +} +// MIDI data byte (program / controller / value): 0..127. +inline std::optional argMidiByte(const Napi::CallbackInfo& info, size_t i) +{ + return argInt(info, i, 0, 127); +} + +} // namespace slopsmith::addon diff --git a/tests/napi-arg-fuzz.test.js b/tests/napi-arg-fuzz.test.js new file mode 100644 index 0000000..1beb593 --- /dev/null +++ b/tests/napi-arg-fuzz.test.js @@ -0,0 +1,75 @@ +// Phase 6 gate (docs/audio-engine-tlc.md §5, deep-read §2): table-driven +// argument fuzz against the real addon. Every chain-mutating binding must +// treat NaN/Infinity/negative/string/missing/object arguments as a clean +// no-op — historically Int32Value() coerced NaN → 0 and mutated SLOT 0. +// Quarantined behind the addon being built (CI native lane), auto-skips +// otherwise. Uses no audio device (engine constructed, never started). +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const os = require('node:os'); +const path = require('node:path'); + +const ADDON = path.join(__dirname, '..', 'build', 'Release', 'slopsmith_audio.node'); +const HAVE_ADDON = fs.existsSync(ADDON); + +function writeImpulseWav(file) { + const buf = Buffer.alloc(44 + 128); + buf.write('RIFF', 0); buf.writeUInt32LE(36 + 128, 4); buf.write('WAVE', 8); + buf.write('fmt ', 12); buf.writeUInt32LE(16, 16); buf.writeUInt16LE(1, 20); + buf.writeUInt16LE(1, 22); buf.writeUInt32LE(48000, 24); buf.writeUInt32LE(96000, 28); + buf.writeUInt16LE(2, 32); buf.writeUInt16LE(16, 34); + buf.write('data', 36); buf.writeUInt32LE(128, 40); + buf.writeInt16LE(32767, 44); + fs.writeFileSync(file, buf); +} + +const GARBAGE = [NaN, Infinity, -Infinity, -1, 1.5, 4097, 'x', null, undefined, {}, []]; + +test('chain-mutating bindings no-op on garbage args and never touch slot 0', { skip: !HAVE_ADDON && 'addon not built' }, async () => { + const audio = require(ADDON); + audio.init(); + const tmp = fs.mkdtempSync(path.join(os.tmpdir(), 'arg-fuzz-')); + const ir = path.join(tmp, 'i.wav'); + writeImpulseWav(ir); + try { + const res = await audio.loadPreset(JSON.stringify({ + chain: [{ type: 2, name: 'fuzz-anchor', path: ir, bypassed: false }], + })); + assert.ok(res?.success, 'anchor preset must load'); + const before = JSON.stringify(audio.getChainState()); + + for (const g of GARBAGE) { + audio.setBypass(g, true); + audio.setBypass(0 /* valid id shape */, g); + audio.removeProcessor(g); + audio.moveProcessor(g, 0); + audio.moveProcessor(0, g); + audio.setParameter(g, 0, 0.5); + audio.setParameter(1, g, 0.5); + audio.setParameter(1, 0, g); + audio.setPan?.(g, 0); + audio.sendMidiToSlot(g, 0, 1, 0); + audio.sendMidiToSlot(1, g, 1, 0); + audio.sendMidiToSlot(1, 0, g, 0); + audio.sendMidiToSlot(1, 0, 1, g); + audio.setMultiBypass([{ slotId: g, bypassed: true }, g, null]); + audio.setGain('output', g); + audio.setGain(g, 1); + } + + const after = JSON.stringify(audio.getChainState()); + assert.equal(after, before, 'garbage args must not mutate any slot'); + // Sanity: a VALID call still works after the fuzz storm. + audio.setBypass(1, true); + const st = audio.getChainState(); + assert.equal(st[0]?.bypassed, true, 'valid call after fuzz must apply'); + audio.setBypass(1, false); + } finally { + await audio.clearChain?.(); + audio.shutdown?.(); + fs.rmSync(tmp, { recursive: true, force: true }); + } +});