Merge pull request #120 from got-feedBack/fix/lifecycle-executor-p0

fix(audio): P0 lifecycle executor — serialized ops, generation guard, module pinning

scripts/bundle-slopsmith.sh

@@ -121,7 +121,11 @@ copy_plugin() {
     local dst="$2"
     mkdir -p "$dst"
     # Use cp -R rather than -r for portable symlink-following semantics.
-    cp -R "$src/." "$dst/"
+    # -L dereferences symlinks INSIDE the tree (rig_builder's
+    # vst/src/*/_shared -> ../_shared): Git Bash cannot create symlinks
+    # without Developer Mode / SeCreateSymbolicLink, and the bundle wants
+    # real files anyway.
+    cp -RL "$src/." "$dst/"
     find "$dst" -name '.git' -type d -prune -exec rm -rf {} +
 }
 

src/audio/CMakeLists.txt

@@ -12,6 +12,7 @@ set(AUDIO_SOURCES
     engine/SourcePool.cpp
     engine/ExtraInputs.cpp
     addon/AddonContext.cpp
+    addon/LifecycleExecutor.cpp
     addon/ChainOps.cpp
     addon/EditorWindows.cpp
     addon/DeviceBindings.cpp

src/audio/VSTHost.cpp

@@ -1,5 +1,6 @@
 #include "VSTHost.h"
 #include "VSTTrace.h"
+#include "addon/PluginModulePin.h"
 
 // The out-of-process scan path is compiled only into the audio addon
 // (SLOPSMITH_AUDIO_ADDON, set in src/audio/CMakeLists.txt). slopsmith-vst-host
@@ -513,6 +514,13 @@ std::unique_ptr<juce::AudioPluginInstance> VSTHost::loadPlugin(
         return nullptr;
     }
 
+    // P0 (guide §12): never unload a plugin module mid-session. JUCE frees
+    // the module when its last instance dies; a message already queued to a
+    // window/timer in that module then indirect-calls into unmapped code —
+    // the CFG fail-fast in the 2026-07-17 field dumps.
+    slopsmith::addon::pinPluginModuleForever(
+        matchedDesc.fileOrIdentifier.toRawUTF8());
+
     return instance;
 }
 
@@ -568,7 +576,8 @@ void VSTHost::loadPluginAsync(
     // copy threads through both lambda hops.
     formatManager.createPluginInstanceAsync(
         matchedDesc, sampleRate, blockSize,
-        [cb = std::move(callback), name = matchedDesc.name]
+        [cb = std::move(callback), name = matchedDesc.name,
+         fileOrId = matchedDesc.fileOrIdentifier]
         (std::unique_ptr<juce::AudioPluginInstance> instance, const juce::String& error)
         {
             VST_TRACE("VSTHost.loadPluginAsync: createPluginInstanceAsync END    "
@@ -584,6 +593,8 @@ void VSTHost::loadPluginAsync(
                                       : juce::String("Failed to create plugin instance"));
                 return;
             }
+            // P0: pin — see the sync loadPlugin path for rationale.
+            slopsmith::addon::pinPluginModuleForever(fileOrId.toRawUTF8());
             cb(std::move(instance), {});
         });
 }

src/audio/addon/AddonContext.cpp

@@ -164,6 +164,10 @@ void initialize(std::function<void()> uiTeardownHook)
     // instead of treating it as already-done.
     alreadyShutDown.store(false, std::memory_order_release);
 
+    // New engine lifetime: ops still queued against the previous engine
+    // (or a previous failed init) are stale from here on.
+    bumpEngineGeneration();
+
     // Start JUCE message thread first (no-op on macOS)
     startJuceMessageThread();
 
@@ -172,8 +176,13 @@ void initialize(std::function<void()> uiTeardownHook)
     std::this_thread::sleep_for(std::chrono::milliseconds(200));
 #endif
 
