From 500433994f9df298ee940e44f45645479547d2d0 Mon Sep 17 00:00:00 2001 From: Eric Bavu Date: Thu, 29 Oct 2026 10:12:39 +0100 Subject: [PATCH 1/2] coremidi: fix hang on shutdown and on quick restart Kill() cannot interrupt a thread blocked in CFRunLoopRun() or sem_wait(), so jackd never exited when stopped. Stop the threads cooperatively instead. Also ignore CoreMIDI notifications until the driver is started and while it is closing, so Restart() cannot run alongside Attach(). --- macosx/coremidi/JackCoreMidiDriver.h | 4 ++++ macosx/coremidi/JackCoreMidiDriver.mm | 29 ++++++++++++++++++++--- macosx/coremidi/JackCoreMidiOutputPort.h | 2 ++ macosx/coremidi/JackCoreMidiOutputPort.mm | 18 ++++++++++++-- 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/macosx/coremidi/JackCoreMidiDriver.h b/macosx/coremidi/JackCoreMidiDriver.h index 4ad6fda09..dc7af15ec 100644 --- a/macosx/coremidi/JackCoreMidiDriver.h +++ b/macosx/coremidi/JackCoreMidiDriver.h @@ -27,6 +27,7 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. #include "JackCoreMidiVirtualOutputPort.h" #include "JackMidiDriver.h" #include "JackThread.h" +#include namespace Jack { @@ -63,6 +64,9 @@ namespace Jack { void Restart(); JackThread fThread; /*! Thread to execute the Process function */ + CFRunLoopRef fRunLoop; // run loop of fThread + std::atomic fClosing; // set by Close(), makes fThread exit + std::atomic fStarted; // true once Start() is done public: diff --git a/macosx/coremidi/JackCoreMidiDriver.mm b/macosx/coremidi/JackCoreMidiDriver.mm index a39f9a5bf..53ba9d3b8 100644 --- a/macosx/coremidi/JackCoreMidiDriver.mm +++ b/macosx/coremidi/JackCoreMidiDriver.mm @@ -63,7 +63,7 @@ JackCoreMidiDriver::JackCoreMidiDriver(const char *name, const char *alias, JackLockedEngine *engine, JackSynchro *table): - JackMidiDriver(name, alias, engine, table),fThread(this) + JackMidiDriver(name, alias, engine, table),fThread(this),fRunLoop(NULL),fClosing(false),fStarted(false) { mach_timebase_info_data_t info; kern_return_t result = mach_timebase_info(&info); @@ -91,6 +91,8 @@ bool JackCoreMidiDriver::Init() { + // needed to stop the run loop in Close() + fRunLoop = CFRunLoopGetCurrent(); return OpenAux(); } @@ -340,7 +342,14 @@ new JackCoreMidiVirtualOutputPort(fAliasName, client_name, bool JackCoreMidiDriver::Execute() { - CFRunLoopRun(); + // CFRunLoopRun() cannot be cancelled, run in short slices and check fClosing + while (!fClosing) { + SInt32 result = CFRunLoopRunInMode(kCFRunLoopDefaultMode, 0.1, false); + if (result == kCFRunLoopRunFinished) { + // nothing attached to the run loop, do not spin + JackSleep(100000); + } + } return false; } @@ -451,7 +460,13 @@ new JackCoreMidiVirtualOutputPort(fAliasName, client_name, int JackCoreMidiDriver::Close() { - fThread.Kill(); + // Kill() cannot interrupt the run loop, stop it and wait for the thread + fClosing = true; + if (fRunLoop) { + CFRunLoopStop(fRunLoop); + } + fThread.Stop(); + fRunLoop = NULL; return CloseAux(); } @@ -552,6 +567,11 @@ new JackCoreMidiVirtualOutputPort(fAliasName, client_name, void JackCoreMidiDriver::HandleNotification(const MIDINotification *message) { + // ignore changes while closing, and while starting as a restart would race with Attach() + if (fClosing || !fStarted) { + return; + } + switch (message->messageID) { case kMIDIMsgObjectAdded: { @@ -598,6 +618,7 @@ new JackCoreMidiVirtualOutputPort(fAliasName, client_name, capture_latency = capture_latency_aux; playback_latency = playback_latency_aux; + fClosing = false; fThread.StartSync(); int count = 0; @@ -672,6 +693,7 @@ new JackCoreMidiVirtualOutputPort(fAliasName, client_name, jack_info("JackCoreMidiDriver::Start - Driver started."); + fStarted = true; return 0; stop_virtual_output_ports: @@ -711,6 +733,7 @@ new JackCoreMidiVirtualOutputPort(fAliasName, client_name, { int result = 0; + fStarted = false; JackMidiDriver::Stop(); jack_info("JackCoreMidiDriver::Stop - disabling physical input ports."); diff --git a/macosx/coremidi/JackCoreMidiOutputPort.h b/macosx/coremidi/JackCoreMidiOutputPort.h index a04d98b3c..2618ed704 100644 --- a/macosx/coremidi/JackCoreMidiOutputPort.h +++ b/macosx/coremidi/JackCoreMidiOutputPort.h @@ -20,6 +20,7 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. #ifndef __JackCoreMidiOutputPort__ #define __JackCoreMidiOutputPort__ +#include #include #include "JackCoreMidiPort.h" @@ -49,6 +50,7 @@ namespace Jack { JackThread *thread; JackMidiAsyncQueue *thread_queue; sem_t *thread_queue_semaphore; + std::atomic stopping; // set by Stop(), makes the thread exit protected: diff --git a/macosx/coremidi/JackCoreMidiOutputPort.mm b/macosx/coremidi/JackCoreMidiOutputPort.mm index f62ebaffb..1221cdf99 100644 --- a/macosx/coremidi/JackCoreMidiOutputPort.mm +++ b/macosx/coremidi/JackCoreMidiOutputPort.mm @@ -33,7 +33,7 @@ JackCoreMidiOutputPort::JackCoreMidiOutputPort(double time_ratio, size_t max_bytes, size_t max_messages): - JackCoreMidiPort(time_ratio) + JackCoreMidiPort(time_ratio), stopping(false) { read_queue = new JackMidiBufferReadQueue(); std::unique_ptr read_queue_ptr(read_queue); @@ -54,6 +54,8 @@ JackCoreMidiOutputPort::~JackCoreMidiOutputPort() { + // the thread must be gone before the semaphore is closed + Stop(); delete thread; sem_close(thread_queue_semaphore); sem_unlink(semaphore_name); @@ -67,10 +69,18 @@ jack_midi_event_t *event = 0; MIDIPacketList *packet_list = (MIDIPacketList *) packet_buffer; for (;;) { + // Stop() sets this before waking the semaphore + if (stopping) { + break; + } MIDIPacket *packet = MIDIPacketListInit(packet_list); assert(packet); if (! event) { event = GetCoreMidiEvent(true); + if (! event) { + // woken by Stop(), or by a wake-up left over from a previous Stop() + continue; + } } jack_midi_data_t *data = event->buffer; jack_nframes_t send_frame = event->time; @@ -227,6 +237,7 @@ { bool result = thread->GetStatus() != JackThread::kIdle; if (! result) { + stopping = false; result = ! thread->StartSync(); if (! result) { jack_error("JackCoreMidiOutputPort::Start - failed to start MIDI " @@ -241,7 +252,10 @@ { bool result = thread->GetStatus() == JackThread::kIdle; if (! result) { - result = ! thread->Kill(); + // Kill() does not make sem_wait() return, wake up the thread and wait for it + stopping = true; + sem_post(thread_queue_semaphore); + result = ! thread->Stop(); if (! result) { jack_error("JackCoreMidiOutputPort::Stop - failed to stop MIDI " "processing thread."); From 8c2b82650f86d4bbb1c8ad118e70428d8980ba6f Mon Sep 17 00:00:00 2001 From: Eric Bavu Date: Thu, 29 Oct 2026 10:12:39 +0100 Subject: [PATCH 2/2] tests: add a CoreMIDI shutdown test Start and stop jackd with the coremidi driver several times in a row. --- tests/jack_coremidi_shutdown_test.sh | 157 +++++++++++++++++++++++++++ 1 file changed, 157 insertions(+) create mode 100755 tests/jack_coremidi_shutdown_test.sh diff --git a/tests/jack_coremidi_shutdown_test.sh b/tests/jack_coremidi_shutdown_test.sh new file mode 100755 index 000000000..e7b9ab1ff --- /dev/null +++ b/tests/jack_coremidi_shutdown_test.sh @@ -0,0 +1,157 @@ +#!/bin/bash + +# Test that a jackd server loaded with the CoreMIDI slave driver can be stopped +# and started again on macOS. +# +# Before the fix this test guards, stopping such a server never finished: the CoreMIDI +# threads did not honour a cancellation request, so jackd stayed blocked while shutting +# down. A server that did start after a quick restart could also register its MIDI ports +# twice. +# +# The test starts "jackd -X coremidi -d dummy", asks it to stop with SIGTERM, checks that +# it exits in time, and repeats that several times without any pause in between. +# No audio device is used. +# +# usage: +# ./jack_coremidi_shutdown_test.sh [path/to/jackd] +# +# environment: +# JACK_TEST_CYCLES number of start/stop cycles (default: 5) +# JACK_TEST_TIMEOUT seconds to wait for a start or a stop (default: 10) +# +# exit status: 0 on success, 1 on failure, 77 if the test cannot run here (not macOS, +# jackd not found, or the CoreMIDI driver is not available). +# +# Note: jackd keeps a registry of at most 8 server names. A server that had to be killed +# keeps its entry, so this test always uses the same server name, which is reclaimed +# the next time it runs. + +set -o pipefail + +JACKD=${1:-jackd} +CYCLES=${JACK_TEST_CYCLES:-5} +TIMEOUT=${JACK_TEST_TIMEOUT:-10} +SERVER_NAME=jack_coremidi_shutdown_test + +LOG=$(mktemp /tmp/jack_coremidi_shutdown_test.XXXXXX) +JACKD_PID= + +#any failed cycle will set this to 1 (error) +FINAL_RETURN_VALUE=0 + +function cleanup() +{ + if [ -n "$JACKD_PID" ] && kill -0 "$JACKD_PID" 2> /dev/null; + then + kill -9 "$JACKD_PID" 2> /dev/null + fi + rm -f "$LOG" +} + +trap cleanup EXIT + +function skip() +{ + echo "SKIPPED: $1" + exit 77 +} + +function fail() +{ + echo "** FAILED: $1" + FINAL_RETURN_VALUE=1 +} + +#wait until the process is gone: $1 = pid, $2 = timeout in seconds +#return 0 if it exited in time +function wait_exit() +{ + local ticks=$(( $2 * 10 )) + while kill -0 "$1" 2> /dev/null; + do + if [ $ticks -le 0 ]; + then + return 1 + fi + ticks=$(( ticks - 1 )) + sleep 0.1 + done + return 0 +} + +#kill a server that did not stop on its own. It can take a moment for the kill to +#be effective if the process is stuck in a kernel call, so try again a few times +function force_kill() +{ + local tries=0 + while kill -0 "$1" 2> /dev/null && [ $tries -lt 10 ]; + do + kill -9 "$1" 2> /dev/null + tries=$(( tries + 1 )) + sleep 0.5 + done + #reap the process so that the shell does not report it later + { wait "$1"; } 2> /dev/null +} + +[ "$(uname)" = "Darwin" ] || skip "this test is for macOS" +command -v "$JACKD" > /dev/null 2>&1 || skip "jackd not found: $JACKD" + +for cycle in $(seq 1 "$CYCLES"); +do + echo "cycle $cycle/$CYCLES" + : > "$LOG" + "$JACKD" -n "$SERVER_NAME" -X coremidi -d dummy -r 48000 -p 256 > "$LOG" 2>&1 < /dev/null & + JACKD_PID=$! + + #wait for the CoreMIDI driver to be fully started + started=0 + ticks=$(( TIMEOUT * 10 )) + while [ $ticks -gt 0 ]; + do + if grep -q "JackCoreMidiDriver::Start - Driver started" "$LOG"; + then + started=1 + break + fi + if ! kill -0 "$JACKD_PID" 2> /dev/null; + then + break + fi + ticks=$(( ticks - 1 )) + sleep 0.1 + done + + if [ $started -ne 1 ]; + then + if grep -q -E "Driver \"coremidi\" cannot be loaded|Could not find|No such file" "$LOG" && [ "$cycle" -eq 1 ]; + then + force_kill "$JACKD_PID" + skip "the CoreMIDI driver is not available" + fi + fail "the server did not start (cycle $cycle)" + sed 's/^/ | /' "$LOG" | tail -n 5 + force_kill "$JACKD_PID" + JACKD_PID= + continue + fi + + kill -TERM "$JACKD_PID" + if wait_exit "$JACKD_PID" "$TIMEOUT"; + then + echo " OK: stopped" + else + fail "the server did not stop within $TIMEOUT s (cycle $cycle)" + force_kill "$JACKD_PID" + fi + JACKD_PID= +done + +if [ $FINAL_RETURN_VALUE -eq 0 ]; +then + echo "PASSED" +else + echo "FAILED" +fi + +exit $FINAL_RETURN_VALUE