Fix the reclaimer stranding a tab-switched-into project: lift the pin and wakeup into FxReclaimQueue, retarget instead of going idle

Also validate the cached track in its project, and stop a drop-free tick resetting the report memo.

src/app/main.cpp

@@ -222,11 +222,11 @@ static void OnTimer()
 
     // Converge the FX chains toward what the view state says they should be — the
     // 1897ms of TrackFX_SetOffline a mode switch no longer pays synchronously. Same
-    // idle discipline as the realtime fast path above: a SINGLE BOOL TEST when there is
+    // idle discipline as the realtime fast path above: ONE bool read when there is
     // nothing to converge. Ordered AFTER the load block deliberately — a reload replaces
     // the view model wholesale, and the applyMode above is what re-derives the queue
     // from it, so draining first would spend a tick against the outgoing model.
-    if (reasampler::g_fxReclaimPending)
+    if (reasampler::fxReclaimPending())
         reasampler::fxReclaimTick(g_session, nullptr);
 
     reasampler::bankPanelRefresh();  // cheap fingerprint compare; no-op when unchanged/closed

src/shell/view/fx_reclaim.cpp

@@ -30,12 +30,9 @@ namespace reasampler {
 
 namespace {
 
-// The derived queue and the project it was derived against. The pointer is
+// The derived queue, the project it was derived against, and the timer's wakeup — see
-// COMPARE-ONLY and never dereferenced beyond the ValidatePtr2 the tick already makes —
+// FxReclaimQueue, which owns all three.
-// a tab switch has to be detectable without following a pointer that may name a closed
-// project.
 FxReclaimQueue g_queue;
-ReaProject*    g_project = nullptr;
 
 // TrackFX_SetOffline is ASSUMED to pump the message loop, so a plugin load can re-enter
 // the timer and land back here mid-drain. Fail closed: a re-entered drain would derive a
@@ -86,9 +83,11 @@ struct ReclaimHost {
     std::string cachedGuid_;
     MediaTrack* cachedTrack_ = nullptr;
 
+    // A track is validated IN a project (reaper_plugin_functions.h:7898); the null form
+    // is defined only where the pointer is itself a project.
     MediaTrack* resolveForStep(const std::string& guid) {
         if (guid == cachedGuid_ && cachedTrack_ &&
-            ValidatePtr2(nullptr, cachedTrack_, "MediaTrack*"))
+            ValidatePtr2(project, cachedTrack_, "MediaTrack*"))
             return cachedTrack_;
         cachedTrack_ = resolveFresh(project, guid);
         cachedGuid_  = guid;
@@ -171,24 +170,15 @@ void reportDrops(ReaProject* proj, const std::vector<TrackFxDrops>& drops) {
     if (!msg.empty()) ShowConsoleMsg(("!SHOW:" + msg).c_str());
 }
 
-// Every path that stops scheduling ticks leaves the same state behind: no queue, no pin,
-// nothing pending. The debounce clock is NOT part of it — see fx_reclaim.h.
-void goIdle() {
-    g_queue.discard();
-    g_project = nullptr;
-    g_fxReclaimPending = false;
-}
-
 }  // namespace
 
-bool g_fxReclaimPending = false;
+bool fxReclaimPending() { return g_queue.pending(); }
 
 void invalidateFxReclaim() {
     // A generation bump and a flag, and deliberately nothing more. Discarding the queue
     // here is what let a re-entered apply free the element the outer drain was writing
     // through, and clear the very wakeup this call had just raised.
     g_queue.invalidate();
-    g_fxReclaimPending = true;
 }
 
 bool fxReclaimDrainInProgress() { return g_draining; }
@@ -200,27 +190,17 @@ void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
     if (modeApplyInProgress()) return;
     DrainLatch latch;
 
-    // Belt-and-braces, not the tab-switch mechanism: OnTimer runs session.poll() before
+    // beginTick owns what a pin/project mismatch means. What is local to here: the model
-    // this tick, and poll() already classifies a tab switch as a Load whose reapply
+    // the re-derivation below reads is the ACTIVE project's, because reaching this line
-    // invalidates the queue — so by the time this test runs, a switch has normally
+    // means OnTimer ran session.poll() this tick — the two guards above are the same pair
-    // already been handled and g_project already matches. This still catches it when
+    // that gate that poll.
-    // poll() was itself skipped this tick (modeApplyInProgress/drain-in-progress
+    ReaProject* project = proj ? proj : EnumProjects(-1, nullptr, 0);
-    // deferral) and the two go stale together. Both arms below mean the same thing —
+    if (!ValidatePtr2(nullptr, project, "ReaProject*")) project = nullptr;
-    // no active project at all, or a project this queue no longer belongs to — so drop
+    if (!g_queue.beginTick(project)) return;
-    // the queue rather than write another project's chains; returning to it re-derives.
-    // The clock goes too — a hold measured in a project we stopped watching is not
-    // evidence about the one we come back to.
-    ReaProject* const project = proj ? proj : EnumProjects(-1, nullptr, 0);
-    if (!ValidatePtr2(nullptr, project, "ReaProject*") || (g_project && g_project != project)) {
-        goIdle();
-        g_queue.forgetHolds();
-        return;
-    }
 
     // DERIVED, never persisted: recomputed from view state every time the queue was
     // invalidated, so a REAPER quit mid-drain simply re-derives and finishes on reopen.
     if (g_queue.stale()) {
-        g_project = project;
         const ViewModeModel& model = session.view();
         g_queue.adopt(makeQueue(
             view::desiredFxState(model, model.activeModeId(), liveTrackGuids(project))));
@@ -229,7 +209,7 @@ void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj) {
     ReclaimHost host{project, &session};
     const ReclaimRunResult run = g_queue.run(host);
     reportDrops(project, run.drops);
-    if (!run.aborted && run.settled) goIdle();
+    if (!run.aborted && run.settled) g_queue.goIdle();
 }
 
 }  // namespace reasampler

src/shell/view/fx_reclaim.h

@@ -137,8 +137,14 @@ struct TrackFxDrops {
 // True when a drop set is worth printing: any change of owner or of the set itself.
 // The one-per-changed-set memo this gates is view_park's `shouldReport`/
 // `reportExposedTracks`, which states the reason; the shell holds the memo.
+//
+// A tick that lost nothing is never worth printing AND must leave the memo alone — it
+// is not evidence about the last set that DID lose something, and letting it overwrite
+// the memo re-arms the identical report for the next re-derivation to print again.
+// Answering false is what holds the memo back, so this is the one gate for both.
 inline bool shouldReportDrops(bool sameOwnerAsLast, const std::vector<TrackFxDrops>& last,
                               const std::vector<TrackFxDrops>& now) {
+    if (now.empty()) return false;
     return !(sameOwnerAsLast && last == now);
 }
 
@@ -256,18 +262,64 @@ struct ReclaimRunResult {
     std::vector<TrackFxDrops> drops;
 };
 
-// Owns the derived queue and the debounce clock across a tick that can be re-entered:
+// Owns the derived queue, the debounce clock, the project both were derived against, and
-// TrackFX_SetOffline is ASSUMED to pump the message loop, so an invalidation can arrive
+// the timer's wakeup — together, because it is their interaction that decides whether a
-// with a drain half-way through a track.
+// tick may happen at all. A `ReaProject*` here is COMPARE-ONLY and never dereferenced, so
+// the whole state machine is exercisable without REAPER (tests/test_fx_reclaim.cpp).
 //
-// The two halves of that safety are structural, not guarded. `invalidate()` bumps a
+// The queue must also survive being re-entered: TrackFX_SetOffline is ASSUMED to pump the
-// counter and touches NOTHING else, so there is nothing for it to clear under a drain.
+// message loop, so an invalidation can arrive with a drain half-way through a track. The
-// `run()` moves the queue and the clock into locals for the drain's whole duration, so
+// two halves of that safety are structural, not guarded. `invalidate()` bumps a counter
-// the object holds nothing the drain is reading — and re-checks the counter before it
+// and raises the wakeup and touches NOTHING else, so there is nothing for it to clear
-// commits either the surviving queue or a single release.
+// under a drain. `run()` moves the queue and the clock into locals for the drain's whole
+// duration, so the object holds nothing the drain is reading — and re-checks the counter
+// before it commits either the surviving queue or a single release.
 class FxReclaimQueue {
 public:
-    void invalidate() { ++generation_; }
+    void invalidate() {
+        ++generation_;
+        pending_ = true;
+    }
+
+    // The ONE test the timer makes per tick. False means settled: nothing to converge
+    // until the next invalidation.
+    bool pending() const { return pending_; }
+
+    // The project the current queue and holds were derived against, or nullptr when idle.
+    ReaProject* project() const { return project_; }
+
+    // The tick's entry transition, and the ONE place the pin moves. `project` is the
+    // validated active project, or nullptr when there is none; false means the caller
+    // must return without touching REAPER.
+    //
+    // A project that does not match the pin is a TAB SWITCH — the ordinary path, not a
+    // corner. Neither the derived work nor the holds are evidence about the project we
+    // arrived in, so both go, and the tick proceeds to re-derive against the new one. The
+    // WAKEUP STANDS: clearing it here (an earlier goIdle in this branch did) stranded the
+    // incoming project's whole reclaim, because the timer gates every tick on it and
+    // nothing re-raises it short of a mode switch or a membership edit there. Only the
+    // no-project arm may clear it — nothing to converge, and nothing to come back to.
+    bool beginTick(ReaProject* project) {
+        if (!project) {
+            goIdle();
+            clock_.forget();
+            return false;
+        }
+        if (project_ && project_ != project) {
+            discard();
+            clock_.forget();
+        }
+        project_ = project;
+        return true;
+    }
+
+    // Settled: drop the queue and the pin and stop the timer's ticks. The debounce clock
+    // deliberately survives — it is keyed on the track, not on the queue.
+    void goIdle() {
+        discard();
+        project_ = nullptr;
+        pending_ = false;
+    }
 
     // No queue at all, or one derived before the last invalidation.
     bool stale() const { return !derived_ || derivedAt_ != generation_; }
@@ -283,8 +335,6 @@ public:
         derived_ = false;
     }
 
-    void forgetHolds() { clock_.forget(); }
-
     const std::vector<ReclaimItem>& items() const { return items_; }
 
     template <class Host>
@@ -327,19 +377,22 @@ public:
 private:
     std::vector<ReclaimItem> items_;
     ParkClock                clock_;
+    ReaProject*              project_    = nullptr;
     unsigned long long       generation_ = 0;
     unsigned long long       derivedAt_  = 0;
     bool                     derived_    = false;
+    bool                     pending_    = false;
 };
 
 // --- shell entry points --------------------------------------------------------
 
 // The ONE test the timer makes per tick, mirroring the realtime capture's single
 // pointer test. False means the queue is settled and there is nothing to converge.
-extern bool g_fxReclaimPending;
+bool fxReclaimPending();
 
-// Marks the derived queue for re-derivation by the NEXT tick; discards nothing here,
+// Raises the wakeup and marks the derived queue for re-derivation by the NEXT tick;
-// because this is reachable from inside a drain. Called from applyMode and nothing else
+// discards nothing here, because this is reachable from inside a drain. Called from
+// applyMode and nothing else
 // — every membership, snapshot and active-mode mutation reapplies through it — on the
 // refusal path as well as the success one (see view.cpp for why the refusal is the
 // load-bearing half). The debounce clock deliberately survives: it is keyed on the
@@ -351,11 +404,12 @@ void invalidateFxReclaim();
 bool fxReclaimDrainInProgress();
 
 // Drains one tick against `proj` (nullptr = current project), re-deriving the queue from
-// the session's view model first if it was invalidated. Returns without touching the
+// the session's view model first if it was invalidated or if the active project moved
-// project while an applyMode is on the stack, while already draining, or when the active
+// (FxReclaimQueue::beginTick). Returns without touching the project while an applyMode is
-// project is not the one the queue was derived against. Takes the SESSION rather than the
+// on the stack, while already draining, or when there is no valid active project at all.
-// model alone because completing an online spends the park record's FX half, and that
+// Takes the SESSION rather than the model alone because completing an online spends the
-// spend has to reach the persisted view state — see FxReclaimQueue::run.
+// park record's FX half, and that spend has to reach the persisted view state — see
+// FxReclaimQueue::run.
 void fxReclaimTick(ReaSamplerSession& session, ReaProject* proj);
 
 }  // namespace reasampler

