Skip to content

Fix PortRegistration race causing heap corruption on plugin removal#96

Draft
sastraxi wants to merge 5 commits into
mod-audio:master-dgfrom
sastraxi:fix/effect-drain-midi
Draft

Fix PortRegistration race causing heap corruption on plugin removal#96
sastraxi wants to merge 5 commits into
mod-audio:master-dgfrom
sastraxi:fix/effect-drain-midi

Conversation

@sastraxi

@sastraxi sastraxi commented Jun 8, 2026

Copy link
Copy Markdown

Fixes a race between PortRegistration() and effects_remove() that causes heap corruption and SIGSEGV when switching pedalboards while an rtmidi device is open.

Fixes #94

Author checklist

  • verify fix on-device
  • stress test

falkTX added 4 commits May 17, 2026 16:14
Signed-off-by: falkTX <falktx@falktx.com>
Signed-off-by: falkTX <falktx@falktx.com>
Signed-off-by: falkTX <falktx@falktx.com>
Signed-off-by: falkTX <falktx@falktx.com>
@sastraxi sastraxi force-pushed the fix/effect-drain-midi branch from 03f9994 to 120fe86 Compare June 8, 2026 04:10
@sastraxi sastraxi marked this pull request as draft June 8, 2026 04:11
PortRegistration() enqueues POSTPONED_JACK_MIDI_CONNECT events on a
JACK callback thread with no teardown guard. effects_remove() drains
g_rtsafe_list in effects_remove_inner_pre(), then frees all plugin
clients via lilv_instance_free()/dlclose()/jack_client_close(). Events
arriving between the drain and those closes race against freed memory,
corrupting the heap and causing SIGSEGV on the next dlclose().

Add a second splice-and-free drain immediately after the teardown loop
in both effects_remove() and effects_remove_multi(), using the same
pattern already present in effects_remove_inner_pre(). Closes mod-audio#94.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@sastraxi sastraxi force-pushed the fix/effect-drain-midi branch 3 times, most recently from 120fe86 to 9732d45 Compare June 14, 2026 16:41
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.

2 participants