fix(vst): reopen heal without the editor — bounded main-thread retry timer, re-armable first-poll heal, unconditional preview-off consume + Mono/drain preview tests

src/vst/reasampler_processor.cpp

@@ -25,6 +25,13 @@
 #include "sample_map.h"       // selectSample, resolvePerformance, buildZonedKeymap, state (de)ser
 #include "wav_trim.h"         // parseWavLayout, extractFloatFrames (shared WAV parse)
 
+#ifdef _WIN32
+#ifndef NOMINMAX
+#define NOMINMAX
+#endif
+#include <windows.h>  // SetTimer/KillTimer (the reopen-heal retry's main-thread timer)
+#endif
+
 using namespace Steinberg;
 using namespace Steinberg::Vst;
 
@@ -84,16 +91,99 @@ std::optional<DecodedZonePcm> decodeRelative(const std::string& projectDir,
     return out;
 }
 
+#ifdef _WIN32
+// --- Reopen-heal retry timer (the non-editor reload trigger; see the header) -----------
+// HWND-less Win32 thread timer: SetTimer(nullptr, ...) queues WM_TIMER on the CALLING
+// thread's message queue and the TIMERPROC fires from its pump — REAPER's main thread,
+// where every reload path already runs (setState, setActive, the editor, this timer).
+// A TIMERPROC carries no user data, so a tiny id->instance registry maps a fired timer
+// back to its processor. Set/kill/fire all happen on the pumping thread; the mutex is
+// defensive against an exotic host threading setState from elsewhere (in which case
+// SetTimer would not fire there anyway and we degrade to the pre-fix editor-open heal).
+constexpr UINT kHealRetryIntervalMs = 250;  // fast enough to catch the tail of project load
+constexpr int kHealRetryMax = 40;           // ~10 s, then stop churning (e.g. missing WAV)
+
+std::mutex g_healRegistryMutex;
+std::vector<std::pair<std::uintptr_t, ReaSamplerProcessor*>> g_healRegistry;
+
+void CALLBACK healTimerProc(HWND, UINT, UINT_PTR id, DWORD) {
+    ReaSamplerProcessor* target = nullptr;
+    {
+        std::lock_guard<std::mutex> lock(g_healRegistryMutex);
+        for (const auto& entry : g_healRegistry) {
+            if (entry.first == static_cast<std::uintptr_t>(id)) {
+                target = entry.second;
+                break;
+            }
+        }
+    }
+    if (!target) {
+        // Orphan fire (the processor disarmed/destroyed between queue and dispatch):
+        // stop the timer here — nobody else holds this id anymore.
+        KillTimer(nullptr, id);
+        return;
+    }
+    // The registry lock is RELEASED before the tick: healTick -> reloadFromBank takes
+    // reloadMutex_ then (via arm/disarm) the registry mutex — one consistent order.
+    target->healTick();
+}
+#endif  // _WIN32
+
 }  // namespace
 
+void ReaSamplerProcessor::armHealRetry() {
+#ifdef _WIN32
+    // Main thread. Idempotent: an already-armed timer keeps its running countdown (the
+    // retry ticks call reloadFromBank, which calls back here on every failed rebuild).
+    if (healTimerId_ != 0) return;
+    const UINT_PTR id = SetTimer(nullptr, 0, kHealRetryIntervalMs, &healTimerProc);
+    if (id == 0) return;  // no message pump on this thread / OS refusal: editor-open heal remains
+    healTimerId_ = static_cast<std::uintptr_t>(id);
+    healRetriesLeft_ = kHealRetryMax;
+    std::lock_guard<std::mutex> lock(g_healRegistryMutex);
+    g_healRegistry.emplace_back(healTimerId_, this);
+#endif
+}
+
+void ReaSamplerProcessor::disarmHealRetry() {
+#ifdef _WIN32
+    // Main thread. The common (already-disarmed) path costs one compare — this is called
+    // at the end of every successful reload.
+    if (healTimerId_ == 0) return;
+    KillTimer(nullptr, static_cast<UINT_PTR>(healTimerId_));
+    {
+        std::lock_guard<std::mutex> lock(g_healRegistryMutex);
+        g_healRegistry.erase(
+            std::remove_if(g_healRegistry.begin(), g_healRegistry.end(),
+                           [this](const auto& entry) { return entry.second == this; }),
+            g_healRegistry.end());
+    }
+    healTimerId_ = 0;
+#endif
+}
+
+void ReaSamplerProcessor::healTick() {
+    // Main thread (the heal timer's TIMERPROC). One bounded retry: reloadFromBank re-reads
+    // the bank over the bridge and itself disarms this timer when it builds an instrument
+    // (or the restored intent is gone). If it stays armed, count the budget down and give
+    // up at zero — a genuinely-missing WAV must not poll ext-state forever.
+    if (healTimerId_ == 0) return;  // raced a disarm between queue and dispatch
+    --healRetriesLeft_;
+    reloadFromBank();
+    if (healTimerId_ != 0 && healRetriesLeft_ <= 0) disarmHealRetry();
+}
+
 FUnknown* ReaSamplerProcessor::createInstance(void* /*context*/) {
     // The host owns the returned reference. Cast up to the combined interface the SDK
     // exposes (IAudioProcessor) so the FUnknown refcount is correctly rooted.
     return static_cast<IAudioProcessor*>(new ReaSamplerProcessor());
 }
 