-    // Create engine on the JUCE message thread (or inline on macOS)
-    const bool initialized = dispatchOnMessageThread([]() {
+    // Create engine on the JUCE message thread (or inline on macOS).
+    // Unbounded wait (P0): on machines where the first device enumeration
+    // blocks the message thread for minutes (Focusrite ASIO, 2026-07-17
+    // dumps), init completes late instead of "failing" while the engine
+    // appears anyway — the desync that left JS driving a half-initialized
+    // addon.
+    const bool initialized = runLifecycleOpOk("engine-init", []() {
         std::shared_ptr<AudioEngine> liveEngine;
         {
             std::lock_guard<std::mutex> lock(engineMutex);
@@ -194,8 +203,9 @@ void initialize(std::function<void()> uiTeardownHook)
                     types[i].outputDevices.size());
     });
     if (!initialized)
-        fprintf(stderr, "[audio-native] initialize: engine creation did not complete "
-                        "on the message thread; audio bindings will no-op until re-init\n");
+        fprintf(stderr, "[audio-native] initialize: engine creation did not run "
+                        "(pump gone or generation moved); audio bindings will "
+                        "no-op until re-init\n");
 }
 
 void doShutdown()
@@ -217,6 +227,12 @@ void doShutdown()
     bool expected = false;
     if (!alreadyShutDown.compare_exchange_strong(expected, true)) return;
 
+    // Invalidate every lifecycle op still queued against the live engine:
+    // they run before our teardown op (FIFO) but must not touch a dying
+    // engine's devices. The teardown op below is stamped AFTER this bump,
+    // so it alone survives the generation check.
+    bumpEngineGeneration();
+
     // 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
@@ -225,7 +241,10 @@ void doShutdown()
 
     if (juceRunning.load() || snapshotEngine() || snapshotVstHost())
     {
-        const bool toreDown = dispatchOnMessageThread([]() {
+        // Bounded wait — the ONE bounded caller (LifecycleExecutor.h): at
+        // process exit, leaking the pump beats hanging exit forever behind
+        // a wedged driver call.
+        const bool toreDown = runLifecycleOpOk("engine-shutdown", []() {
             // Editors reference their slot's processor; engine.reset() below
             // frees the whole chain, so destroy the editor windows first (#56).
             if (shutdownUiTeardown) shutdownUiTeardown();
@@ -239,7 +258,7 @@ void doShutdown()
                 std::lock_guard<std::mutex> lock(vstHostMutex);
                 vstHost.reset();
             }
-        });
+        }, /*maxWaitMs=*/60000);
         if (!toreDown)
         {
             // Editor teardown / stopAudio / engine destruction have NOT

src/audio/addon/AddonContext.h

@@ -20,6 +20,7 @@
 
 #include "../AudioEngine.h"
 #include "../VSTHost.h"
+#include "LifecycleExecutor.h"
 
 #include <juce_events/juce_events.h>
 
@@ -74,36 +75,26 @@ inline bool dispatchOnMessageThread(Func&& func)
 // 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.
+// P0 (guide §12): routed through the LifecycleExecutor. False now means the
+// op verifiably did NOT run (pump gone, or the engine generation moved while
+// it was queued) — the old "false but it may still run later" desync state
+// is gone. The wait is unbounded; a watchdog logs every 15 s while a driver
+// call blocks the message thread.
+// CAPTURE RULE unchanged: 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
-    // 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
+    return runLifecycleOpOk("device-lifecycle",
+                            std::function<void()>(std::forward<Func>(func)));
+#else
+    // macOS: dispatch runs inline (no separate pump). Linux/ALSA keeps its
+    // long-standing "called from the Node main thread" contract untouched.
     func();
     return true;
+#endif
 }
 
 // Pending-async-load registry: LoadVSTWorker / LoadPresetWorker block on a

src/audio/addon/EditorWindows.cpp

@@ -86,35 +86,22 @@ void destroyAllPluginEditorWindowsOnMessageThread()
 //     timeout so a lingering-editor UAF stays diagnosable.
 bool closeAllPluginEditorWindows()
 {
-    auto* mm = juce::MessageManager::getInstanceWithoutCreating();
-    if (mm != nullptr && mm->isThisTheMessageThread())
+    // P0 (guide §12): serialized behind every other lifecycle op with an
+    // unbounded wait — the old 15 s give-up let a caller proceed to free
+    // slot processors while this teardown was still queued (#56, delayed).
+    // A false return now means the teardown verifiably did NOT run (pump
+    // gone / generation moved) — callers still must not free processors on
+    // false, but there is no longer a "maybe it runs later" state.
+    // Note: inline execution when already on the message thread is handled
+    // inside runLifecycleOp.
+    const bool toreDown = runLifecycleOpOk("editor-teardown", []()
     {
         destroyAllPluginEditorWindowsOnMessageThread();
-        return true;
-    }
-
-    auto done = std::make_shared<juce::WaitableEvent>();
-    const bool posted = juce::MessageManager::callAsync([done]()
-    {
-        destroyAllPluginEditorWindowsOnMessageThread();
-        done->signal();
     });
-    if (!posted)
-    {
-        fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: message queue refused the post; "
-                        "editors may still be alive\n");
-        return false;
-    }
-    if (!done->wait(15000))
-    {
-        // The queued teardown hasn't run: editors may still hold pointers into
-        // the chain. Callers must NOT free slot processors on a false return —
-        // proceeding here is exactly the #56 use-after-free, just delayed.
-        fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: editor teardown did not complete "
-                        "within 15s; caller must not free chain processors\n");
-        return false;
-    }
-    return true;
+    if (!toreDown)
+        fprintf(stderr, "[audio-native] closeAllPluginEditorWindows: teardown did "
+                        "not run; caller must not free chain processors\n");
+    return toreDown;
 }
 
 Napi::Value OpenPluginEditor(const Napi::CallbackInfo& info)

