Reclaim drain owns its work: invalidation only bumps a generation, and a generation that moved aborts the tick's commit and its release.

src/shell/view/fx_reclaim.cpp

@@ -32,14 +32,12 @@ namespace {
 // COMPARE-ONLY and never dereferenced beyond the ValidatePtr2 the tick already makes —
 // a tab switch has to be detectable without following a pointer that may name a closed
 // project.
-std::vector<ReclaimItem> g_items;
-ReaProject*              g_project = nullptr;
-ParkClock                g_clock;
+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 walk the
-// same queue the outer one holds a reference into, re-running the very item that outer
-// step is still writing.
+// the timer and land back here mid-drain. Fail closed: a re-entered drain would derive a
+// second queue for the same project and race the outer one's writes track for track.
 bool g_draining = false;
 struct DrainLatch {
     DrainLatch() { g_draining = true; }
@@ -112,13 +110,15 @@ struct ReclaimHost {
         return true;
     }
 
+    // Reached only after the drain has unwound and the queue has proved its generation
+    // did not move, so `model` is still the object the queue was derived against.
     void releaseFx(const std::string& guid) { model->releaseSnapshotFx(guid); }
 };
 
 // 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_items.clear();
+    g_queue.discard();
     g_project = nullptr;
     g_fxReclaimPending = false;
 }