-// Out-of-line so unique_ptr<ReaSamplerEmbed> sees the complete type here.
-ReaSamplerProcessor::~ReaSamplerProcessor() = default;
+// Out-of-line so unique_ptr<ReaSamplerEmbed> sees the complete type here. The heal-retry
+// disarm is defensive (terminate already disarms per the VST3 lifecycle): it removes this
+// instance from the timer registry so a host that skips terminate can never leave a fired
+// TIMERPROC holding a dangling pointer.
+ReaSamplerProcessor::~ReaSamplerProcessor() { disarmHealRetry(); }
 
 tresult PLUGIN_API ReaSamplerProcessor::queryInterface(const TUID iid, void** obj) {
     // S6: expose REAPER's inline-embed interface. REAPER queries the IEditController for
@@ -135,9 +225,11 @@ tresult PLUGIN_API ReaSamplerProcessor::initialize(FUnknown* context) {
 }
 
 tresult PLUGIN_API ReaSamplerProcessor::terminate() {
-    // process() is not running at terminate. Free the live + draining instruments and
-    // drain the graveyard. Take the pointers out of the atomics first so nothing else
+    // process() is not running at terminate. Stop the reopen-heal retry timer first (its
+    // tick would reload into a dying instance), then free the live + draining instruments
+    // and drain the graveyard. Take the pointers out of the atomics first so nothing else
     // races them.
+    disarmHealRetry();
     std::lock_guard<std::mutex> lock(reloadMutex_);
     delete live_.exchange(nullptr);
     delete draining_.exchange(nullptr);
@@ -152,8 +244,15 @@ tresult PLUGIN_API ReaSamplerProcessor::setActive(TBool state) {
     // no reload could free while active). The build/drain are off the audio thread —
     // setActive is a main/UI-thread call.
     if (state) {
+        // NOTE: this is also the non-editor reload trigger's anchor — if this reload runs
+        // before the extension's PROJEXTSTATE is parseable, reloadFromBank arms the bounded
+        // heal-retry timer itself (see the header), so a restored instance played via host
+        // MIDI with the editor never opened still comes up sounding.
         reloadFromBank();
     } else {
+        // An inactive instance has nothing to heal into — stop the retry; the reactivation
+        // reload above re-arms it if the bank is still not parseable then.
+        disarmHealRetry();
         std::lock_guard<std::mutex> lock(reloadMutex_);
         // process is guaranteed stopped: free EVERYTHING. The live instrument too — its
         // voices are frozen mid-flight, and if it survived deactivation the reactivate
@@ -551,7 +650,22 @@ std::string ReaSamplerProcessor::reloadFromBank() {
     //    `built` is heap-owned; release() hands ownership to the atomic; the drain-evicted
     //    pointer is re-owned by the graveyard.
     //
+    const bool loaded = static_cast<bool>(built);
     publishBuiltLocked(std::move(built));
+
+    // Reopen-heal retry, the NON-editor trigger (see the header): this reload built NOTHING
+    // despite restored intent (a selection or zones) while the bridge is connected — during
+    // project load that almost always means the extension's PROJEXTSTATE block is not yet
+    // parseable — so arm the bounded main-thread retry timer. Every other outcome disarms:
+    // the timer only lives while there is something to heal. (Leaf-mutex order holds:
+    // reloadMutex_ -> registry mutex, matching the TIMERPROC which releases the registry
+    // before ticking into this function.)
+    const bool intent = !selectedSampleId().empty() || !performanceMap().empty();
+    if (!loaded && intent && bridge_.isConnected()) {
+        armHealRetry();
+    } else {
+        disarmHealRetry();
+    }
     return resolvedId;
 }
 
@@ -737,6 +851,18 @@ ReaSamplerProcessor::pollBankSync(bool isFocusedTarget) {
         // editor re-snapshots its bank view.
         result.reloaded = genChanged || healReload;
     }
+
+    // Heal RETRY: if the heal reload above STILL left nothing live (the first poll was
+    // itself too early — the bank blob still absent/unparseable), reset the sentinel so
+    // the NEXT tick re-arms the first-poll heal instead of spending it one-shot. Bounded
+    // by the intent check inside the heal (a deliberately-empty instance never sets
+    // healReload, so never re-arms). This also heals a bank whose generation counter was
+    // never bumped: `bankGenerationChanged` is a plain != against a counter that stays 0
+    // for such a bank (0 != 0 never fires), so the generation path alone could never
+    // recover it.
+    if (healReload && live_.load(std::memory_order_acquire) == nullptr) {
+        lastSeenBankGeneration_ = -1;
+    }
     return result;
 }
 
@@ -850,14 +976,20 @@ tresult PLUGIN_API ReaSamplerProcessor::process(ProcessData& data) {
             }
         }
     }
-    if (inst || drain) {
+    {
         const std::uint32_t off = previewOffRequest_.load(std::memory_order_acquire);
         const std::uint16_t offSeq = static_cast<std::uint16_t>(off >> 16);
         if (offSeq != 0 && offSeq != previewOffConsumed_) {
+            // Consume UNCONDITIONALLY (mirror of the on path): a stale off left pending
+            // while nothing was loaded would otherwise survive until a (heal) reload lands
+            // and release the NEXT preview press in the same block.
             previewOffConsumed_ = offSeq;
             // Route the preview note-off to BOTH engines (mirror of the host note-off): a
             // preview held across a reload — e.g. a curve edit committed mid-press — must
             // release the old-snapshot voice now draining, not just the (fresh) live one.
+            // NOTE: preview shares the host-MIDI note space — noteOff releases the newest
+            // voice at that pitch, so a preview release can release a host-held note at
+            // the same pitch (inherent to routing preview through the real note path).
             if (inst) inst->engine.noteOff(static_cast<int>(off & 0xFF));
             if (drain) drain->engine.noteOff(static_cast<int>(off & 0xFF));
         }

src/vst/reasampler_processor.h

@@ -240,7 +240,32 @@ public:
     void previewNoteOn(int note);
     void previewNoteOff(int note);
 
+    // Reopen-heal retry tick — the target of the module-internal Win32 heal timer (see
+    // armHealRetry below), NOT host-facing. Public only because the file-static TIMERPROC
+    // in the .cpp must reach it. Main thread; re-runs reloadFromBank (which disarms the
+    // timer itself on success) and disarms when the bounded retry budget runs out.
+    void healTick();
+
 private:
+    // --- Reopen-heal retry (the NON-editor reload trigger) --------------------------
+    // A project-restored instance with intent (a selection or zones) can come up SILENT:
+    // REAPER runs track-FX setState before the extension's PROJEXTSTATE block is parsed,
+    // so the setState-time reload reads an empty bank. The editor's WM_TIMER poll heals
+    // that — but only if the user opens the editor; an instance played via host MIDI with
+    // the editor never attached stayed silent indefinitely. Mechanism: reloadFromBank
+    // itself detects "built NOTHING despite restored intent, bridge connected" (off the
+    // audio thread — it just tried) and arms a BOUNDED, HWND-less Win32 retry timer
+    // (SetTimer + TIMERPROC: fires on the arming thread's message pump — REAPER's main
+    // thread, where every reload path already runs). Each tick re-runs reloadFromBank,
+    // which disarms on success or when the intent is gone; the bound stops the churn for
+    // an instance whose WAV is genuinely missing. A deliberately-empty instance never
+    // arms (no intent). process() is untouched — fully RT-safe. Main-thread only.
+    // No-ops on non-Windows builds (the VST target is Windows-only).
+    void armHealRetry();
+    void disarmHealRetry();
+    std::uintptr_t healTimerId_ = 0;  // 0 = not armed (main thread only)
+    int healRetriesLeft_ = 0;         // remaining timer-tick retries (main thread only)
+
     // Phase S drain retirement (FA1-review Major #2): if process() has published that the
     // CURRENT drain instrument is fully idle (every engine voice silent),
     // move it out of the drain slot into the graveyard and prune — so an edited-away snapshot
@@ -356,7 +381,11 @@ private:
     // intent (a selection or zones) — the project-load ordering can run setState before the
     // extension's PROJEXTSTATE block is parseable, so the bridge read came back empty — the
     // first poll reloads instead of silently baselining, or the instrument would stay silent
-    // until some param change forced a reload. NOT read on the audio thread.
+    // until some param change forced a reload. If that heal reload STILL leaves nothing live,
+    // pollBankSync resets this back to the -1 sentinel so the next tick re-arms the heal —
+    // the retry is bounded by the intent check (a deliberately-empty instance never heals),
+    // and it also covers a bank whose generation counter was never bumped (0 != 0 can never
+    // fire the generation path). NOT read on the audio thread.
     std::int64_t lastSeenBankGeneration_ = -1;
 
     // S-VIEW-4 preview-trigger velocity (MIDI 1..127). Persisted in component state (v6) so the

tests/test_sampler_core.cpp

@@ -2253,6 +2253,48 @@ static void testPreviewNoteObeysVoicing() {
     CHECK(approx(buf2[0], 1.0, 1e-6));
 }
 
+// PREVIEW JOINS THE MONO HELD STACK: a preview routed through the real note path is a mono
+// stack entry like any host note — it TAKES the single voice on press (last-note priority)
+// and its release FALLS BACK to the still-held host note instead of cutting to silence.
+// Pins the processor's mailbox-drain contract for Mono the way testPreviewNoteObeysVoicing
+// pins it for Poly steal.
+static void testPreviewNoteJoinsMonoHeldStack() {
+    Keymap km = twoLevelKeymap();
+    VoiceEngine eng(4, km, 0, 0, VoiceMode::Mono, MonoTrigger::Retrigger);
+    CHECK(eng.noteOn(50, 127) == 0);            // the host-MIDI note: zone A sounds
+    CHECK(approx(probeFrame(eng), 0.25, 1e-6));
+    CHECK(eng.noteOn(70, 127) == 0);            // the preview press: TAKES the voice
+    CHECK(eng.activeVoiceCount() == 1);         // still mono — the preview is no side-car
+    CHECK(approx(probeFrame(eng), 0.75, 1e-6));
+    eng.noteOff(70);                            // preview release: FALLBACK to the held note
+    CHECK(approx(probeFrame(eng), 0.25, 1e-6));
+    eng.noteOff(50);                            // host note up: gate off (flat release = instant)
+    CHECK(approx(probeFrame(eng), 0.0, 1e-9));
+    CHECK(eng.activeVoiceCount() == 0);
+}
+
+// PREVIEW NOTE-OFF ROUTES TO THE DRAIN ENGINE: mirror of process()'s dual-engine off
+// routing. A preview held across a reload leaves its ringing voice in the DISPLACED
+// (draining) snapshot while the fresh live engine has no voice at that pitch. The off is
+// sent to BOTH — exactly what the mailbox drain does: the fresh engine must safely no-op,
+// the drain engine must release its voice (otherwise the old-snapshot preview would
+// sustain until the next reload hard-cut it).
+static void testPreviewNoteOffRoutesToDrainEngine() {
+    Keymap km = twoLevelKeymap();
+    VoiceEngine drainEng(2, km);                // was live when the preview fired
+    VoiceEngine liveEng(2, km);                 // the post-reload fresh snapshot: no voices
+    CHECK(drainEng.noteOn(70, 127) != VoiceEngine::kNoVoice);
+    CHECK(approx(probeFrame(drainEng), 0.75, 1e-6));  // the preview rings in the old snapshot
+    CHECK(liveEng.activeVoiceCount() == 0);
+    // The preview release, drained to BOTH engines like a host note-off:
+    liveEng.noteOff(70);
+    drainEng.noteOff(70);
+    CHECK(approx(probeFrame(liveEng), 0.0, 1e-9));    // fresh engine: safe no-op, stays silent
+    CHECK(liveEng.activeVoiceCount() == 0);
+    CHECK(approx(probeFrame(drainEng), 0.0, 1e-9));   // flat release: gates off NOW
+    CHECK(drainEng.activeVoiceCount() == 0);          // the old-snapshot voice released
+}
+
 // GA2 — bounded-blend overshoot regression: mid-ramp output must stay within full scale.
 //
 // Construction of the worst case (§1 reviewer finding): retrig a sine at a point where the
@@ -2534,6 +2576,8 @@ int main() {
     testPreviewReauditionDeclicksViaEngineSteal();
     testDeclickBoundedBlendNoOvershoot();
     testPreviewNoteObeysVoicing();
+    testPreviewNoteJoinsMonoHeldStack();
+    testPreviewNoteOffRoutesToDrainEngine();
 
     // GA3 — Preserve tail wind-down (writer freeze at source exhaustion).
     testPreserveTailFinalWindowGapFree();