mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-08-11 03:09:56 +00:00
fix(audio): promote in-process VST3 to sandbox on editor-open (Windows crash) (#54)
Windows-only crash fix: opening an in-process VST3 editor faults via WndProc on the background message thread. OpenPluginEditor now promotes the slot to the out-of-process sandbox (state transferred via get/setStateInformation) via the new SignalChain::replaceProcessor, and opens the editor there. Review hardening (multi-angle + Codex): state capture runs under the audio lock + SEH guard (SignalChain::captureVstStateForPromotion) so it can't race processBlock or fault the app; the transient sandbox pin is undone on promotion failure (isCrashedPlugin/removeCrashedPlugin) so a healthy plugin isn't stranded; replaceProcessor stages type/name/path for correct blocklist attribution; shared prepareForPlayback helper. CI green incl. addon (windows-latest). Editor-open crash repro on a real Windows host still recommended as follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
850d0926c7
commit
27e8f56ad8
@@ -384,6 +384,15 @@ void SignalChain::queueMidiMessage(int targetSlotId, const juce::MidiMessage& ms
|
||||
// If queue full, message silently dropped (acceptable for PC messages)
|
||||
}
|
||||
|
||||
// Shared prepare sequence for a processor entering the live chain — used by
|
||||
// addProcessor and replaceProcessor so the channel config and prepare ordering
|
||||
// stay identical between them. Call from inside invokePlugin's fault guard.
|
||||
static void prepareForPlayback(juce::AudioProcessor& p, double sampleRate, int blockSize)
|
||||
{
|
||||
p.setPlayConfigDetails(2, 2, sampleRate, blockSize);
|
||||
p.prepareToPlay(sampleRate, blockSize);
|
||||
}
|
||||
|
||||
int SignalChain::addProcessor(std::unique_ptr<juce::AudioProcessor> processor,
|
||||
ProcessorSlot::Type type,
|
||||
const juce::String& name,
|
||||
@@ -403,8 +412,7 @@ int SignalChain::addProcessor(std::unique_ptr<juce::AudioProcessor> processor,
|
||||
// slot is dropped, rather than taking the app down.
|
||||
invokePlugin(*slot, [&](juce::AudioProcessor& p)
|
||||
{
|
||||
p.setPlayConfigDetails(2, 2, currentSampleRate, currentBlockSize);
|
||||
p.prepareToPlay(currentSampleRate, currentBlockSize);
|
||||
prepareForPlayback(p, currentSampleRate, currentBlockSize);
|
||||
});
|
||||
if (! slot->processor) return -1;
|
||||
|
||||
@@ -421,6 +429,91 @@ void SignalChain::removeProcessor(int slotId)
|
||||
if (idx >= 0) slots.remove(idx);
|
||||
}
|
||||
|
||||
bool SignalChain::replaceProcessor(int slotId, std::unique_ptr<juce::AudioProcessor> processor)
|
||||
{
|
||||
if (!processor) return false;
|
||||
|
||||
// Copy the target slot's identity (type/name/path) onto the staging slot
|
||||
// BEFORE preparing, so if the incoming processor faults during prepareToPlay
|
||||
// invokePlugin's catch blocklists the RIGHT plugin path (addProcessor sets
|
||||
// these before its own prepare for the same reason). Without this the staging
|
||||
// path is empty and the fault is recorded against "".
|
||||
ProcessorSlot staging;
|
||||
{
|
||||
const juce::ScopedLock sl(lock);
|
||||
const int idx = findSlotIndex(slotId);
|
||||
if (idx < 0) return false; // nothing to replace
|
||||
staging.type = slots[idx]->type;
|
||||
staging.name = slots[idx]->name;
|
||||
staging.path = slots[idx]->path;
|
||||
}
|
||||
|
||||
// Prepare the incoming processor before it goes live, exactly as addProcessor
|
||||
// does — under invokePlugin's SEH/signal guard so a fault in prepareToPlay is
|
||||
// contained (the processor is dropped) rather than taking the app down.
|
||||
staging.processor = std::move(processor);
|
||||
invokePlugin(staging, [&](juce::AudioProcessor& p)
|
||||
{
|
||||
prepareForPlayback(p, currentSampleRate, currentBlockSize);
|
||||
});
|
||||
if (! staging.processor) return false; // faulted during prepare → leave the slot as-is
|
||||
|
||||
std::unique_ptr<juce::AudioProcessor> old;
|
||||
{
|
||||
const juce::ScopedLock sl(lock);
|
||||
const int idx = findSlotIndex(slotId);
|
||||
if (idx < 0) return false; // slot was removed underneath us
|
||||
auto* slot = slots[idx];
|
||||
old = std::move(slot->processor);
|
||||
slot->processor = std::move(staging.processor);
|
||||
}
|
||||
// Tear the old processor down OUTSIDE the audio lock: releaseResources() (and
|
||||
// a VST3 destructor) can block, and must never stall process() on it.
|
||||
if (old)
|
||||
{
|
||||
old->releaseResources();
|
||||
old.reset();
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
bool SignalChain::captureVstStateForPromotion(int slotId, juce::MemoryBlock& state)
|
||||
{
|
||||
// Hold the audio lock across the whole check+snapshot: hasEditor() and
|
||||
// getStateInformation() are plugin calls on a LIVE processor, and process()
|
||||
// runs processBlock on that same instance under this lock (ScopedTryLock, so
|
||||
// it simply drops a block here rather than deadlocking). Doing the snapshot
|
||||
// off-lock would be a data race with the audio thread.
|
||||
const juce::ScopedLock sl(lock);
|
||||
const int idx = findSlotIndex(slotId);
|
||||
if (idx < 0) return false;
|
||||
auto* slot = slots[idx];
|
||||
if (! slot->processor) return false;
|
||||
if (slot->type != ProcessorSlot::Type::VST) return false;
|
||||
// Already out-of-process — nothing to promote (its editor path is safe).
|
||||
if (dynamic_cast<slopsmith::sandbox::SandboxedProcessor*>(slot->processor.get()) != nullptr)
|
||||
return false;
|
||||
|
||||
// Run the plugin calls under invokePlugin's SEH/signal guard: a plugin that
|
||||
// faults in hasEditor()/getStateInformation() is contained + blocklisted +
|
||||
// released (leaving slot->processor null), never fatal.
|
||||
bool promotable = false;
|
||||
invokePlugin(*slot, [&](juce::AudioProcessor& p)
|
||||
{
|
||||
if (! p.hasEditor()) return; // editor-less VST3 → nothing to open
|
||||
p.getStateInformation(state);
|
||||
promotable = true;
|
||||
});
|
||||
// slot->processor is null iff the guarded call faulted (invokePlugin released
|
||||
// it). Don't promote from a released slot; the empty `state` is discarded.
|
||||
if (! promotable || slot->processor == nullptr)
|
||||
{
|
||||
state.reset();
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
void SignalChain::moveProcessor(int fromIndex, int toIndex)
|
||||
{
|
||||
const juce::ScopedLock sl(lock);
|
||||
|
||||
Reference in New Issue
Block a user