@@ -128,10 +128,15 @@ void goIdle() {
 bool g_fxReclaimPending = false;
 
 void invalidateFxReclaim() {
-    goIdle();
+    // 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; }
+
 void fxReclaimTick(ViewModeModel& model, ReaProject* proj) {
     if (g_draining) return;
     // The model is mid-rewrite and its records are being consumed out from under any
@@ -148,20 +153,21 @@ void fxReclaimTick(ViewModeModel& model, ReaProject* proj) {
     ReaProject* const project = proj ? proj : EnumProjects(-1, nullptr, 0);
     if (!ValidatePtr2(nullptr, project, "ReaProject*") || (g_project && g_project != project)) {
         goIdle();
-        g_clock.forget();
+        g_queue.forgetHolds();
         return;
     }
 
     // DERIVED, never persisted: recomputed from view state every time the queue was
-    // discarded, so a REAPER quit mid-drain simply re-derives and finishes on reopen.
-    if (!g_project) {
+    // invalidated, so a REAPER quit mid-drain simply re-derives and finishes on reopen.
+    if (g_queue.stale()) {
         g_project = project;
-        g_items = makeQueue(
-            view::desiredFxState(model, model.activeModeId(), liveTrackGuids(project)));
+        g_queue.adopt(makeQueue(
+            view::desiredFxState(model, model.activeModeId(), liveTrackGuids(project))));
     }
 
     ReclaimHost host{project, &model};
-    if (drainTick(g_items, g_clock, host).settled) goIdle();
+    const ReclaimRunResult run = g_queue.run(host);
+    if (!run.aborted && run.settled) goIdle();
 }
 
 }  // namespace reasampler

src/shell/view/fx_reclaim.h

@@ -92,6 +92,9 @@ public:
 
     void forget() { since_.clear(); }
 
+    // The owner swaps the clock out for the duration of a drain — see FxReclaimQueue.
+    void swap(ParkClock& other) { since_.swap(other.since_); }
+
 private:
     std::map<std::string, double> since_;
 };
@@ -119,6 +122,12 @@ inline int nextEligible(const std::vector<ReclaimItem>& items, const ParkClock&
 struct ReclaimTickResult {
     int  writes  = 0;
     bool settled = false;
+
+    // Tracks whose park record the tick drove fully online, so the record's FX half is
+    // spent. REPORTED, not applied: releasing is a model write and the model is exactly
+    // what a re-entered load can replace mid-drain, so only the owner may commit it —
+    // see FxReclaimQueue::run.
+    std::vector<std::string> released;
 };
 
 // One tick of the convergence loop. Templated on `Host` so the ordering, budget and
@@ -131,7 +140,14 @@ struct ReclaimTickResult {
 //   bool   writeOffline(guid, fxIndex, offline)          compare-before-write inside its
 //                                                        own discarded undo block;
 //                                                        true iff it actually wrote
-//   void   releaseFx(guid)                               release the record's FX half
+//   void   releaseFx(guid)                               release the record's FX half;
+//                                                        called by FxReclaimQueue::run
+//                                                        after the drain has unwound,
+//                                                        never from inside the drain
+//
+// `items` and `clock` must be the caller's OWN storage, not shared state a re-entered
+// call could reach: a write pumps the message loop. FxReclaimQueue::run is what makes
+// that true for the live loop.
 template <class Host>
 ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, Host& host) {
     ReclaimTickResult result;
@@ -166,11 +182,12 @@ ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, H
             for (const FxOfflineWrite& w : resolved.writes)
                 if (host.writeOffline(item.work.trackGuid, w.fxIndex, w.offline)) ++result.writes;
 
-            // The record's FX half is released HERE and only here: the chain has now
-            // been driven to every state the record captured, so the record is spent.
-            // Releasing it any earlier destroys the only account of which plugins the
-            // USER had offline before the park, and the next online blanket-enables them.
-            if (item.work.kind == view::FxWorkKind::Online) host.releaseFx(item.work.trackGuid);
+            // The record is spent only once the chain has been driven to every state it
+            // captured — releasing any earlier destroys the only account of which plugins
+            // the USER had offline before the park, and the next online blanket-enables
+            // them. Reported rather than applied; see ReclaimTickResult::released.
+            if (item.work.kind == view::FxWorkKind::Online)
+                result.released.push_back(item.work.trackGuid);
         }
         item.done = true;  // both arms: a step is atomic, so no item is half-moved
 
@@ -181,23 +198,104 @@ ReclaimTickResult drainTick(std::vector<ReclaimItem>& items, ParkClock& clock, H
     return result;
 }
 
+// --- the queue's owner ----------------------------------------------------------
+
+struct ReclaimRunResult {
+    int  writes  = 0;
+    bool settled = false;
+
+    // Something invalidated the queue while the drain was on the stack. The work and
+    // the releases are DISCARDED, `settled` is never true, and the caller must not go
+    // idle — the next tick re-derives against whatever changed.
+    bool aborted = false;
+};
+
+// Owns the derived queue and the debounce clock across a tick that can be re-entered:
+// TrackFX_SetOffline is ASSUMED to pump the message loop, so an invalidation can arrive
+// with a drain half-way through a track.
+//
+// The two halves of that safety are structural, not guarded. `invalidate()` bumps a
+// counter and touches NOTHING else, so there is nothing for it to clear 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_; }
+
+    // No queue at all, or one derived before the last invalidation.
+    bool stale() const { return !derived_ || derivedAt_ != generation_; }
+
+    void adopt(std::vector<ReclaimItem> items) {
+        items_    = std::move(items);
+        derived_  = true;
+        derivedAt_ = generation_;
+    }
+
+    void discard() {
+        items_.clear();
+        derived_ = false;
+    }
+
+    void forgetHolds() { clock_.forget(); }
+
+    const std::vector<ReclaimItem>& items() const { return items_; }
+
+    template <class Host>
+    ReclaimRunResult run(Host& host) {
+        const unsigned long long entered = generation_;
+
+        std::vector<ReclaimItem> work;
+        ParkClock                clock;
+        work.swap(items_);
+        clock.swap(clock_);
+
+        const ReclaimTickResult tick = drainTick(work, clock, host);
+
+        // The hold is keyed on the track and outlives any re-derivation, so it comes
+        // back whether or not the drain's own work survives.
+        clock_.swap(clock);
+
+        if (generation_ != entered) return ReclaimRunResult{tick.writes, false, true};
+
+        items_.swap(work);
+        // A release dropped by an abort is re-owed, not lost: the next derivation plans
+        // the same Online item, its writes all find the chain already there, and the
+        // release fires then.
+        for (const std::string& guid : tick.released) host.releaseFx(guid);
+        return ReclaimRunResult{tick.writes, tick.settled, false};
+    }
+
+private:
+    std::vector<ReclaimItem> items_;
+    ParkClock                clock_;
+    unsigned long long       generation_ = 0;
+    unsigned long long       derivedAt_  = 0;
+    bool                     derived_    = 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;
 
-// Discards the derived queue. Called from every path that can change what the FX chains
-// SHOULD be — which is applyMode's success path and nothing else, because every
-// membership, snapshot and active-mode mutation reapplies through it. The debounce clock
-// deliberately survives: it is keyed on the track, not on the queue.
+// Marks the derived queue for re-derivation by the NEXT tick; 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
+// track, not on the queue.
 void invalidateFxReclaim();
 
+// True while a drain is on the stack. Exposed so the timer's other work can stand off
+// the model the drain is holding, the same way it stands off an applyMode.
+bool fxReclaimDrainInProgress();
+
 // Drains one tick against `proj` (nullptr = current project), re-deriving the queue from
 // `model` first if it was invalidated. Returns without touching the project while an
-// applyMode is on the stack, while already draining (TrackFX_SetOffline is assumed to
-// pump the message loop), or when the active project is not the one the queue was
-// derived against.
+// applyMode is on the stack, while already draining, or when the active project is not
+// the one the queue was derived against.
 void fxReclaimTick(ViewModeModel& model, ReaProject* proj);
 
 }  // namespace reasampler

src/shell/view/view.cpp

@@ -62,7 +62,9 @@ bool transportBlocksModeSwitch(ReaProject* proj) {
 
 bool modeApplyInProgress() { return g_applying; }
 
-bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj) {
+namespace {
+
+bool applyModeImpl(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj) {
     if (g_applying) return false;  // see ApplyLatch; same fail-closed shape as the guards below
     ApplyLatch latch;
 
@@ -203,12 +205,26 @@ bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject
     // an undo rolling the model back underneath it.
     noteAppliedState(project, model);
 
-    // Every membership, snapshot and active-mode mutation in the system reaches the
-    // project through this call, so this is the whole invalidation surface: what the FX
-    // chains SHOULD be has just changed, and the reclaimer's queue is a cache of that
-    // intent, never state.
-    invalidateFxReclaim();
     return true;
 }
 
+} // namespace
+
+bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj) {
+    const bool applied = applyModeImpl(model, targetModeId, proj);
+
+    // Every membership, snapshot and active-mode mutation in the system reaches the
+    // project through this call, so this is the whole invalidation surface: the
+    // reclaimer's queue is a cache of what the FX chains SHOULD be, never state.
+    //
+    // Dropped on a REFUSAL too, and that is the load-bearing half. Callers mutate the
+    // model FIRST and reapply to make it visible (design_view_actions' tag/untag/
+    // show-both, render_in_place, the load tick), and every one of them discards this
+    // return — so a refusal is exactly the case where the mutation stands and nothing
+    // else would drop a queue that now converges toward the mode the user just left.
+    // The cost when the refusal followed no mutation is one re-derivation on the timer.
+    invalidateFxReclaim();
+    return applied;
+}
+
 } // namespace reasampler

src/shell/view/view.h

@@ -59,6 +59,9 @@ bool modeApplyInProgress();
 // (modeApplyInProgress), or if the project does not validate. A reapply (target ==
 // active) is never gated and never touches solo. `proj` == nullptr means the current
 // project, resolved ONCE and used for every call this apply makes.
+//
+// Either way — applied or refused — this drops the FX reclaimer's derived queue, which
+// is why callers may safely discard the return after mutating the model.
 bool applyMode(ViewModeModel& model, const std::string& targetModeId, ReaProject* proj);
 
 } // namespace reasampler