src/audio/addon/LifecycleExecutor.cpp

@@ -0,0 +1,155 @@
+// LifecycleExecutor implementation — see LifecycleExecutor.h for the
+// contract and the field evidence that motivated it (guide §12 P0).
+
+#include "LifecycleExecutor.h"
+
+#include <juce_events/juce_events.h>
+
+#include <atomic>
+#include <cstdio>
+#include <exception>
+#include <memory>
+#include <mutex>
+
+namespace slopsmith::addon {
+
+static std::atomic<std::uint64_t> engineGeneration{1};
+
+std::uint64_t currentEngineGeneration()
+{
+    return engineGeneration.load(std::memory_order_acquire);
+}
+
+std::uint64_t bumpEngineGeneration()
+{
+    return engineGeneration.fetch_add(1, std::memory_order_acq_rel) + 1;
+}
+
+// Submission lock: MessageManager::callAsync is itself FIFO, but two
+// threads racing between "decide to submit" and "post" could otherwise
+// observe an ordering different from the one their callers reasoned about.
+// All lifecycle submissions serialize here so queue order == submit order.
+static std::mutex submitMutex;
+
+static constexpr int kWatchdogIntervalMs = 15000;
+
+LifecycleOpResult runLifecycleOp(const char* name,
+                                 std::function<void()> func,
+                                 int maxWaitMs)
+{
+    const auto stamped = currentEngineGeneration();
+
+    auto* mm = juce::MessageManager::getInstanceWithoutCreating();
+    if (mm == nullptr)
+    {
+        fprintf(stderr, "[lifecycle] op '%s': no message pump; refusing\n", name);
+        return LifecycleOpResult::neverRan;
+    }
+
+    // Already on the message thread (engine init path, macOS inline mode,
+    // or a lifecycle op submitting a nested op): dispatch-and-wait on
+    // ourselves would deadlock, so run inline.
+    //
+    // Ordering note (CodeRabbit #120): an inline op runs as part of the
+    // CURRENT message-thread turn, i.e. ahead of ops still queued behind
+    // it. That is intentional, not a FIFO leak: a nested submission is
+    // semantically part of the outer op and must complete inside it, and a
+    // message-thread caller can never interleave with a *running* op — the
+    // queue drains on this very thread. The FIFO contract is between
+    // off-thread submitters, which all take the queued path below.
+    if (mm->isThisTheMessageThread())
+    {
+        if (stamped != currentEngineGeneration())
+        {
+            fprintf(stderr, "[lifecycle] op '%s': stale generation (inline); skipped\n", name);
+            return LifecycleOpResult::stale;
+        }
+        try { func(); }
+        catch (const std::exception& e) {
+            fprintf(stderr, "[lifecycle] op '%s' (inline) threw: %s\n", name, e.what());
+        }
+        catch (...) {
+            fprintf(stderr, "[lifecycle] op '%s' (inline) threw (unknown)\n", name);
+        }
+        return LifecycleOpResult::completed;
+    }
+
+    struct Shared {
+        juce::WaitableEvent done;
+        std::atomic<bool> wasStale{false};
+    };
+    auto shared = std::make_shared<Shared>();
+
+    bool posted = false;
+    {
+        std::lock_guard<std::mutex> lock(submitMutex);
+        posted = juce::MessageManager::callAsync(
+            [func = std::move(func), shared, stamped, name]() mutable {
+                if (stamped != currentEngineGeneration())
+                {
+                    fprintf(stderr, "[lifecycle] op '%s': engine generation moved "
+                                    "%llu -> %llu while queued; skipped\n",
+                            name,
+                            (unsigned long long) stamped,
+                            (unsigned long long) currentEngineGeneration());
+                    shared->wasStale.store(true, std::memory_order_release);
+                }
+                else
+                {
+                    // Always reach the signal below: a throwing op would
+                    // otherwise leave its unbounded caller waiting forever
+                    // (CodeRabbit #120).
+                    try { func(); }
+                    catch (const std::exception& e) {
+                        fprintf(stderr, "[lifecycle] op '%s' threw: %s\n", name, e.what());
+                    }
+                    catch (...) {
+                        fprintf(stderr, "[lifecycle] op '%s' threw (unknown)\n", name);
+                    }
+                }
+                shared->done.signal();
+            });
+    }
+    if (!posted)
+    {
+        fprintf(stderr, "[lifecycle] op '%s': message queue refused the post; "
+                        "op will not run\n", name);
+        return LifecycleOpResult::neverRan;
+    }
+
+    int waitedMs = 0;
+    while (!shared->done.wait(kWatchdogIntervalMs))
+    {
+        waitedMs += kWatchdogIntervalMs;
+
+        // Pump death while we wait means the op can never run. Distinguish
+        // from a merely blocked pump: the MessageManager singleton is
+        // destroyed by stopJuceMessageThread.
+        if (juce::MessageManager::getInstanceWithoutCreating() == nullptr)
+        {
+            fprintf(stderr, "[lifecycle] op '%s': message pump died after %d ms "
+                            "wait; op will not run\n", name, waitedMs);
+            return LifecycleOpResult::neverRan;
+        }
+
+        if (maxWaitMs > 0 && waitedMs >= maxWaitMs)
+        {
+            fprintf(stderr, "[lifecycle] op '%s': abandoning wait after %d ms "
+                            "(bounded caller); op may still run\n", name, waitedMs);
+            return LifecycleOpResult::abandonedWait;
+        }
+
+        // Watchdog: report, never decide (guide §12 P0 item 5). The usual
+        // culprit for a multi-second stall is an audio driver call (ASIO
+        // open/reset) blocking the message thread.
+        fprintf(stderr, "[lifecycle] watchdog: op '%s' still waiting after %d ms — "
+                        "message thread blocked (audio driver call?); continuing "
+                        "to wait\n", name, waitedMs);
+    }
+
+    return shared->wasStale.load(std::memory_order_acquire)
+               ? LifecycleOpResult::stale
+               : LifecycleOpResult::completed;
+}
+
+} // namespace slopsmith::addon