tests/test_fx_reclaim.cpp

@@ -4,10 +4,12 @@
 // yield, the queue being a cache of intent that a model change discards, the user's own
 // offline plugins surviving an online, the release reaching the persisted model, and —
 // via a host that invalidates from inside a write — that a re-entered invalidation
-// reaches nothing the drain is using.
+// reaches nothing the drain is using. The queue owns the project pin and the timer's
+// wakeup for the same reason: a compare-only pointer makes the tab switch assertable.
 
 #include "../src/shell/view/fx_reclaim.h"
 
+#include <cstdint>
 #include <cstdio>
 #include <map>
 #include <optional>
@@ -176,6 +178,11 @@ FxReclaimQueue queueOf(const ViewModeModel& vm, const std::set<std::string>& liv
     return q;
 }
 
+// Two project handles. The queue treats a project pointer as COMPARE-ONLY and never
+// dereferences it, so a fabricated address is a faithful stand-in for REAPER's.
+ReaProject* const kProjA = reinterpret_cast<ReaProject*>(static_cast<std::uintptr_t>(0x1));
+ReaProject* const kProjB = reinterpret_cast<ReaProject*>(static_cast<std::uintptr_t>(0x2));
+
 }  // namespace
 
 // -- 1. debounce ---------------------------------------------------------------
