fix: close race and UI gaps found in Link Audio review #2

Merged

peterswimm merged 1 commit from feat/per-node-link-audio-publish into main

2026-08-28 22:14:59 +00:00

peterswimm commented

2026-08-28 22:14:52 +00:00

Owner

Adversarial review of the previous commit found several real issues:

FilterGraph::setNodeLinkAudioPublish's enable path replaced a node's
tap slot without holding the graph's callback lock (unlike the
disable path), so registerNodeSink()'s internal dedup-by-erase could
free a NodeSinkGroup while the audio thread was still publishing
through it. Now takes the same lock and clears the old slot before
registering the new one, mirroring the disable path.
Audio Input/Audio Output could never actually get the LINK button -
the whole footer-button block is gated behind a pre-existing
pluginName exclusion list that skips those two node types, so half
of this feature's stated scope was unreachable from the UI despite
the FilterGraph/LinkAudioService plumbing being fully wired for it.
They now get the button in a small strip below the title label,
since they have none of the normal edit/mappings/bypass/delete chrome
to anchor it to.
determineSize() didn't reserve width for the 4th footer button, so
linkAudioButton could overlap bypassButton on narrow generic-shell
nodes (most ordinary third-party plugins).
The LINK button's (and the pre-existing edit button's) async popup
menu captured this by raw pointer; now uses Component::SafePointer
so a node deletion or patch reload while the menu is open is safe.
LinkAudioService::prepare()'s per-node sink resize used a flat size
instead of that sink's actual channel count, inconsistent with the
master-sink resize right above it.

Adversarial review of the previous commit found several real issues: - FilterGraph::setNodeLinkAudioPublish's enable path replaced a node's tap slot without holding the graph's callback lock (unlike the disable path), so registerNodeSink()'s internal dedup-by-erase could free a NodeSinkGroup while the audio thread was still publishing through it. Now takes the same lock and clears the old slot before registering the new one, mirroring the disable path. - Audio Input/Audio Output could never actually get the LINK button - the whole footer-button block is gated behind a pre-existing pluginName exclusion list that skips those two node types, so half of this feature's stated scope was unreachable from the UI despite the FilterGraph/LinkAudioService plumbing being fully wired for it. They now get the button in a small strip below the title label, since they have none of the normal edit/mappings/bypass/delete chrome to anchor it to. - determineSize() didn't reserve width for the 4th footer button, so linkAudioButton could overlap bypassButton on narrow generic-shell nodes (most ordinary third-party plugins). - The LINK button's (and the pre-existing edit button's) async popup menu captured `this` by raw pointer; now uses Component::SafePointer so a node deletion or patch reload while the menu is open is safe. - LinkAudioService::prepare()'s per-node sink resize used a flat size instead of that sink's actual channel count, inconsistent with the master-sink resize right above it.

peterswimm added 1 commit

2026-08-28 22:14:53 +00:00

fix: close race and UI gaps found in Link Audio review 78dd5e761f

Adversarial review of the previous commit found several real issues:

- FilterGraph::setNodeLinkAudioPublish's enable path replaced a node's
  tap slot without holding the graph's callback lock (unlike the
  disable path), so registerNodeSink()'s internal dedup-by-erase could
  free a NodeSinkGroup while the audio thread was still publishing
  through it. Now takes the same lock and clears the old slot before
  registering the new one, mirroring the disable path.
- Audio Input/Audio Output could never actually get the LINK button -
  the whole footer-button block is gated behind a pre-existing
  pluginName exclusion list that skips those two node types, so half
  of this feature's stated scope was unreachable from the UI despite
  the FilterGraph/LinkAudioService plumbing being fully wired for it.
  They now get the button in a small strip below the title label,
  since they have none of the normal edit/mappings/bypass/delete chrome
  to anchor it to.
- determineSize() didn't reserve width for the 4th footer button, so
  linkAudioButton could overlap bypassButton on narrow generic-shell
  nodes (most ordinary third-party plugins).
- The LINK button's (and the pre-existing edit button's) async popup
  menu captured `this` by raw pointer; now uses Component::SafePointer
  so a node deletion or patch reload while the menu is open is safe.
- LinkAudioService::prepare()'s per-node sink resize used a flat size
  instead of that sink's actual channel count, inconsistent with the
  master-sink resize right above it.

peterswimm merged commit 39a3ce3d45 into main

2026-08-28 22:14:59 +00:00

peterswimm deleted branch feat/per-node-link-audio-publish

2026-08-28 22:15:00 +00:00

peterswimm referenced this pull request from a commit

2026-08-28 22:15:01 +00:00

Merge pull request 'fix: close race and UI gaps found in Link Audio review' (#2) from feat/per-node-link-audio-publish into main