Skip to content

fix: crash when changing a plugin node's input port configuration - #1177

Open
ctruett wants to merge 1 commit into
kushview:mainfrom
ctruett:stale-connections
Open

fix: crash when changing a plugin node's input port configuration#1177
ctruett wants to merge 1 commit into
kushview:mainfrom
ctruett:stale-connections

Conversation

@ctruett

@ctruett ctruett commented Aug 31, 2026

Copy link
Copy Markdown

Summary

Fixes a crash (SIGSEGV) that occurs when applying a new input bus layout to a connected plugin node from the node's I/O configuration window.

GraphBuilder::createRenderingOpsForNode now filters connections through isConnectionLegal() before consuming them — the same invariant addConnection() enforces when arcs are created, and the same predicate removeIllegalConnections() uses for cleanup.

Root cause

Connections can outlive the port configuration they were created against. The crash sequence:

  1. The user changes a plugin node's input layout (e.g. 8 ins → mono). EngineService::changeBusesLayout applies the new buses via setBusesLayoutWithoutEnabling() and renumbers the node's ports (node.resetPorts()).
  2. The parent graph's connections array still holds arcs referencing the old port indices.
  3. The rendering sequence is rebuilt (prepareToRender()buildRenderingSequence()) before removeIllegalConnections() purges the stale arcs.
  4. A stale audio arc whose old dest-port index now lands on a renumbered Control port reaches BindParameterOp (src->getParameter (srcPort), node->getParameter (port)) (graphbuilder.cpp:657). Processor::getParameter() returns nullptr for non-Control ports, and BindParameterOp's constructor calls param1->addListener (this) unconditionally → null dereference.

Why the filter belongs in the builder: the window between port renumbering and removeIllegalConnections() exists in every reconfiguration path — the sync service flow, the async Processor::PortResetter path, and any future one. Guarding at the consumption site closes all of them. It also mirrors the implicit protection in JUCE's own AudioProcessorGraph builder, which iterates channels bounded by the current layout rather than raw connections.

Changes

  • src/engine/graphbuilder.cppcreateRenderingOpsForNode: skip arcs failing isConnectionLegal(). 4 lines + comment.
  • test/BusesLayoutTests.cpp — new BusesLayoutTests suite reproducing the exact changeBusesLayout sequence (8-input processor with 8 parameters shrunk to mono while connected); crashes on the old code, passes now.
  • test/CMakeLists.txt — CTest registration for the new suite.

Testing

  • New regression test BusesLayoutTests (red → green; SIGSEGV exit 139 before the fix).
  • Full suite: 50/50 pass (ctest), including rebase onto current main.

Notes

The service flow still rebuilds before purging stale arcs (removeIllegalConnections() runs after prepareToRender() in changeBusesLayout). That ordering is harmless with this guard in place, but reordering it would be reasonable follow-up hardening — kept out of this PR to stay minimal.

A connection can outlive the port configuration it was created
against: changing a plugin node's input buses renumbers its ports
while the old arcs still reference them, and the rendering sequence
is rebuilt before removeIllegalConnections() runs. GraphBuilder
consumed those stale arcs unconditionally; a stale audio arc whose
dest port index now lands on a Control port produced a
BindParameterOp with a null source parameter, crashing in its
constructor.

Filter arcs through isConnectionLegal() in createRenderingOpsForNode
- the same invariant addConnection() enforces when arcs are created.
This also hardens the async PortResetter path, which has the same
window between refreshPorts() and removeIllegalConnections().

Adds a regression test (BusesLayoutTests) reproducing the exact
EngineService::changeBusesLayout sequence with an 8-input plugin
shrunk to mono while connected.
@ctruett
ctruett marked this pull request as ready for review August 31, 2026 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant