From 95b32ba1607543fecacaf98d0af6db7f17761194 Mon Sep 17 00:00:00 2001 From: OmikronApex Date: Tue, 14 Jul 2026 02:23:09 +0200 Subject: [PATCH] refactor(audio): extract EditorWindows (phase 7b) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Moves the in-process plugin editor cluster — PluginEditorWindow, the slotId→window map, the message-thread teardown pair (closeAll / destroyAll, the #56 use-after-free guards), OpenPluginEditor with the full Windows sandbox-promotion flow, and ClosePluginEditor — verbatim into src/audio/addon/EditorWindows.{h,cpp}. NodeAddon keeps using-declarations; the export table and ClearChain/LoadPreset teardown calls are unchanged. The two bindings pick up NapiHelpers slot-id validation while moving (the same deep-read §2 fix the other bindings got in phase 6 — a NaN slot id used to coerce to slot 0 and open/close the wrong editor). Co-Authored-By: Claude Fable 5 --- src/audio/CMakeLists.txt | 1 + src/audio/NodeAddon.cpp | 371 +---------------------------- src/audio/addon/EditorWindows.cpp | 377 ++++++++++++++++++++++++++++++ src/audio/addon/EditorWindows.h | 28 +++ 4 files changed, 414 insertions(+), 363 deletions(-) create mode 100644 src/audio/addon/EditorWindows.cpp create mode 100644 src/audio/addon/EditorWindows.h diff --git a/src/audio/CMakeLists.txt b/src/audio/CMakeLists.txt index e2306bd..1284418 100644 --- a/src/audio/CMakeLists.txt +++ b/src/audio/CMakeLists.txt @@ -13,6 +13,7 @@ set(AUDIO_SOURCES engine/ExtraInputs.cpp addon/AddonContext.cpp addon/ChainOps.cpp + addon/EditorWindows.cpp SourceChain.cpp SignalChain.cpp VSTHost.cpp diff --git a/src/audio/NodeAddon.cpp b/src/audio/NodeAddon.cpp index 46fdb48..cce63b5 100644 --- a/src/audio/NodeAddon.cpp +++ b/src/audio/NodeAddon.cpp @@ -29,6 +29,12 @@ #include "addon/AddonContext.h" #include "addon/NapiHelpers.h" #include "addon/ChainOps.h" +#include "addon/EditorWindows.h" + +using slopsmith::addon::closeAllPluginEditorWindows; +using slopsmith::addon::destroyAllPluginEditorWindowsOnMessageThread; +using slopsmith::addon::OpenPluginEditor; +using slopsmith::addon::ClosePluginEditor; // Lifetime/threading moved to addon/AddonContext (TLC phase 6); the usings // keep the 100+ existing binding bodies unchanged. @@ -95,11 +101,7 @@ static int loadSafeBlockSize(const AudioEngine& eng) } // 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 -// the message thread — can tear editors down before engine.reset() frees the -// processors those editors point at (use-after-free; feedBack-desktop#56). -static void destroyAllPluginEditorWindowsOnMessageThread(); +// thread — lives in addon/EditorWindows now (TLC phase 7). // ── Lifecycle ───────────────────────────────────────────────────────────────── @@ -2465,8 +2467,7 @@ static Napi::Value SetBypass(const Napi::CallbackInfo& info) // owns an AudioProcessorEditor bound to its slot's processor, so if the // processor is freed first the editor's next timer/paint callback dereferences // freed memory (use-after-free → DEP-execute crash seconds after pause; -// feedBack-desktop#56). Defined below, after the editorWindows map. -static void closeAllPluginEditorWindows(); +// feedBack-desktop#56). Lives in addon/EditorWindows now. static Napi::Value ClearChain(const Napi::CallbackInfo& info) { @@ -2575,362 +2576,6 @@ static Napi::Value GetChainState(const Napi::CallbackInfo& info) // ── Plugin Editor Window ────────────────────────────────────────────────────── -class PluginEditorWindow; -static std::map> editorWindows; - -class PluginEditorWindow : public juce::DocumentWindow -{ -public: - PluginEditorWindow(juce::AudioProcessorEditor* ed, const juce::String& title) - : DocumentWindow(title, juce::Colours::darkgrey, DocumentWindow::closeButton) - { - setContentOwned(ed, true); - setResizable(true, false); - setUsingNativeTitleBar(true); - centreWithSize(ed->getWidth(), ed->getHeight()); - setVisible(true); - toFront(true); - } - - void closeButtonPressed() override - { - // Remove from map so editor can be reopened - for (auto it = editorWindows.begin(); it != editorWindows.end(); ++it) - { - if (it->second.get() == this) - { - auto slotId = it->first; - juce::MessageManager::callAsync([slotId]() { - editorWindows.erase(slotId); - }); - break; - } - } - setVisible(false); - } -}; - -// Inline teardown: destroys every editor window. Caller MUST already be on the -// message thread (editorWindows holds JUCE GUI objects). Forward-declared near -// Init for doShutdown's use. -static void destroyAllPluginEditorWindowsOnMessageThread() -{ - // Fails fast in assertion-enabled builds if a caller violates the - // precondition. Compiled out here under -DJUCE_DISABLE_ASSERTIONS, so it is - // documentation + a debug-build tripwire, never runtime cost. - JUCE_ASSERT_MESSAGE_THREAD - editorWindows.clear(); -} - -// See the forward declaration above ClearChain for why this exists. Tears down -// the in-process editor windows so they are destroyed before the caller frees -// the processors those editors point at. Clearing an empty map is cheap, so -// calling this on every teardown is fine even when no editor is open. -// -// IMPORTANT: every caller runs on a MAIN-thread / message-thread context — -// ClearChain and LoadPreset are N-API calls on the Node thread, doShutdown uses -// the inline variant directly. This is NOT called from a libuv worker (that is -// why LoadPreset closes editors before queuing LoadPresetWorker, rather than -// letting the worker do it). Given that: -// - Already on the message thread (doShutdown; ClearChain / LoadPreset on -// macOS, where Node's main thread IS the JUCE message thread) → tear down -// inline; posting-and-waiting on ourselves would deadlock. -// - Otherwise (ClearChain / LoadPreset on Linux/Windows, where the JUCE -// message thread is a dedicated std::thread) → post to that thread and block -// until the editors are gone. Its 50ms dispatch loop drains this promptly, -// so there is no macOS-style stall here. Report a refused post / wait -// timeout so a lingering-editor UAF stays diagnosable. -static void closeAllPluginEditorWindows() -{ - auto* mm = juce::MessageManager::getInstanceWithoutCreating(); - if (mm != nullptr && mm->isThisTheMessageThread()) - { - destroyAllPluginEditorWindowsOnMessageThread(); - return; - } - - auto done = std::make_shared(); - const bool posted = juce::MessageManager::callAsync([done]() - { - destroyAllPluginEditorWindowsOnMessageThread(); - done->signal(); - }); - if (!posted) - { - fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: message queue refused the post; " - "editors may briefly outlive their processors\n"); - return; - } - if (!done->wait(15000)) - fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: editor teardown did not complete " - "within 15s; proceeding\n"); -} - -static Napi::Value OpenPluginEditor(const Napi::CallbackInfo& info) -{ - auto env = info.Env(); - auto liveEngine = snapshotEngine(); - if (!liveEngine || info.Length() < 1) - return Napi::Boolean::New(env, false); - - int slotId = info[0].As().Int32Value(); - - auto slot = liveEngine->getSignalChain().getSlot(slotId); - if (!slot || !slot->processor || !slot->processor->hasEditor()) - return Napi::Boolean::New(env, false); - - // Sandboxed plugins: the editor is a top-level window owned by the - // sandbox child process. No host-side PluginEditorWindow and no - // cross-process SetParent reparent — that path produced a blank - // rendered surface for D3D / OpenGL plugins (Neural DSP Archetypes, - // etc.) because their render context lives in the child. The child's - // kOpenEditor handler brings the existing window to front on a repeat - // click, so re-entry is cheap and we don't track host-side state. - // - // Dispatch off the N-API call thread: requestOpenEditor() uses a - // blocking control->request (kDefaultReplyTimeoutMs = 10s), which on - // a slow or hung sandbox would otherwise stall V8's JS thread for - // the full timeout. Capture slotId rather than a raw processor - // pointer and re-resolve inside the message-thread lambda — that - // closes a UAF window where the slot could be removed (or the engine - // torn down) between this call returning and the async firing. - // Return optimistically; matches the in-process path below. - // - // SandboxedProcessor is compiled on all desktop platforms now (the POSIX - // sandbox runtime is active — see src/audio/CMakeLists.txt), so the - // editor-open IPC routes to the sandbox child on macOS/Linux too. The - // child owns a floating editor window (Reaper-style); the host only tracks - // the open/closed bit. -#if defined(SLOPSMITH_AUDIO_ADDON) - if (auto* sb = dynamic_cast(slot->processor.get())) - { - // Synchronous gate: if the sandbox child is already gone (crashed - // or shut down) there's no point scheduling the IPC. Return false - // so the renderer can surface "editor unavailable" rather than - // toggling its UI into a fake-open state that no event will ever - // contradict. hasEditor() above already gated on isAlive() but a - // crash between then and now is possible — re-check here. - if (!sb->isAlive()) - return Napi::Boolean::New(env, false); - const bool queued = juce::MessageManager::callAsync([slotId]() - { - auto liveEngine = snapshotEngine(); - if (!liveEngine) return; - if (auto* slot = liveEngine->getSignalChain().getSlot(slotId)) - if (auto* sb = dynamic_cast(slot->processor.get())) - sb->requestOpenEditor(); - }); - if (!queued) - { - // Message queue refused the post — typically only during - // shutdown. Surface the failure so the renderer doesn't - // toggle its UI into a fake-open state. - return Napi::Boolean::New(env, false); - } - return Napi::Boolean::New(env, true); - } -#endif - - // In-process plugin — host-side PluginEditorWindow flow. If a window - // already exists for this slot, bring it to front rather than creating - // a duplicate. - auto it = editorWindows.find(slotId); - if (it != editorWindows.end() && it->second) - { - if (it->second->isVisible()) - { - it->second->toFront(true); - return Napi::Boolean::New(env, true); - } - // Window was hidden/closed, remove stale entry - editorWindows.erase(it); - } - - // Create editor on the message thread. Capture slotId only — re-resolve - // the slot via snapshotEngine() + getSlot(slotId) inside the lambda so a - // SignalChain::removeProcessor() between this call returning and the - // async firing can't leave us calling createEditorAndMakeActive() on a - // dangling juce::AudioProcessor*. Mirrors the sandbox branch's pattern. - const bool queued = juce::MessageManager::callAsync([slotId]() - { - auto liveEngine = snapshotEngine(); - if (!liveEngine) return; - auto& chain = liveEngine->getSignalChain(); - auto* slot = chain.getSlot(slotId); - if (!slot || !slot->processor) return; - - // ── Windows editor-crash class fix ─────────────────────────────────── - // An in-process VST3 editor is created on JUCE's BACKGROUND message - // thread (V8 owns the OS main thread inside a Node addon). On Windows a - // Qt-using / window-on-init plugin then faults via USER32->WndProc on - // WM_ACTIVATEAPP with NO host frame on the stack, so the SignalChain SEH - // guard can't catch it and the whole app dies (0xC0000005 / 0xC0000409). - // Fix: never open a VST3 editor in-process on Windows — promote the slot - // to the out-of-process sandbox (which hosts the editor on a real - // top-level message thread, the environment the plugin needs) and open - // it there. Compiled on every platform so the swap path keeps building; - // gated to Windows at runtime since the in-process editor is fine on - // macOS/Linux (no WndProc) and the sandbox hop is pure overhead there. - static constexpr bool kPromoteEditorToSandbox = - #if JUCE_WINDOWS - true; - #else - false; - #endif - if (kPromoteEditorToSandbox) - { - // Decide + snapshot state SAFELY. captureVstStateForPromotion runs - // hasEditor()/getStateInformation() under the audio lock and the SEH - // guard (see its contract), so they neither race process()'s - // processBlock nor fault the app — an UNguarded getStateInformation on - // the very plugins this promotion targets would reintroduce the editor - // crash on the message thread. It returns true only for a non-sandboxed - // in-process VST3 that actually has an editor. - juce::MemoryBlock state; - if (chain.captureVstStateForPromotion(slotId, state)) - { - const juce::String path = slot->path; // immutable; message-thread only - fprintf(stderr, "[AudioEngine] editor-open: promoting in-process VST3 to sandbox: slot %d '%s'\n", - slotId, path.toRawUTF8()); - - juce::PluginDescription desc; - desc.fileOrIdentifier = path; - desc.name = juce::File(path).getFileNameWithoutExtension(); - - // tryLoadSandboxed only accepts a plugin that shouldSandbox() - // approves, so pin this path to the runtime sandbox list first. - // Remember whether it was ALREADY pinned: if the promotion fails - // we undo only OUR pin below, so a healthy, never-crashed plugin - // isn't left permanently forced to a sandbox that just proved - // unavailable (while a pre-existing/real blocklist entry stays). - const bool wasAlreadyPinned = slopsmith::sandbox::isCrashedPlugin(path); - slopsmith::sandbox::addCrashedPlugin(path); - - bool promoted = false; - juce::String err; - auto sandboxed = slopsmith::sandbox::tryLoadSandboxed( - desc, chain.getCurrentSampleRate(), chain.getCurrentBlockSize(), err); - if (sandboxed) - { - if (state.getSize() > 0) - sandboxed->setStateInformation(state.getData(), (int) state.getSize()); - if (chain.replaceProcessor(slotId, std::move(sandboxed))) - { - promoted = true; - bool editorOpened = false; - if (auto* slot2 = chain.getSlot(slotId)) - if (auto* sb = dynamic_cast(slot2->processor.get())) - editorOpened = sb->requestOpenEditor(); - fprintf(stderr, "[AudioEngine] editor-open: sandbox promotion OK for slot %d (editor %s)\n", - slotId, editorOpened ? "opened" : "FAILED to open"); - } - else - { - fprintf(stderr, "[AudioEngine] editor-open: replaceProcessor failed for slot %d\n", slotId); - } - } - else - { - fprintf(stderr, "[AudioEngine] editor-open: sandbox promotion failed for '%s': %s\n", - path.toRawUTF8(), err.toRawUTF8()); - } - - // Undo our transient pin on failure so a plugin that never crashed - // isn't stranded on the (evidently unavailable) sandbox route. - if (! promoted && ! wasAlreadyPinned) - slopsmith::sandbox::removeCrashedPlugin(path); - - // Promoted or not, never fall through to the in-process editor on - // Windows — that is the WndProc/Qt crash path this branch exists - // to avoid. - return; - } - // Not promotable (non-VST / editor-less / already-sandboxed, or the - // guarded capture faulted and released the processor). Fall through to - // the in-process branch below, which is safe for all of those cases - // (an already-sandboxed slot opens its editor out-of-process; an - // editor-less or released processor simply opens no window). - } - - // In-process editor: non-VST3, editor-less, already-sandboxed, or POSIX - // (where the in-process editor is safe). - auto* processor = slot->processor.get(); - auto name = slot->name; - juce::AudioProcessorEditor* editor = nullptr; - try { - editor = processor->createEditorAndMakeActive(); - } catch (const std::exception& e) { - fprintf(stderr, "[AudioEngine] createEditorAndMakeActive crashed for '%s': %s\n", name.toRawUTF8(), e.what()); - } catch (...) { - fprintf(stderr, "[AudioEngine] createEditorAndMakeActive crashed for '%s': unknown error\n", name.toRawUTF8()); - } - if (editor) - { - editorWindows[slotId] = std::make_unique(editor, name); - fprintf(stderr, "[AudioEngine] Opened editor for slot %d: %s (%dx%d)\n", - slotId, name.toRawUTF8(), editor->getWidth(), editor->getHeight()); - } - }); - - return Napi::Boolean::New(env, queued); -} - -static Napi::Value ClosePluginEditor(const Napi::CallbackInfo& info) -{ - auto env = info.Env(); - if (info.Length() < 1) return Napi::Boolean::New(env, false); - - int slotId = info[0].As().Int32Value(); - - // Sandboxed plugins: route the close to the sandbox child via IPC. - // No host-side PluginEditorWindow exists for these. - // - // Same shape as the open path: dispatch off the N-API thread and - // re-resolve the slot inside the lambda. requestCloseEditor() - // ultimately writes to the control pipe (writeFrame can block up - // to ~5s on a stalled reader), so running it synchronously here - // would freeze JS / the renderer UI on a slow sandbox; the - // re-resolve guards against slot-removal UAF between the napi call - // and the async firing. - // - // All desktop platforms: route the close to the sandbox child via IPC - // (SandboxedProcessor is compiled everywhere now). In-process plugins fall - // through to the host-side editor-window teardown below. -#if defined(SLOPSMITH_AUDIO_ADDON) - if (auto liveEngine = snapshotEngine()) - { - if (auto* slot = liveEngine->getSignalChain().getSlot(slotId)) - { - if (slot->processor - && dynamic_cast(slot->processor.get())) - { - const bool queued = juce::MessageManager::callAsync([slotId]() - { - auto liveEngine = snapshotEngine(); - if (!liveEngine) return; - if (auto* slot = liveEngine->getSignalChain().getSlot(slotId)) - if (auto* sb = dynamic_cast(slot->processor.get())) - sb->requestCloseEditor(); - }); - return Napi::Boolean::New(env, queued); - } - } - } -#endif - - // In-process plugin — tear down the host-side editor window. - auto it = editorWindows.find(slotId); - if (it != editorWindows.end()) - { - juce::MessageManager::callAsync([slotId]() - { - editorWindows.erase(slotId); - }); - return Napi::Boolean::New(env, true); - } - return Napi::Boolean::New(env, false); -} - // ── Parameters ──────────────────────────────────────────────────────────────── static Napi::Value GetParameters(const Napi::CallbackInfo& info) diff --git a/src/audio/addon/EditorWindows.cpp b/src/audio/addon/EditorWindows.cpp new file mode 100644 index 0000000..d74ac59 --- /dev/null +++ b/src/audio/addon/EditorWindows.cpp @@ -0,0 +1,377 @@ +// EditorWindows implementation — moved verbatim from NodeAddon.cpp (TLC plan +// phase 7 / §3.4). Only edits: statics live in this namespace now, and the +// two bindings validate their slot-id argument through NapiHelpers (the same +// deep-read 2 fix the other bindings got in phase 6 — a NaN slot id used to +// coerce to slot 0 and open/close the wrong editor). + +#include "EditorWindows.h" + +#include "AddonContext.h" +#include "NapiHelpers.h" +#include "../Sandbox/SandboxedProcessor.h" +#include "../Sandbox/CrashAttribution.h" + +#include +#include +#include + +namespace slopsmith::addon { + +class PluginEditorWindow; +static std::map> editorWindows; + +class PluginEditorWindow : public juce::DocumentWindow +{ +public: + PluginEditorWindow(juce::AudioProcessorEditor* ed, const juce::String& title) + : DocumentWindow(title, juce::Colours::darkgrey, DocumentWindow::closeButton) + { + setContentOwned(ed, true); + setResizable(true, false); + setUsingNativeTitleBar(true); + centreWithSize(ed->getWidth(), ed->getHeight()); + setVisible(true); + toFront(true); + } + + void closeButtonPressed() override + { + // Remove from map so editor can be reopened + for (auto it = editorWindows.begin(); it != editorWindows.end(); ++it) + { + if (it->second.get() == this) + { + auto slotId = it->first; + juce::MessageManager::callAsync([slotId]() { + editorWindows.erase(slotId); + }); + break; + } + } + setVisible(false); + } +}; + +// Inline teardown: destroys every editor window. Caller MUST already be on the +// message thread (editorWindows holds JUCE GUI objects). Forward-declared near +// Init for doShutdown's use. +void destroyAllPluginEditorWindowsOnMessageThread() +{ + // Fails fast in assertion-enabled builds if a caller violates the + // precondition. Compiled out here under -DJUCE_DISABLE_ASSERTIONS, so it is + // documentation + a debug-build tripwire, never runtime cost. + JUCE_ASSERT_MESSAGE_THREAD + editorWindows.clear(); +} + +// See the forward declaration above ClearChain for why this exists. Tears down +// the in-process editor windows so they are destroyed before the caller frees +// the processors those editors point at. Clearing an empty map is cheap, so +// calling this on every teardown is fine even when no editor is open. +// +// IMPORTANT: every caller runs on a MAIN-thread / message-thread context — +// ClearChain and LoadPreset are N-API calls on the Node thread, doShutdown uses +// the inline variant directly. This is NOT called from a libuv worker (that is +// why LoadPreset closes editors before queuing LoadPresetWorker, rather than +// letting the worker do it). Given that: +// - Already on the message thread (doShutdown; ClearChain / LoadPreset on +// macOS, where Node's main thread IS the JUCE message thread) → tear down +// inline; posting-and-waiting on ourselves would deadlock. +// - Otherwise (ClearChain / LoadPreset on Linux/Windows, where the JUCE +// message thread is a dedicated std::thread) → post to that thread and block +// until the editors are gone. Its 50ms dispatch loop drains this promptly, +// so there is no macOS-style stall here. Report a refused post / wait +// timeout so a lingering-editor UAF stays diagnosable. +void closeAllPluginEditorWindows() +{ + auto* mm = juce::MessageManager::getInstanceWithoutCreating(); + if (mm != nullptr && mm->isThisTheMessageThread()) + { + destroyAllPluginEditorWindowsOnMessageThread(); + return; + } + + auto done = std::make_shared(); + const bool posted = juce::MessageManager::callAsync([done]() + { + destroyAllPluginEditorWindowsOnMessageThread(); + done->signal(); + }); + if (!posted) + { + fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: message queue refused the post; " + "editors may briefly outlive their processors\n"); + return; + } + if (!done->wait(15000)) + fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: editor teardown did not complete " + "within 15s; proceeding\n"); +} + +Napi::Value OpenPluginEditor(const Napi::CallbackInfo& info) +{ + auto env = info.Env(); + auto liveEngine = snapshotEngine(); + const auto slotIdOpt = argSlotId(info, 0); + if (!liveEngine || !slotIdOpt) + return Napi::Boolean::New(env, false); + const int slotId = *slotIdOpt; + + auto slot = liveEngine->getSignalChain().getSlot(slotId); + if (!slot || !slot->processor || !slot->processor->hasEditor()) + return Napi::Boolean::New(env, false); + + // Sandboxed plugins: the editor is a top-level window owned by the + // sandbox child process. No host-side PluginEditorWindow and no + // cross-process SetParent reparent — that path produced a blank + // rendered surface for D3D / OpenGL plugins (Neural DSP Archetypes, + // etc.) because their render context lives in the child. The child's + // kOpenEditor handler brings the existing window to front on a repeat + // click, so re-entry is cheap and we don't track host-side state. + // + // Dispatch off the N-API call thread: requestOpenEditor() uses a + // blocking control->request (kDefaultReplyTimeoutMs = 10s), which on + // a slow or hung sandbox would otherwise stall V8's JS thread for + // the full timeout. Capture slotId rather than a raw processor + // pointer and re-resolve inside the message-thread lambda — that + // closes a UAF window where the slot could be removed (or the engine + // torn down) between this call returning and the async firing. + // Return optimistically; matches the in-process path below. + // + // SandboxedProcessor is compiled on all desktop platforms now (the POSIX + // sandbox runtime is active — see src/audio/CMakeLists.txt), so the + // editor-open IPC routes to the sandbox child on macOS/Linux too. The + // child owns a floating editor window (Reaper-style); the host only tracks + // the open/closed bit. +#if defined(SLOPSMITH_AUDIO_ADDON) + if (auto* sb = dynamic_cast(slot->processor.get())) + { + // Synchronous gate: if the sandbox child is already gone (crashed + // or shut down) there's no point scheduling the IPC. Return false + // so the renderer can surface "editor unavailable" rather than + // toggling its UI into a fake-open state that no event will ever + // contradict. hasEditor() above already gated on isAlive() but a + // crash between then and now is possible — re-check here. + if (!sb->isAlive()) + return Napi::Boolean::New(env, false); + const bool queued = juce::MessageManager::callAsync([slotId]() + { + auto liveEngine = snapshotEngine(); + if (!liveEngine) return; + if (auto* slot = liveEngine->getSignalChain().getSlot(slotId)) + if (auto* sb = dynamic_cast(slot->processor.get())) + sb->requestOpenEditor(); + }); + if (!queued) + { + // Message queue refused the post — typically only during + // shutdown. Surface the failure so the renderer doesn't + // toggle its UI into a fake-open state. + return Napi::Boolean::New(env, false); + } + return Napi::Boolean::New(env, true); + } +#endif + + // In-process plugin — host-side PluginEditorWindow flow. If a window + // already exists for this slot, bring it to front rather than creating + // a duplicate. + auto it = editorWindows.find(slotId); + if (it != editorWindows.end() && it->second) + { + if (it->second->isVisible()) + { + it->second->toFront(true); + return Napi::Boolean::New(env, true); + } + // Window was hidden/closed, remove stale entry + editorWindows.erase(it); + } + + // Create editor on the message thread. Capture slotId only — re-resolve + // the slot via snapshotEngine() + getSlot(slotId) inside the lambda so a + // SignalChain::removeProcessor() between this call returning and the + // async firing can't leave us calling createEditorAndMakeActive() on a + // dangling juce::AudioProcessor*. Mirrors the sandbox branch's pattern. + const bool queued = juce::MessageManager::callAsync([slotId]() + { + auto liveEngine = snapshotEngine(); + if (!liveEngine) return; + auto& chain = liveEngine->getSignalChain(); + auto* slot = chain.getSlot(slotId); + if (!slot || !slot->processor) return; + + // ── Windows editor-crash class fix ─────────────────────────────────── + // An in-process VST3 editor is created on JUCE's BACKGROUND message + // thread (V8 owns the OS main thread inside a Node addon). On Windows a + // Qt-using / window-on-init plugin then faults via USER32->WndProc on + // WM_ACTIVATEAPP with NO host frame on the stack, so the SignalChain SEH + // guard can't catch it and the whole app dies (0xC0000005 / 0xC0000409). + // Fix: never open a VST3 editor in-process on Windows — promote the slot + // to the out-of-process sandbox (which hosts the editor on a real + // top-level message thread, the environment the plugin needs) and open + // it there. Compiled on every platform so the swap path keeps building; + // gated to Windows at runtime since the in-process editor is fine on + // macOS/Linux (no WndProc) and the sandbox hop is pure overhead there. + static constexpr bool kPromoteEditorToSandbox = + #if JUCE_WINDOWS + true; + #else + false; + #endif + if (kPromoteEditorToSandbox) + { + // Decide + snapshot state SAFELY. captureVstStateForPromotion runs + // hasEditor()/getStateInformation() under the audio lock and the SEH + // guard (see its contract), so they neither race process()'s + // processBlock nor fault the app — an UNguarded getStateInformation on + // the very plugins this promotion targets would reintroduce the editor + // crash on the message thread. It returns true only for a non-sandboxed + // in-process VST3 that actually has an editor. + juce::MemoryBlock state; + if (chain.captureVstStateForPromotion(slotId, state)) + { + const juce::String path = slot->path; // immutable; message-thread only + fprintf(stderr, "[AudioEngine] editor-open: promoting in-process VST3 to sandbox: slot %d '%s'\n", + slotId, path.toRawUTF8()); + + juce::PluginDescription desc; + desc.fileOrIdentifier = path; + desc.name = juce::File(path).getFileNameWithoutExtension(); + + // tryLoadSandboxed only accepts a plugin that shouldSandbox() + // approves, so pin this path to the runtime sandbox list first. + // Remember whether it was ALREADY pinned: if the promotion fails + // we undo only OUR pin below, so a healthy, never-crashed plugin + // isn't left permanently forced to a sandbox that just proved + // unavailable (while a pre-existing/real blocklist entry stays). + const bool wasAlreadyPinned = slopsmith::sandbox::isCrashedPlugin(path); + slopsmith::sandbox::addCrashedPlugin(path); + + bool promoted = false; + juce::String err; + auto sandboxed = slopsmith::sandbox::tryLoadSandboxed( + desc, chain.getCurrentSampleRate(), chain.getCurrentBlockSize(), err); + if (sandboxed) + { + if (state.getSize() > 0) + sandboxed->setStateInformation(state.getData(), (int) state.getSize()); + if (chain.replaceProcessor(slotId, std::move(sandboxed))) + { + promoted = true; + bool editorOpened = false; + if (auto* slot2 = chain.getSlot(slotId)) + if (auto* sb = dynamic_cast(slot2->processor.get())) + editorOpened = sb->requestOpenEditor(); + fprintf(stderr, "[AudioEngine] editor-open: sandbox promotion OK for slot %d (editor %s)\n", + slotId, editorOpened ? "opened" : "FAILED to open"); + } + else + { + fprintf(stderr, "[AudioEngine] editor-open: replaceProcessor failed for slot %d\n", slotId); + } + } + else + { + fprintf(stderr, "[AudioEngine] editor-open: sandbox promotion failed for '%s': %s\n", + path.toRawUTF8(), err.toRawUTF8()); + } + + // Undo our transient pin on failure so a plugin that never crashed + // isn't stranded on the (evidently unavailable) sandbox route. + if (! promoted && ! wasAlreadyPinned) + slopsmith::sandbox::removeCrashedPlugin(path); + + // Promoted or not, never fall through to the in-process editor on + // Windows — that is the WndProc/Qt crash path this branch exists + // to avoid. + return; + } + // Not promotable (non-VST / editor-less / already-sandboxed, or the + // guarded capture faulted and released the processor). Fall through to + // the in-process branch below, which is safe for all of those cases + // (an already-sandboxed slot opens its editor out-of-process; an + // editor-less or released processor simply opens no window). + } + + // In-process editor: non-VST3, editor-less, already-sandboxed, or POSIX + // (where the in-process editor is safe). + auto* processor = slot->processor.get(); + auto name = slot->name; + juce::AudioProcessorEditor* editor = nullptr; + try { + editor = processor->createEditorAndMakeActive(); + } catch (const std::exception& e) { + fprintf(stderr, "[AudioEngine] createEditorAndMakeActive crashed for '%s': %s\n", name.toRawUTF8(), e.what()); + } catch (...) { + fprintf(stderr, "[AudioEngine] createEditorAndMakeActive crashed for '%s': unknown error\n", name.toRawUTF8()); + } + if (editor) + { + editorWindows[slotId] = std::make_unique(editor, name); + fprintf(stderr, "[AudioEngine] Opened editor for slot %d: %s (%dx%d)\n", + slotId, name.toRawUTF8(), editor->getWidth(), editor->getHeight()); + } + }); + + return Napi::Boolean::New(env, queued); +} + +Napi::Value ClosePluginEditor(const Napi::CallbackInfo& info) +{ + auto env = info.Env(); + const auto slotIdOpt = argSlotId(info, 0); + if (!slotIdOpt) return Napi::Boolean::New(env, false); + const int slotId = *slotIdOpt; + + // Sandboxed plugins: route the close to the sandbox child via IPC. + // No host-side PluginEditorWindow exists for these. + // + // Same shape as the open path: dispatch off the N-API thread and + // re-resolve the slot inside the lambda. requestCloseEditor() + // ultimately writes to the control pipe (writeFrame can block up + // to ~5s on a stalled reader), so running it synchronously here + // would freeze JS / the renderer UI on a slow sandbox; the + // re-resolve guards against slot-removal UAF between the napi call + // and the async firing. + // + // All desktop platforms: route the close to the sandbox child via IPC + // (SandboxedProcessor is compiled everywhere now). In-process plugins fall + // through to the host-side editor-window teardown below. +#if defined(SLOPSMITH_AUDIO_ADDON) + if (auto liveEngine = snapshotEngine()) + { + if (auto* slot = liveEngine->getSignalChain().getSlot(slotId)) + { + if (slot->processor + && dynamic_cast(slot->processor.get())) + { + const bool queued = juce::MessageManager::callAsync([slotId]() + { + auto liveEngine = snapshotEngine(); + if (!liveEngine) return; + if (auto* slot = liveEngine->getSignalChain().getSlot(slotId)) + if (auto* sb = dynamic_cast(slot->processor.get())) + sb->requestCloseEditor(); + }); + return Napi::Boolean::New(env, queued); + } + } + } +#endif + + // In-process plugin — tear down the host-side editor window. + auto it = editorWindows.find(slotId); + if (it != editorWindows.end()) + { + juce::MessageManager::callAsync([slotId]() + { + editorWindows.erase(slotId); + }); + return Napi::Boolean::New(env, true); + } + return Napi::Boolean::New(env, false); +} + + +} // namespace slopsmith::addon diff --git a/src/audio/addon/EditorWindows.h b/src/audio/addon/EditorWindows.h new file mode 100644 index 0000000..3afe80b --- /dev/null +++ b/src/audio/addon/EditorWindows.h @@ -0,0 +1,28 @@ +#pragma once + +// EditorWindows — in-process plugin editor windows + the open/close bindings +// and the Windows sandbox-promotion flow (TLC plan phase 7 / §3.4). Moved +// verbatim from NodeAddon.cpp. Owns the slotId→window map (message-thread +// only) and the teardown helpers every chain-clearing path must run BEFORE +// freeing slot processors (use-after-free; feedBack-desktop#56). + +#include + +namespace slopsmith::addon { + +// Inline teardown: destroys every editor window. Caller MUST already be on +// the message thread (the window map holds JUCE GUI objects). doShutdown's +// UI teardown hook points here. +void destroyAllPluginEditorWindowsOnMessageThread(); + +// Tears down the in-process editor windows so they are destroyed before the +// caller frees the processors those editors point at. Safe from the Node +// thread (posts to the message thread and blocks, bounded) or the message +// thread itself (inline). Clearing an empty map is cheap. +void closeAllPluginEditorWindows(); + +// N-API bindings (registered by NodeAddon's export table). +Napi::Value OpenPluginEditor(const Napi::CallbackInfo& info); +Napi::Value ClosePluginEditor(const Napi::CallbackInfo& info); + +} // namespace slopsmith::addon