got-feedBack/feedBack-desktop
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack-desktop.git synced 2026-09-10 22:14:10 +00:00
Code Issues Actions 1 Packages Projects Releases Wiki Activity

refactor(audio): extract AddonContext + NapiHelpers, guard raw N-API args (phase 6)

Browse Source 
AddonContext (src/audio/addon/): engine/vstHost lifetime + snapshot rule,
the JUCE message thread with the macOS no-pump fork quarantined into ONE
file, the shutdown latch (exposed as isShuttingDown), doShutdown with a UI
teardown hook (NodeAddon points it at the editor-window nuke, #56), and the
pending-async-load registry. NodeAddon keeps using-declarations so the
binding bodies are unchanged. Also fixes SetBackingSpeed's bare `engine`
dereference — the one binding that dodged the file's own snapshot rule.

NapiHelpers (typed extractors argInt/argSlotId/argFiniteFloat/argBool/
argMidiChannel/argMidiByte) + rewrites of the unguarded bindings — the
deep-read §2 fix, done once: SetParameter/SetBypass/RemoveProcessor/
MoveProcessor/SetMultiBypass/SendMidiToSlot/SetGain plus the St-1 routing
quartet (SetPan/SetPostGain/SetBranch/SetBranchSrc). NaN slot ids no longer
coerce to slot 0; MIDI channel/program are range-checked before JUCE.

New gate: tests/napi-arg-fuzz.test.js — table-driven garbage (NaN/Inf/
negative/string/missing/object) against the real addon; chain state must be
byte-identical after the storm and a valid call must still apply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
OmikronApex
2026-07-14 01:41:07 +02:00
co-authored by Claude Fable 5
parent 6b0bfb7be3
commit d4e0bfc272
6 changed files with 527 additions and 295 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/audio/CMakeLists.txt
+1
View File
@@ -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
src/audio/NodeAddon.cpp
+91 -295
View File
@@ -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 <juce_events/juce_events.h>
// 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<AudioEngine> 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<AudioEngine> snapshotEngine()
{
std::lock_guard<std::mutex> 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> vstHost;
static std::mutex vstHostMutex;
static std::shared_ptr<VSTHost> snapshotVstHost()
{
std::lock_guard<std::mutex> 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<bool> juceRunning{false};
static std::atomic<bool> 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 <typename Func>
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::WaitableEvent>();
juce::MessageManager::callAsync([func = std::forward<Func>(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<AudioEngine> liveEngine;
{
std::lock_guard<std::mutex> lock(engineMutex);
engine = std::make_shared<AudioEngine>();
liveEngine = engine;
}
{
std::lock_guard<std::mutex> lock(vstHostMutex);
vstHost = std::make_shared<VSTHost>();
}
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<std::mutex> lock(engineMutex);
engine.reset();
}
{
std::lock_guard<std::mutex> 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<Napi::String>().Utf8Value();
float value = info[1].As<Napi::Number>().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<std::shared_ptr<juce::WaitableEvent>> pendingLoads;
static void registerPendingLoad(std::shared_ptr<juce::WaitableEvent> evt)
{
std::lock_guard<std::mutex> lock(pendingLoadsMutex);
pendingLoads.insert(std::move(evt));
}
static void unregisterPendingLoad(const std::shared_ptr<juce::WaitableEvent>& evt)
{
std::lock_guard<std::mutex> lock(pendingLoadsMutex);
pendingLoads.erase(evt);
}
static void cancelAllPendingLoads()
{
std::lock_guard<std::mutex> 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<juce::AudioProcessor> 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<juce::AudioProcessor> 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<Napi::Number>().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<Napi::Number>().Int32Value();
int to = info[1].As<Napi::Number>().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<Napi::Number>().Int32Value();
bool bypassed = info[1].As<Napi::Boolean>().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<Napi::Number>().Int32Value();
float pan = (float) info[1].As<Napi::Number>().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<Napi::Number>().Int32Value();
float gain = (float) info[1].As<Napi::Number>().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<Napi::Number>().Int32Value();
int branch = info[1].As<Napi::Number>().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<Napi::Number>().Int32Value();
int src = info[1].As<Napi::Number>().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<Napi::Number>().Int32Value();
int paramIdx = info[1].As<Napi::Number>().Int32Value();
float value = info[2].As<Napi::Number>().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<Napi::Number>().Int32Value();
int msgType = info[1].As<Napi::Number>().Int32Value();
int channel = info[2].As<Napi::Number>().Int32Value();
juce::MidiMessage midiMsg;
if (msgType == 0) // Program Change
{
int program = info[3].As<Napi::Number>().Int32Value();
midiMsg = juce::MidiMessage::programChange(channel, program);
}
else if (msgType == 1) // Control Change
{
int controller = info[3].As<Napi::Number>().Int32Value();
int value = info.Length() > 4 ? info[4].As<Napi::Number>().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<Napi::Number>().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<Napi::Number>().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<Napi::Object>();
int slotId = item.Get("slotId").As<Napi::Number>().Int32Value();
bool bypassed = item.Get("bypassed").As<Napi::Boolean>().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<Napi::Object>();
auto slotVal = item.Get("slotId");
auto bypVal = item.Get("bypassed");
if (!slotVal.IsNumber() || !bypVal.IsBoolean()) continue;
const double raw = slotVal.As<Napi::Number>().DoubleValue();
if (!std::isfinite(raw) || raw != std::floor(raw) || raw < 0.0 || raw > 4096.0) continue;
changes.add({ (int) raw, bypVal.As<Napi::Boolean>().Value() });
}
liveEngine->getSignalChain().setMultiBypass(changes);
src/audio/addon/AddonContext.cpp
+228
View File
@@ -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 <chrono>
#include <cstdio>
#include <set>
#include <thread>
namespace slopsmith::addon {
static std::shared_ptr<AudioEngine> engine;
static std::mutex engineMutex;
static std::shared_ptr<VSTHost> vstHost;
static std::mutex vstHostMutex;
static std::thread juceMessageThread;
static std::atomic<bool> juceRunning{false};
static std::atomic<bool> 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<void()> shutdownUiTeardown;
std::shared_ptr<AudioEngine> snapshotEngine()
{
std::lock_guard<std::mutex> lock(engineMutex);
return engine;
}
std::shared_ptr<VSTHost> snapshotVstHost()
{
std::lock_guard<std::mutex> 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<void()> 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::WaitableEvent>();
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<std::shared_ptr<juce::WaitableEvent>> pendingLoads;
bool isShuttingDown()
{
return alreadyShutDown.load(std::memory_order_acquire);
}
void registerPendingLoad(std::shared_ptr<juce::WaitableEvent> evt)
{
std::lock_guard<std::mutex> lock(pendingLoadsMutex);
pendingLoads.insert(std::move(evt));
}
void unregisterPendingLoad(const std::shared_ptr<juce::WaitableEvent>& evt)
{
std::lock_guard<std::mutex> lock(pendingLoadsMutex);
pendingLoads.erase(evt);
}
void cancelAllPendingLoads()
{
std::lock_guard<std::mutex> lock(pendingLoadsMutex);
for (auto& evt : pendingLoads) evt->signal();
pendingLoads.clear();
}
// ── Lifecycle ────────────────────────────────────────────────────────────────
void initialize(std::function<void()> 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<AudioEngine> liveEngine;
{
std::lock_guard<std::mutex> lock(engineMutex);
engine = std::make_shared<AudioEngine>();
liveEngine = engine;
}
{
std::lock_guard<std::mutex> lock(vstHostMutex);
vstHost = std::make_shared<VSTHost>();
}
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<std::mutex> lock(engineMutex);
engine.reset();
}
{
std::lock_guard<std::mutex> 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
src/audio/addon/AddonContext.h
+66
View File
@@ -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 <juce_events/juce_events.h>
#include <atomic>
#include <functional>
#include <memory>
#include <mutex>
namespace slopsmith::addon {
std::shared_ptr<AudioEngine> snapshotEngine();
std::shared_ptr<VSTHost> 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<void()> 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<void()> func);
template <typename Func>
inline void dispatchOnMessageThread(Func&& func)
{
dispatchOnMessageThreadImpl(std::function<void()>(std::forward<Func>(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<juce::WaitableEvent> evt);
void unregisterPendingLoad(const std::shared_ptr<juce::WaitableEvent>& evt);
void cancelAllPendingLoads();
} // namespace slopsmith::addon
src/audio/addon/NapiHelpers.h
+66
View File
@@ -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<Napi::Number>() path to copy.
#include <napi.h>
#include <cmath>
#include <optional>
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<int> 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<Napi::Number>().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<int> 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<float> argFiniteFloat(const Napi::CallbackInfo& info, size_t i)
{
if (i >= info.Length() || ! info[i].IsNumber()) return std::nullopt;
const double raw = info[i].As<Napi::Number>().DoubleValue();
if (! std::isfinite(raw)) return std::nullopt;
return (float) raw;
}
inline std::optional<bool> argBool(const Napi::CallbackInfo& info, size_t i)
{
if (i >= info.Length() || ! info[i].IsBoolean()) return std::nullopt;
return info[i].As<Napi::Boolean>().Value();
}
// MIDI channel: JUCE expects 1..16 and asserts otherwise.
inline std::optional<int> 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<int> argMidiByte(const Napi::CallbackInfo& info, size_t i)
{
return argInt(info, i, 0, 127);
}
} // namespace slopsmith::addon
tests/napi-arg-fuzz.test.js
+75
View File
@@ -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 });
}
});