src/audio/addon/LifecycleExecutor.h

@@ -0,0 +1,78 @@
+#pragma once
+
+// LifecycleExecutor — P0 of the audio architecture guide (§12 P0).
+//
+// One serialized queue for native lifecycle mutations (engine init/shutdown,
+// device create/destroy, editor teardown). Ops are named, stamped with the
+// engine generation at submit time, and executed strictly in submission
+// order on the JUCE message thread.
+//
+// What this replaces: the "dispatch and give up after 15 s" contract
+// (AddonContext.h, KNOWN LIMITATION). Field evidence 2026-07-17 (three
+// identical CFG fail-fast dumps): a caller that abandons its wait proceeds
+// to mutate lifecycle state while the abandoned op is still queued — two
+// owners, no ordering authority. The executor removes the abandon path:
+//
+//   - The wait is not bounded by a give-up. A watchdog logs every
+//     kWatchdogIntervalMs while an op is stalled (message thread blocked by
+//     a driver call) and keeps waiting. The op result the caller sees is
+//     always the truth of what ran.
+//   - The only false returns are "the op will never run" (message pump gone
+//     or post refused) and "the op was stale" (engine generation changed
+//     between submit and execution — see below). Callers can treat false as
+//     final; there is no "maybe it still runs later" state.
+//   - `maxWaitMs` exists for the one caller that must not block forever:
+//     process shutdown, where leaking the pump beats hanging exit. Everyone
+//     else passes 0 (unbounded).
+//
+// Generation guard: initialize/doShutdown bump the engine generation. An op
+// stamped under an older generation no-ops when it finally executes, so
+// work that was queued against a torn-down engine can never touch its
+// replacement. This is the lifecycle-level generalization of
+// chainGeneration.
+//
+// The executor serializes; it never authorizes. Policy (who may mutate
+// what) belongs to the JS-side lease registry (guide §7.1).
+//
+// macOS: no background pump exists (AppKit owns the real main thread —
+// AddonContext.cpp fork note), so ops execute inline on the caller thread,
+// still generation-checked. Same contract, degenerate queue.
+
+#include <cstdint>
+#include <functional>
+
+namespace slopsmith::addon {
+
+// Monotonic engine generation. Bumped by initialize()/doShutdown().
+std::uint64_t currentEngineGeneration();
+std::uint64_t bumpEngineGeneration();
+
+enum class LifecycleOpResult {
+    completed,      // op ran to completion under its submit-time generation
+    stale,          // generation changed before the op ran; op no-oped
+    neverRan,       // pump gone / post refused — op will never run
+    abandonedWait,  // maxWaitMs expired; op may still run (shutdown-only path)
+};
+
+// Run `func` on the JUCE message thread, serialized behind every previously
+// submitted lifecycle op. Blocks until the op completes (watchdog logs
+// while stalled). `name` appears in every log line and in the watchdog.
+// `maxWaitMs` = 0 means wait indefinitely (bail only if the pump dies).
+LifecycleOpResult runLifecycleOp(const char* name,
+                                 std::function<void()> func,
+                                 int maxWaitMs = 0);
+
+// Convenience: true only for LifecycleOpResult::completed.
+inline bool runLifecycleOpOk(const char* name, std::function<void()> func,
+                             int maxWaitMs = 0)
+{
+    return runLifecycleOp(name, std::move(func), maxWaitMs)
+           == LifecycleOpResult::completed;
+}
+
+// Plugin-module pinning (guide §12 P0 item 4) lives in PluginModulePin.h —
+// header-only, because VSTHost.cpp is compiled into targets across three
+// CMake projects and an out-of-line definition broke the ones that don't
+// build this executor (PR #120 CI).
+
+} // namespace slopsmith::addon