@@ -633,6 +640,138 @@ static void testAnOfflineTickReportsNoDrops() {
     CHECK(r.drops.empty());
 }
 
+// -- 8. the pin and the wakeup ------------------------------------------------------
+
+static void testATabSwitchRetargetsTheQueueAndLeavesTheWakeupStanding() {
+    // Project A, one parked leaf owed offline work, derived against A and pinned to it.
+    ViewModeModel a = parkedDesignLeaf({"{FX-A}"});
+    FakeHost hostA;
+    hostA.chains["{T}"] = onlineChain({"{FX-A}"});
+
+    FxReclaimQueue q;
+    q.invalidate();  // what applyMode does on its way out of the switch that parked {T}
+    CHECK(q.pending());
+    CHECK(q.beginTick(kProjA));
+    CHECK(q.project() == kProjA);
+    q.adopt(derive(a, {"{T}"}));
+    CHECK(!q.run(hostA).settled);  // observes the hold at t=0; still owed
+    CHECK(!q.stale());
+
+    // The tab switch. poll()'s Load reapply invalidates on the way in, and NOTHING there
+    // moves the pin, so the tick arrives holding A's work with B active.
+    q.invalidate();
+    CHECK(q.beginTick(kProjB));  // the tick proceeds — it does not bail on the mismatch
+    CHECK(q.pending());          // and the timer will still call it: the strand's one bit
+    CHECK(q.project() == kProjB);
+    CHECK(q.stale());            // A's work is gone, B's is owed
+    CHECK(q.items().empty());
+
+    // B's own parked leaf converges on this very tick and every one after it.
+    ViewModeModel b = parkedDesignLeaves({{"{T2}", {"{FX-B}"}}});
+    FakeHost hostB;
+    hostB.chains["{T2}"] = onlineChain({"{FX-B}"});
+    q.adopt(derive(b, {"{T2}"}));
+    CHECK(!q.run(hostB).settled);  // B's own hold starts here
+    hostB.clock += kParkHoldMs;
+    const ReclaimRunResult done = q.run(hostB);
+    CHECK(done.settled);
+    CHECK(hostB.writeLog == std::vector<std::string>{"{T2}:0=off"});
+    CHECK(hostB.offlineAt("{T2}", 0));
+
+    // ...and A's chain was never touched from B: the queue was dropped, not run.
+    CHECK(hostA.writeLog.empty());
+
+    // Only now, settled, does the shell stop the ticks.
+    q.goIdle();
+    CHECK(!q.pending());
+    CHECK(q.project() == nullptr);
+}
+
+static void testGoingIdleKeepsTheParkHoldWhileARetargetDropsIt() {
+    // A hold is evidence about a track in ONE project, so a tab switch drops it. An
+    // ordinary settle does not: restarting every 3s hold on the next unrelated membership
+    // edit would defer an unchanged track's reclaim indefinitely.
+    ViewModeModel vm = parkedDesignLeaf({"{FX-A}"});
+    FakeHost host;
+    host.chains["{T}"] = onlineChain({"{FX-A}"});
+
+    FxReclaimQueue q;
+    CHECK(q.beginTick(kProjA));
+    q.adopt(derive(vm, {"{T}"}));
+    q.run(host);  // hold starts at t=0
+    host.clock = kParkHoldMs;
+    CHECK(q.run(host).settled);
+    CHECK(host.writeLog.size() == 1);
+
+    // Settled: the queue and the pin go, the hold stays. A membership edit elsewhere
+    // re-derives the SAME offline item, which is matured on arrival — one tick, no
+    // second 3s wait, and nothing to write because the chain is already there.
+    q.goIdle();
+    q.invalidate();
+    CHECK(q.beginTick(kProjA));
+    q.adopt(derive(vm, {"{T}"}));
+    const ReclaimRunResult resumed = q.run(host);
+    CHECK(resumed.settled);
+    CHECK(resumed.writes == 0);
+    CHECK(host.writeLog.size() == 1);
+
+    // The mirror: away to B and back to A, and the same item must wait a fresh hold.
+    q.invalidate();
+    CHECK(q.beginTick(kProjB));
+    CHECK(q.beginTick(kProjA));
+    q.adopt(derive(vm, {"{T}"}));
+    const double back = host.clock;
+    CHECK(!q.run(host).settled);  // the hold restarts here, at `back`
+    host.clock = back + kParkHoldMs - 1.0;
+    CHECK(!q.run(host).settled);
+    host.clock = back + kParkHoldMs;
+    CHECK(q.run(host).settled);
+}
+
+static void testOnlyANoProjectTickClearsTheWakeup() {
+    ViewModeModel vm = parkedDesignLeaf({"{FX-A}"});
+    FakeHost host;
+    host.chains["{T}"] = onlineChain({"{FX-A}"});
+
+    FxReclaimQueue q;
+    q.invalidate();
+    CHECK(q.beginTick(kProjA));
+    q.adopt(derive(vm, {"{T}"}));
+    q.run(host);
+    host.clock += kParkHoldMs;
+
+    // An abort leaves the wakeup up: the work is re-owed, and the shell's settle test
+    // never runs on this path.
+    host.reentrant = &q;
+    host.invalidateAfterWrite = 1;
+    const ReclaimRunResult r = q.run(host);
+    CHECK(r.aborted);
+    CHECK(q.pending());
+
+    // No active project at all — the one arm with nothing to converge and nothing to
+    // come back to. Everything goes, wakeup included.
+    CHECK(!q.beginTick(nullptr));
+    CHECK(!q.pending());
+    CHECK(q.project() == nullptr);
+    CHECK(q.stale());
+    CHECK(q.items().empty());
+}
+
+static void testATickThatLostNothingNeitherPrintsNorDisturbsTheDropMemo() {
+    const std::vector<TrackFxDrops> none;
+    const std::vector<TrackFxDrops> lost{TrackFxDrops{"{T}", FxRestoreDrops{1, 0, 0}}};
+
+    CHECK(shouldReportDrops(/*sameOwnerAsLast=*/false, none, lost));  // first sighting
+    CHECK(!shouldReportDrops(/*sameOwnerAsLast=*/true, lost, lost));  // the re-derivation
+
+    // The regression: every ordinary tick carries no drops at all. Treating that as a
+    // change would overwrite the memo and re-arm the identical report for the next
+    // re-derivation — the shell holds the memo back on exactly this answer.
+    CHECK(!shouldReportDrops(/*sameOwnerAsLast=*/true, lost, none));
+    CHECK(!shouldReportDrops(/*sameOwnerAsLast=*/false, lost, none));
+    CHECK(!shouldReportDrops(/*sameOwnerAsLast=*/true, lost, lost));
+}
+
 int main() {
     testFastFlipMovesZeroPluginsButAHeldParkStillReclaims();
 
@@ -657,6 +796,11 @@ int main() {
     testADropIsReportedOnceAcrossRepeatedRederivations();
     testAnOfflineTickReportsNoDrops();
 
+    testATabSwitchRetargetsTheQueueAndLeavesTheWakeupStanding();
+    testGoingIdleKeepsTheParkHoldWhileARetargetDropsIt();
+    testOnlyANoProjectTickClearsTheWakeup();
+    testATickThatLostNothingNeitherPrintsNorDisturbsTheDropMemo();
+
     if (g_fail == 0) std::printf("All tests passed.\n");
     return g_fail ? 1 : 0;
 }