Merge pull request #113 from got-feedBack/fix/asio-device-lifecycle-message-thread

fix(audio): run device lifecycle ops on the JUCE message thread (Windows ASIO UAF)

src/audio/addon/AddonContext.h

@@ -54,6 +54,58 @@ inline bool dispatchOnMessageThread(Func&& func)
     return dispatchOnMessageThreadImpl(std::function<void()>(std::forward<Func>(func)));
 }
 
+// Run a device-lifecycle mutation (anything that can create or destroy a
+// juce::AudioIODevice: setAudioDevices, start/stopAudio, device-type
+// switches, stream-output open/close, extra-input add/remove) on the JUCE
+// message thread.
+//
+// Why: on Windows, ASIO devices arm juce::Timers (the driver's deferred
+// kAsioResetRequest, device-change detection) that fire on the message
+// thread. Destroying the device from the Node thread while such a timer is
+// queued or mid-callback is a use-after-free — tester crash 2026-07-15:
+// ASIOAudioIODevice::timerCallback → reloadChannelNames on a freed device,
+// ~5 min after start, every run (Focusrite USB ASIO reset request).
+// Hopping the mutation onto the message thread serialises it with those
+// timer callbacks, so a timer can never observe a half-destroyed device.
+//
+// Windows-only hop, by design: macOS's dispatchOnMessageThread already runs
+// inline (no separate pump), and Linux/ALSA keeps its long-standing
+// "called from the Node main thread" contract untouched. Inline when the
+// caller already IS the message thread (engine init runs there), because
+// dispatch-and-wait from the message thread would deadlock.
+//
+// Returns false when the dispatched work did not verifiably complete (post
+// refused or 15 s timeout) — same contract as dispatchOnMessageThread.
+// CAPTURE RULE: on timeout the queued closure may still run later, so the
+// closure must own everything it touches — capture by value (engine
+// snapshot, args) and write results through a shared_ptr, never through
+// references to the caller's stack.
+//
+// KNOWN LIMITATION (timeout desync): when the 15 s wait expires the binding
+// reports failure to JS, but the closure may still complete afterwards —
+// e.g. addSource returns -1 yet the source gets created, holding its input
+// device until the next reconfigure. Memory-safe by the capture rule above,
+// just not logically reconciled; only reachable with the message thread
+// jammed > 15 s. Callers that care can re-query engine state (listSources,
+// getCurrentDevice) after a reported failure.
+template <typename Func>
+inline bool runDeviceLifecycleOp(Func&& func)
+{
+#if JUCE_WINDOWS
+    // No MessageManager means the pump is gone (pre-init or mid-shutdown):
+    // report failure instead of running the mutation unserialised on the
+    // caller's thread — that would reintroduce the race this helper exists
+    // to prevent (CodeRabbit #113 review).
+    auto* mm = juce::MessageManager::getInstanceWithoutCreating();
+    if (mm == nullptr)
+        return false;
+    if (!mm->isThisTheMessageThread())
+        return dispatchOnMessageThread(std::forward<Func>(func));
+#endif
+    func();
+    return true;
+}
+
 // 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

src/audio/addon/DetectionBindings.cpp

@@ -13,6 +13,7 @@
 #include <cmath>
 #include <cstdio>
 #include <limits>
+#include <memory>
 #include <string>
 
 namespace slopsmith::addon {
@@ -506,7 +507,14 @@ Napi::Value AddSource(const Napi::CallbackInfo& info)
         const int k = info[1].As<Napi::Number>().Int32Value();
         if (k >= 0) deviceKey = k;  // negatives ignored → primary
     }
-    return Napi::Number::New(env, liveEngine->addSource(channel, deviceKey));
+    // deviceKey != 0 opens an extra input AudioIODevice — device lifecycle,
+    // so it must run on the JUCE message thread (see runDeviceLifecycleOp).
+    auto sourceId = std::make_shared<int>(-1);
+    if (!runDeviceLifecycleOp([liveEngine, channel, deviceKey, sourceId] {
+            *sourceId = liveEngine->addSource(channel, deviceKey);
+        }))
+        return Napi::Number::New(env, -1);
+    return Napi::Number::New(env, *sourceId);
 }
 
 // removeSource(sourceId) -> boolean. sources[0] cannot be removed.
@@ -516,7 +524,15 @@ Napi::Value RemoveSource(const Napi::CallbackInfo& info)
     auto liveEngine = snapshotEngine();
     if (!liveEngine || info.Length() < 1 || !info[0].IsNumber())
         return Napi::Boolean::New(env, false);