src/audio/addon/PluginModulePin.h

@@ -0,0 +1,99 @@
+#pragma once
+
+// Plugin-module pinning — P0 item 4 of the audio architecture guide (§12).
+//
+// JUCE refcounts VST3 modules and unloads them when the last instance dies;
+// a window/timer message already queued to that module's code then fires
+// into unmapped or reused pages — the CFG fail-fast
+// (0xc0000409 / FAST_FAIL_GUARD_ICALL_CHECK_FAILURE) in the 2026-07-17
+// field dumps. Pinning tells the loader to never unload the module for the
+// life of the process.
+//
+// Header-only ON PURPOSE: VSTHost.cpp is compiled into four different
+// targets across three CMake projects (the addon, slopsmith-vst-host in
+// two projects, slopsmith-vst-scan on macOS); an out-of-line definition
+// broke every link that didn't add the extra .cpp (PR #120 CI).
+//
+// The pin matches loaded modules by PATH, not base name: the identifier we
+// get is usually the bundle directory (…\Foo.vst3\), while the loader knows
+// the inner …\Contents\x86_64-win\Foo.vst3 file — and two plugins may share
+// a base name. Enumerate loaded modules and pin every one whose full path
+// starts with the normalized identifier (K32* exports so no psapi.lib).
+// No-op off Windows and for modules that are not currently loaded (e.g.
+// sandboxed plugins — the child process owns those).
+
+#include <cstdio>
+
+#if defined(_WIN32)
+ #ifndef WIN32_LEAN_AND_MEAN
+  #define WIN32_LEAN_AND_MEAN
+ #endif
+ #ifndef NOMINMAX
+  #define NOMINMAX
+ #endif
+ #include <windows.h>
+ // PSAPI_VERSION 2 maps EnumProcessModules/GetModuleFileNameExW onto the
+ // kernel32-exported K32* variants — no psapi.lib link needed (matters for
+ // the extra targets this header serves; see the header-only note above).
+ #ifndef PSAPI_VERSION
+  #define PSAPI_VERSION 2
+ #endif
+ #include <psapi.h>
+ #include <vector>
+ #include <cwctype>
+#endif
+
+#include <juce_core/juce_core.h>
+
+namespace slopsmith::addon {
+
+inline void pinPluginModuleForever(const char* fileOrIdentifierUtf8)
+{
+#if defined(_WIN32)
+    const juce::String wanted =
+        juce::String::fromUTF8(fileOrIdentifierUtf8).replaceCharacter('/', '\\');
+    if (wanted.isEmpty())
+        return;
+
+    auto startsWithIgnoreCase = [](const wchar_t* full, const wchar_t* prefix) {
+        while (*prefix != 0)
+        {
+            if (*full == 0
+                || std::towlower(static_cast<wint_t>(*full))
+                       != std::towlower(static_cast<wint_t>(*prefix)))
+                return false;
+            ++full;
+            ++prefix;
+        }
+        return true;
+    };
+
+    std::vector<HMODULE> modules(1024);
+    DWORD needed = 0;
+    if (!EnumProcessModules(GetCurrentProcess(), modules.data(),
+                               static_cast<DWORD>(modules.size() * sizeof(HMODULE)),
+                               &needed))
+        return;
+    modules.resize(needed / sizeof(HMODULE));
+
+    for (HMODULE mod : modules)
+    {
+        wchar_t modPath[MAX_PATH * 2] = {};
+        if (GetModuleFileNameExW(GetCurrentProcess(), mod, modPath,
+                                    static_cast<DWORD>(sizeof(modPath) / sizeof(modPath[0]))) == 0)
+            continue;
+        if (!startsWithIgnoreCase(modPath, wanted.toWideCharPointer()))
+            continue;
+
+        HMODULE pinned = nullptr;
+        if (GetModuleHandleExW(GET_MODULE_HANDLE_EX_FLAG_PIN, modPath, &pinned))
+            fprintf(stderr, "[lifecycle] pinned plugin module '%s' for process "
+                            "lifetime\n",
+                    juce::String(modPath).toRawUTF8());
+    }
+#else
+    (void) fileOrIdentifierUtf8;
+#endif
+}
+
+} // namespace slopsmith::addon