mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-12 07:08:12 +00:00
fix(audio): run device lifecycle ops on the JUCE message thread (Windows)
Tester crash (dump 2026-07-15, Focusrite USB ASIO): the driver's deferred kAsioResetRequest fires a juce::Timer on the addon's JUCE message thread (ASIOAudioIODevice::timerCallback -> reloadChannelNames) while the Node thread concurrently destroys the device inside setAudioDevices/stopAudio — use-after-free, ~5 minutes after every launch. New runDeviceLifecycleOp() marshals every binding that can create or destroy a juce::AudioIODevice (setDevice, device-type switches, start/stop, stream output open/close, extra-input bind/unbind, add/removeSource) onto the message thread on Windows, serialising them with those timers. Inline on macOS (dispatch already inline) and Linux (ALSA main-thread contract unchanged), and inline when already on the message thread to avoid self-deadlock. Closures capture by value and return through shared_ptr so a timed-out dispatch that runs late can't touch the caller's dead stack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
4dc68a2a28
commit
354052bc7c
@@ -54,6 +54,44 @@ inline bool dispatchOnMessageThread(Func&& func)
|
|||||||
return dispatchOnMessageThreadImpl(std::function<void()>(std::forward<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.
|
||||||
|
template <typename Func>
|
||||||
|
inline bool runDeviceLifecycleOp(Func&& func)
|
||||||
|
{
|
||||||
|
#if JUCE_WINDOWS
|
||||||
|
if (auto* mm = juce::MessageManager::getInstanceWithoutCreating())
|
||||||
|
if (!mm->isThisTheMessageThread())
|
||||||
|
return dispatchOnMessageThread(std::forward<Func>(func));
|
||||||
|
#endif
|
||||||
|
func();
|
||||||
|
return true;
|
||||||
|
}
|
||||||
|
|
||||||
// Pending-async-load registry: LoadVSTWorker / LoadPresetWorker block on a
|
// Pending-async-load registry: LoadVSTWorker / LoadPresetWorker block on a
|
||||||
// WaitableEvent until the message-thread continuation fires; doShutdown
|
// WaitableEvent until the message-thread continuation fires; doShutdown
|
||||||
// signals every registered event so no worker waits forever once the pump
|
// signals every registered event so no worker waits forever once the pump
|
||||||
|
|||||||
@@ -13,6 +13,7 @@
|
|||||||
#include <cmath>
|
#include <cmath>
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
#include <limits>
|
#include <limits>
|
||||||
|
#include <memory>
|
||||||
#include <string>
|
#include <string>
|
||||||
|
|
||||||
namespace slopsmith::addon {
|
namespace slopsmith::addon {
|
||||||
@@ -506,7 +507,14 @@ Napi::Value AddSource(const Napi::CallbackInfo& info)
|
|||||||
const int k = info[1].As<Napi::Number>().Int32Value();
|
const int k = info[1].As<Napi::Number>().Int32Value();
|
||||||
if (k >= 0) deviceKey = k; // negatives ignored → primary
|
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.
|
// removeSource(sourceId) -> boolean. sources[0] cannot be removed.
|
||||||
@@ -516,7 +524,15 @@ Napi::Value RemoveSource(const Napi::CallbackInfo& info)
|
|||||||
auto liveEngine = snapshotEngine();
|
auto liveEngine = snapshotEngine();
|
||||||
if (!liveEngine || info.Length() < 1 || !info[0].IsNumber())
|
if (!liveEngine || info.Length() < 1 || !info[0].IsNumber())
|
||||||
return Napi::Boolean::New(env, false);
|
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.
|
// 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)");
|
return Napi::String::New(env, "bindInputDevice(deviceKey:number, deviceName:string)");
|
||||||
const int deviceKey = info[0].As<Napi::Number>().Int32Value();
|
const int deviceKey = info[0].As<Napi::Number>().Int32Value();
|
||||||
const std::string name = info[1].As<Napi::String>().Utf8Value();
|
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.
|
// unbindInputDevice(deviceKey) -> boolean. Stops + releases the extra device.
|
||||||
@@ -582,7 +605,14 @@ Napi::Value UnbindInputDevice(const Napi::CallbackInfo& info)
|
|||||||
auto liveEngine = snapshotEngine();
|
auto liveEngine = snapshotEngine();
|
||||||
if (!liveEngine || info.Length() < 1 || !info[0].IsNumber())
|
if (!liveEngine || info.Length() < 1 || !info[0].IsNumber())
|
||||||
return Napi::Boolean::New(env, false);
|
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);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
|||||||
@@ -13,6 +13,7 @@
|
|||||||
#include <cmath>
|
#include <cmath>
|
||||||
#include <cstdio>
|
#include <cstdio>
|
||||||
#include <limits>
|
#include <limits>
|
||||||
|
#include <memory>
|
||||||
#include <string>
|
#include <string>
|
||||||
|
|
||||||
namespace slopsmith::addon {
|
namespace slopsmith::addon {
|
||||||
@@ -215,8 +216,12 @@ Napi::Value SetDeviceType(const Napi::CallbackInfo& info)
|
|||||||
return Napi::Boolean::New(env, false);
|
return Napi::Boolean::New(env, false);
|
||||||
|
|
||||||
auto typeName = info[0].As<Napi::String>().Utf8Value();
|
auto typeName = info[0].As<Napi::String>().Utf8Value();
|
||||||
bool result = liveEngine->setDeviceType(juce::String(typeName));
|
auto result = std::make_shared<bool>(false);
|
||||||
return Napi::Boolean::New(env, result);
|
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)
|
Napi::Value SetOutputDeviceType(const Napi::CallbackInfo& info)
|
||||||
@@ -226,7 +231,12 @@ Napi::Value SetOutputDeviceType(const Napi::CallbackInfo& info)
|
|||||||
if (!liveEngine || info.Length() < 1 || !info[0].IsString())
|
if (!liveEngine || info.Length() < 1 || !info[0].IsString())
|
||||||
return Napi::Boolean::New(env, false);
|
return Napi::Boolean::New(env, false);
|
||||||
auto typeName = info[0].As<Napi::String>().Utf8Value();
|
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)
|
Napi::Value SetDevice(const Napi::CallbackInfo& info)
|
||||||
@@ -299,7 +309,15 @@ Napi::Value SetDevice(const Napi::CallbackInfo& info)
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Main thread only — JUCE's ALSA backend deadlocks if called from a worker.
|
// 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("ok", r.ok);
|
||||||
result.Set("duplex", r.duplex);
|
result.Set("duplex", r.duplex);
|
||||||
result.Set("sampleRate", r.sampleRate);
|
result.Set("sampleRate", r.sampleRate);
|
||||||
@@ -313,13 +331,15 @@ Napi::Value SetDevice(const Napi::CallbackInfo& info)
|
|||||||
|
|
||||||
Napi::Value StartAudio(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();
|
return info.Env().Undefined();
|
||||||
}
|
}
|
||||||
|
|
||||||
Napi::Value StopAudio(const Napi::CallbackInfo& info)
|
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();
|
return info.Env().Undefined();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -340,15 +360,21 @@ Napi::Value SetStreamOutputDevice(const Napi::CallbackInfo& info)
|
|||||||
return Napi::String::New(env, "setStreamOutputDevice(typeName:string, deviceName:string)");
|
return Napi::String::New(env, "setStreamOutputDevice(typeName:string, deviceName:string)");
|
||||||
const std::string typeName = info[0].As<Napi::String>().Utf8Value();
|
const std::string typeName = info[0].As<Napi::String>().Utf8Value();
|
||||||
const std::string devName = info[1].As<Napi::String>().Utf8Value();
|
const std::string devName = info[1].As<Napi::String>().Utf8Value();
|
||||||
return Napi::String::New(env,
|
auto err = std::make_shared<juce::String>();
|
||||||
liveEngine->setStreamOutputDevice(juce::String(typeName), juce::String(devName)).toStdString());
|
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
|
// clearStreamOutput() -> undefined
|
||||||
Napi::Value ClearStreamOutput(const Napi::CallbackInfo& info)
|
Napi::Value ClearStreamOutput(const Napi::CallbackInfo& info)
|
||||||
{
|
{
|
||||||
auto liveEngine = snapshotEngine();
|
auto liveEngine = snapshotEngine();
|
||||||
if (liveEngine) liveEngine->clearStreamOutput();
|
if (liveEngine)
|
||||||
|
runDeviceLifecycleOp([liveEngine] { liveEngine->clearStreamOutput(); });
|
||||||
return info.Env().Undefined();
|
return info.Env().Undefined();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user