-    return Napi::Boolean::New(env, liveEngine->removeSource(info[0].As<Napi::Number>().Int32Value()));
+    // May close the source's extra input AudioIODevice — message thread hop
+    // for the same reason as AddSource.
+    const int sourceId = info[0].As<Napi::Number>().Int32Value();
+    auto removed = std::make_shared<bool>(false);
+    if (!runDeviceLifecycleOp([liveEngine, sourceId, removed] {
+            *removed = liveEngine->removeSource(sourceId);
+        }))
+        return Napi::Boolean::New(env, false);
+    return Napi::Boolean::New(env, *removed);
 }
 
 // listSources() -> [{ id, inputChannel, active }]. Null on a missing engine.
@@ -572,7 +588,14 @@ Napi::Value BindInputDevice(const Napi::CallbackInfo& info)
         return Napi::String::New(env, "bindInputDevice(deviceKey:number, deviceName:string)");
     const int deviceKey = info[0].As<Napi::Number>().Int32Value();
     const std::string name = info[1].As<Napi::String>().Utf8Value();
-    return Napi::String::New(env, liveEngine->bindInputDevice(deviceKey, name).toStdString());
+    // Opens a physical AudioIODevice — message thread hop (runDeviceLifecycleOp).
+    auto err = std::make_shared<juce::String>();
+    if (!runDeviceLifecycleOp([liveEngine, deviceKey, name, err] {
+            *err = liveEngine->bindInputDevice(deviceKey, name);
+        }))
+        return Napi::String::New(env,
+            "bindInputDevice did not complete (message thread unavailable or timed out)");
+    return Napi::String::New(env, err->toStdString());
 }
 
 // unbindInputDevice(deviceKey) -> boolean. Stops + releases the extra device.
@@ -582,7 +605,14 @@ Napi::Value UnbindInputDevice(const Napi::CallbackInfo& info)
     auto liveEngine = snapshotEngine();
     if (!liveEngine || info.Length() < 1 || !info[0].IsNumber())
         return Napi::Boolean::New(env, false);
-    return Napi::Boolean::New(env, liveEngine->unbindInputDevice(info[0].As<Napi::Number>().Int32Value()));
+    // Closes a physical AudioIODevice — message thread hop (runDeviceLifecycleOp).
+    const int deviceKey = info[0].As<Napi::Number>().Int32Value();
+    auto unbound = std::make_shared<bool>(false);
+    if (!runDeviceLifecycleOp([liveEngine, deviceKey, unbound] {
+            *unbound = liveEngine->unbindInputDevice(deviceKey);
+        }))
+        return Napi::Boolean::New(env, false);
+    return Napi::Boolean::New(env, *unbound);
 }
 
 

src/audio/addon/DeviceBindings.cpp

@@ -13,6 +13,7 @@
 #include <cmath>
 #include <cstdio>
 #include <limits>
+#include <memory>
 #include <string>
 
 namespace slopsmith::addon {
@@ -125,9 +126,24 @@ Napi::Value ProbeDeviceOptions(const Napi::CallbackInfo& info)
         return obj;
     }
 
-    auto options = liveEngine->probeDeviceOptionsDual(
-        juce::String(inputType), juce::String(inputName),
-        juce::String(outputType), juce::String(outputName));
+    // Probing constructs + destroys short-lived ASIO device objects
+    // (DeviceSetup::probeDual / rateSupportedBy createDevice) — same
+    // create/destroy-off-the-message-thread pattern as the lifecycle ops, so
+    // it takes the same hop (PR #113 review finding 1). Query-only (never
+    // open()), but a probe ASIOAudioIODevice owns the same reset-timer
+    // machinery as the live one.
+    auto optionsPtr = std::make_shared<AudioEngine::DeviceOptions>();
+    if (!runDeviceLifecycleOp([liveEngine, inputType, inputName, outputType, outputName, optionsPtr] {
+            *optionsPtr = liveEngine->probeDeviceOptionsDual(
+                juce::String(inputType), juce::String(inputName),
+                juce::String(outputType), juce::String(outputName));
+        }))
+    {
+        obj.Set("error", "device probe did not complete (message thread unavailable or timed out)");
+        obj.Set("compatible", false);
+        return obj;
+    }
+    const auto& options = *optionsPtr;
     obj.Set("type", options.inputType.toStdString());      // legacy alias
     obj.Set("inputType", options.inputType.toStdString());
     obj.Set("outputType", options.outputType.toStdString());
@@ -215,8 +231,12 @@ Napi::Value SetDeviceType(const Napi::CallbackInfo& info)
         return Napi::Boolean::New(env, false);
 
     auto typeName = info[0].As<Napi::String>().Utf8Value();
-    bool result = liveEngine->setDeviceType(juce::String(typeName));
-    return Napi::Boolean::New(env, result);
+    auto result = std::make_shared<bool>(false);
+    if (!runDeviceLifecycleOp([liveEngine, typeName, result] {
+            *result = liveEngine->setDeviceType(juce::String(typeName));
+        }))
+        return Napi::Boolean::New(env, false);
+    return Napi::Boolean::New(env, *result);
 }
 
 Napi::Value SetOutputDeviceType(const Napi::CallbackInfo& info)
@@ -226,7 +246,12 @@ Napi::Value SetOutputDeviceType(const Napi::CallbackInfo& info)
     if (!liveEngine || info.Length() < 1 || !info[0].IsString())
         return Napi::Boolean::New(env, false);
     auto typeName = info[0].As<Napi::String>().Utf8Value();
-    return Napi::Boolean::New(env, liveEngine->setOutputDeviceType(juce::String(typeName)));
+    auto result = std::make_shared<bool>(false);
+    if (!runDeviceLifecycleOp([liveEngine, typeName, result] {
+            *result = liveEngine->setOutputDeviceType(juce::String(typeName));
+        }))
+        return Napi::Boolean::New(env, false);
+    return Napi::Boolean::New(env, *result);
 }
 
 Napi::Value SetDevice(const Napi::CallbackInfo& info)
@@ -299,7 +324,15 @@ Napi::Value SetDevice(const Napi::CallbackInfo& info)
     }
 
     // Main thread only — JUCE's ALSA backend deadlocks if called from a worker.
-    const auto r = liveEngine->setAudioDevices(cfg);
+    // On Windows this hops to the JUCE message thread (runDeviceLifecycleOp)
+    // so device destruction can't race the ASIO reset/device-change timers.
+    auto res = std::make_shared<AudioEngine::DeviceConfigResult>();
+    if (!runDeviceLifecycleOp([liveEngine, cfg, res] { *res = liveEngine->setAudioDevices(cfg); }))
+    {
+        result.Set("error", "device reconfigure did not complete (message thread unavailable or timed out)");
+        return result;
+    }
+    const auto& r = *res;
     result.Set("ok", r.ok);
     result.Set("duplex", r.duplex);
     result.Set("sampleRate", r.sampleRate);
@@ -313,13 +346,15 @@ Napi::Value SetDevice(const Napi::CallbackInfo& info)
 
 Napi::Value StartAudio(const Napi::CallbackInfo& info)
 {
-    if (auto liveEngine = snapshotEngine()) liveEngine->startAudio();
+    if (auto liveEngine = snapshotEngine())
+        runDeviceLifecycleOp([liveEngine] { liveEngine->startAudio(); });
     return info.Env().Undefined();
 }
 
 Napi::Value StopAudio(const Napi::CallbackInfo& info)
 {
-    if (auto liveEngine = snapshotEngine()) liveEngine->stopAudio();
+    if (auto liveEngine = snapshotEngine())
+        runDeviceLifecycleOp([liveEngine] { liveEngine->stopAudio(); });
     return info.Env().Undefined();
 }
 
@@ -340,15 +375,21 @@ Napi::Value SetStreamOutputDevice(const Napi::CallbackInfo& info)
         return Napi::String::New(env, "setStreamOutputDevice(typeName:string, deviceName:string)");
     const std::string typeName = info[0].As<Napi::String>().Utf8Value();
     const std::string devName  = info[1].As<Napi::String>().Utf8Value();
-    return Napi::String::New(env,
-        liveEngine->setStreamOutputDevice(juce::String(typeName), juce::String(devName)).toStdString());
+    auto err = std::make_shared<juce::String>();
+    if (!runDeviceLifecycleOp([liveEngine, typeName, devName, err] {
+            *err = liveEngine->setStreamOutputDevice(juce::String(typeName), juce::String(devName));
+        }))
+        return Napi::String::New(env,
+            "stream output open did not complete (message thread unavailable or timed out)");
+    return Napi::String::New(env, err->toStdString());
 }
 
 // clearStreamOutput() -> undefined
 Napi::Value ClearStreamOutput(const Napi::CallbackInfo& info)
 {
     auto liveEngine = snapshotEngine();
-    if (liveEngine) liveEngine->clearStreamOutput();
+    if (liveEngine)
+        runDeviceLifecycleOp([liveEngine] { liveEngine->clearStreamOutput(); });
     return info.Env().Undefined();
 }