diff --git a/.vscode/settings.json b/.vscode/settings.json new file mode 100644 index 00000000..1b949339 --- /dev/null +++ b/.vscode/settings.json @@ -0,0 +1,12 @@ +{ + "workbench.colorCustomizations": { + "titleBar.activeBackground": "#282768", + "titleBar.activeForeground": "#ffffff", + "titleBar.inactiveBackground": "#282768", + "titleBar.inactiveForeground": "#ffffff", + "activityBar.background": "#1d1c4a", + "activityBar.foreground": "#ffffff", + "statusBar.background": "#282768", + "statusBar.foreground": "#ffffff" + } +} diff --git a/ChangeLog.rst b/ChangeLog.rst index fde1d3ef..d1ae4ed0 100644 --- a/ChangeLog.rst +++ b/ChangeLog.rst @@ -1,7 +1,7 @@ ChangeLog ######### -* Unreleased (sastraxi fork) +* Unreleased (TreeFallSound fork) * netadapter: clear PI controller integrator on ringbuffer reset. ``JackPIControler::OurOfBounds()`` exists for this but had zero call @@ -27,11 +27,45 @@ ChangeLog kernel-route-table behavior. * ``build-macos-pkg.sh`` — produces a ``.pkg`` installer that drops the fork's binaries, libs, headers, and intclient ``.so``s into - ``/usr/local`` on Apple Silicon (the manual-install prefix; Homebrew + ``/usr/local`` on Apple Silicon (the manual-instaSSll prefix; Homebrew is at ``/opt/homebrew``). Required because ``installer/build-pkg.sh`` in the JackRouter repo ``check_jack``s a pre-installed jack2 and refuses to build without one — without this, fresh JackRouter users on Apple Silicon have no way to get the multicast-pin code. + * netJACK2 (master): recover from a slave that disappears without + sending ``KILL_MASTER`` (yanked cable, hard-killed slave). A master + whose RT link hits a fatal recv/send error marks itself dead + (``fDead``) and the manager reaps it on its next listen pass + (``ReapDeadMasters``) instead of stalling the master cycle ~2 s per + period indefinitely. Replaces the "ugly temporary fix" ``ThreadExit`` + that had been in ``JackNetMasterInterface::FatalRecvError`` since 2008. + * netJACK2 (master): fix a use-after-free in ``KillMaster``. The old + ``erase(it); delete *it;`` freed through an iterator that ``erase`` + had already invalidated. This path runs on the *normal* multicast + ``KILL_MASTER`` clean-stop, not only on edge cases. + * netJACK2 (master): dedupe masters by slave name in ``InitMaster``. + A slave that restarts quickly keeps feeding its old master packets, + so that master never times out or self-declares dead, and + ``FindMaster`` only matches ``fID`` which a re-announcing slave + lacks. Without the dedupe each re-announcement spawns another JACK + client (``pistomp-01``, ``-02``, ...) all fighting for the same ports. + * netJACK2: initialize ``fDead`` in the ``session_params_t`` + constructor as well, so a master created through that path cannot + start life reading an indeterminate dead flag. + * netJACK2: fail closed when a requested multicast interface pin cannot + be applied. ``NewSocket()`` refuses the socket rather than letting + the slave fall back to the default route; the reconnect loop retries. + * netJACK2: pin *unicast* egress to an interface, not only the + multicast join. On Linux ``JACK_NETJACK_MULTICAST_IF`` set only + ``IP_MULTICAST_IF``, so the post-discovery unicast RT stream still + followed the default route and could leave over wifi when both the + wired and the wifi interface carried a ``169.254`` link-local + address. The slave now also applies ``IP_UNICAST_IF``; the master + captures the arrival interface of each ``SLAVE_AVAILABLE`` via + ``IP_PKTINFO``, latches the first one seen, and pins every spawned + master socket to it (``IP_BOUND_IF`` on macOS/BSD, ``IP_UNICAST_IF`` + on Linux). Env-gated and fail-closed; the latch re-resolves if the + pinned interface disappears. * 1.9.22 (2023-02-02) diff --git a/NETJACK-REAPING.md b/NETJACK-REAPING.md new file mode 100644 index 00000000..2159c9fc --- /dev/null +++ b/NETJACK-REAPING.md @@ -0,0 +1,132 @@ +# The netmanager master is not reaped when the slave goes away + +## The symptom + +A netJACK2 slave stops. The cause can be a clean stop, a hard kill, or a +yanked cable. The master side does not recover. + +The dead master client stays registered in the server. Its ports stay +connected. Every audio cycle then waits for a packet that never comes. The +wait is `PACKET_TIMEOUT * NETWORK_DEFAULT_LATENCY` = 2 seconds. The audio +budget at 48 kHz / 128 frames is 2.67 milliseconds. The failure is permanent +and silent. + +If the slave restarts, a second master is created. The graph fills with +`pistomp-01`, `pistomp-02`, and more. Each one fights for the same ports. + +## The cause + +There are four defects. Each one is on its own path. + +### 1. `FatalRecvError` kills the RT thread + +`JackNetMasterInterface::FatalRecvError` and `FatalSendError` call +`ThreadExit()`. That code runs as the JACK process callback, on the real-time +graph thread (`JackNetMaster::Process`). `ThreadExit()` ends that thread. + +The client is still registered. Nothing services it. The server still calls +its cycle, and the cycle waits out the full `PACKET_TIMEOUT` every time. A +comment in the source calls this "an UGLY temporary way". + +### 2. `KillMaster` is a use-after-free + +`JackNetMasterManager::KillMaster` does this: + + fMasterList.erase(master_it); + delete (*master_it); + +`erase()` makes the iterator not valid. The next line reads it. The `delete` +then acts on whatever that read produced. So the multicast `KILL_MASTER` +path — the one clean stop uses — does not reap the master either. It corrupts +the heap. + +### 3. `InitMaster` does not check for an existing slave + +`JackNetMasterManager::InitMaster` creates a new `JackNetMaster` for every +`SLAVE_AVAILABLE` packet. It does not look for a master that already holds +that slave name. A slave that restarts, or a fast Ethernet Audio toggle on +the pedal, sends a fresh `SLAVE_AVAILABLE` while the old master is still in +the list. A duplicate is the result. + +The old master often cannot detect this on its own. The restarted slave +sends to the same multicast group and port, so the old master keeps +receiving packets and never times out. `FindMaster` only matches `fID`, and +a re-announcing slave does not carry the old `fID`. So nothing removes the +stale master. + +### 4. The slave does not pin its multicast interface + +`JackNetAdapter` calls `fSocket.SetMulticastIF()` before `NewSocket()`. The +socket does not exist yet, so the `setsockopt` acts on file descriptor 0. +`NewSocket()` never re-applies the option. The slave's reconnect loop builds +a new socket on every attempt, and none of them is pinned. + +A leaked netadapter then sends its `SLAVE_AVAILABLE` announcements out the +default route. On a host with the cable on one interface and Wi-Fi on +another, teardown deletes the cable's route, and the announcements go out +**over Wi-Fi**. + +## The correction + +### 1. Reap off the RT thread + +`FatalRecvError` and `FatalSendError` set an atomic flag `fDead` and return. +They do not call `ThreadExit()`. `Exit()` still runs, so `fRunning` becomes +false and the multicast euthanasia request is still sent. + +`JackNetMasterManager::Run` calls `ReapDeadMasters()` at the top of its loop. +That function walks the master list and destroys every master whose +`IsDead()` is true. The loop wakes at least every `MANAGER_INIT_TIMEOUT` +(2 seconds), so a silent network is still handled. + +### 2. Capture the pointer before the erase + +A new function `RemoveMaster(master_list_it_t)` reads the pointer, then +erases, then deletes: + + JackNetMaster* master = *master_it; + fMasterList.erase(master_it); + delete master; + +`KillMaster`, `ReapDeadMasters`, and the dedupe path all use it. + +### 3. Dedupe by slave name + +`InitMaster` walks the master list first. It reaps every master whose +`fParams.fName` equals the new slave's name. Then it creates the new master. + +`ReapDeadMasters()` runs at the top of the same `Run()` pass, so a +self-declared-dead master is already gone. A name match in `InitMaster` is +therefore always a live master, deliberately superseded. + +The `sleep 3` in the pi-Stomp `jackbridge-pi-up` helper was a workaround for +this defect. It can be removed once this correction is in the field. + +### 4. Re-apply the multicast interface on every socket + +`JackNetUnixSocket` stores the interface name in `fMcastIF`. `SetMulticastIF` +records the name and applies it only if the socket already exists. +`NewSocket()` re-applies it, through a new private `ApplyMulticastIF()`. + +The re-apply is fail closed. If a pin was asked for and cannot be set, +`NewSocket()` closes the socket and returns `SOCKET_ERROR`. The slave then +retries in its reconnect loop instead of announcing on the default route. So +the "over Wi-Fi" case is handled inside the fork, and the pi teardown does +not need to add or remove kernel routes. + +## Note + +Defect 1 also returns `SOCKET_ERROR` from the process callback. What the +server does with it is traced, not guessed: for an internal client like +netmanager, `JackClient::CycleSignalAux` sees `status != 0` and calls +`End()`, which clears `fActive` and deactivates the client — the graph +stops scheduling it at all. That is acceptable, even helpful: `fRunning` +is already false, any cycle that still runs returns 0 immediately, and +the manager reaps the master within one loop pass (at most 2 s). + +The slave reconnect loop (`JackNetSlaveInterface::Init`) stays unbounded. A +legitimate slave waits there for its master. Defect 3's correction is what +makes an unbounded wait safe on the master side. + +The Windows socket (`JackNetWinSocket`) is not changed. It is not built for +the pi-Stomp targets. Its `SetMulticastIF` keeps the apply-at-call behavior. diff --git a/build-macos-pkg.sh b/build-macos-pkg.sh index 9a63bb52..d52c67e0 100755 --- a/build-macos-pkg.sh +++ b/build-macos-pkg.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Build a macOS .pkg installer for the sastraxi/jack2 fork. +# Build a macOS .pkg installer for the TreeFallSound/jack2 fork. # # The fork carries the pi-stomp/JackBridge patches on top of upstream # v1.9.22 (PI controller reset, master-side and slave-side multicast @@ -30,14 +30,14 @@ STAGING="$BUILD/staging-pkg" PKG_OUT_DIR="$BUILD" # Version: explicit arg wins, else derived from the fork's commit count -# past v1.9.22 + a short SHA. e.g. 1.9.22+sastraxi.3.gb3bfc408 +# past v1.9.22 + a short SHA. e.g. 1.9.22+treefall.3.gb3bfc408 if [ $# -ge 1 ]; then VERSION="$1" else BASE="1.9.22" COUNT=$(git rev-list --count "v1.9.22..HEAD" 2>/dev/null || echo "0") SHORT=$(git rev-parse --short HEAD) - VERSION="${BASE}+sastraxi.${COUNT}.g${SHORT}" + VERSION="${BASE}+treefall.${COUNT}.g${SHORT}" fi echo "==> jack2 fork @ $(git rev-parse --short HEAD) ($(git log -1 --pretty=%s))" @@ -77,24 +77,22 @@ python3 ./waf install --destdir="$STAGING" echo echo "==> build the .pkg" -PKG_ID="com.sastraxi.jack2" +PKG_ID="com.treefallsound.jack2" PKG_OUT="$PKG_OUT_DIR/jack2-${VERSION}.pkg" -# --root: the directory whose contents become the payload (so the -# payload layout matches the install-location layout, /usr/local/...) -# --install-location: the absolute path the payload is rooted at on -# the target. The pkg is relocatable to any prefix only if we -# don't hardcode paths in dylibs, but our jackd and dylibs use -# absolute @rpath-style install_names, so /usr/local is the -# only sensible install-location for this fork. pkgbuild \ --root "$STAGING" \ --identifier "$PKG_ID" \ --version "$VERSION" \ - --install-location /usr/local \ + --install-location / \ --ownership recommended \ "$PKG_OUT" +# The staging tree already contains usr/local/... because waf installs with +# DESTDIR. The package root is therefore /; using /usr/local here would place +# the payload at /usr/local/usr/local and leave the live JACK installation +# untouched. + echo echo "==> done" ls -la "$PKG_OUT" @@ -105,6 +103,5 @@ echo echo "Install with:" echo " sudo installer -pkg $PKG_OUT -target /" echo -echo "Verify after install:" -echo " /usr/local/bin/jackd --version" -echo " strings /usr/local/lib/jack/netmanager.so | grep JACK_NETJACK_MULTICAST_IF" +echo "JACK netJACK interface pinning landed in netmanager.so:" +strings /usr/local/lib/jack/netmanager.so | grep -E 'JACK_NETJACK_MULTICAST_IF|pinning masters' || echo " NOT FOUND — install the package at /usr/local and restart JACK" diff --git a/common/JackAudioAdapterInterface.cpp b/common/JackAudioAdapterInterface.cpp index d7fe1b3e..f4ba12d3 100644 --- a/common/JackAudioAdapterInterface.cpp +++ b/common/JackAudioAdapterInterface.cpp @@ -283,9 +283,11 @@ namespace Jack } } } - // Reset all ringbuffers in case of failure if (failure) { - jack_error("JackAudioAdapterInterface::PushAndPull ringbuffer failure... reset"); + uint64_t failure_count = FailureReportCount(); + if (failure_count > 0) { + jack_error("JackAudioAdapterInterface::PushAndPull ringbuffer failure... reset; failures since last report = %llu", (unsigned long long)failure_count); + } if (fAdaptative) { GrowRingBufferSize(); jack_info("Ringbuffer size = %d frames", fRingbufferCurSize); @@ -297,9 +299,21 @@ namespace Jack } } + uint64_t JackAudioAdapterInterface::FailureReportCount() + { + ++fFailureCount; + jack_time_t now = GetMicroSeconds(); + if (fLastFailureReport != 0 && now - fLastFailureReport < 1000000) { + return 0; + } + fLastFailureReport = now; + uint64_t count = fFailureCount; + fFailureCount = 0; + return count; + } + int JackAudioAdapterInterface::PullAndPush(float** inputBuffer, float** outputBuffer, unsigned int frames) { - fPullAndPushTime = GetMicroSeconds(); if (!fRunning) { return 0; } diff --git a/common/JackAudioAdapterInterface.h b/common/JackAudioAdapterInterface.h index 36be35aa..9dc37c7b 100644 --- a/common/JackAudioAdapterInterface.h +++ b/common/JackAudioAdapterInterface.h @@ -94,6 +94,10 @@ namespace Jack bool fRunning; bool fAdaptative; + uint64_t fFailureCount; + jack_time_t fLastFailureReport; + + uint64_t FailureReportCount(); void ResetRingBuffers(); void AdaptRingBufferSize(); @@ -114,7 +118,9 @@ namespace Jack fRingbufferCurSize(ring_buffer_size), fPullAndPushTime(0), fRunning(false), - fAdaptative(true) + fAdaptative(true), + fFailureCount(0), + fLastFailureReport(0) {} JackAudioAdapterInterface(jack_nframes_t host_buffer_size, @@ -133,7 +139,9 @@ namespace Jack fRingbufferCurSize(ring_buffer_size), fPullAndPushTime(0), fRunning(false), - fAdaptative(true) + fAdaptative(true), + fFailureCount(0), + fLastFailureReport(0) {} virtual ~JackAudioAdapterInterface() diff --git a/common/JackClient.cpp b/common/JackClient.cpp index 74a9dca8..57209c09 100644 --- a/common/JackClient.cpp +++ b/common/JackClient.cpp @@ -29,6 +29,12 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. #include "driver_interface.h" #include "JackLibGlobals.h" +#ifdef __APPLE__ +#include "JackWorkgroup.h" +#include "JackPosixThread.h" +#include +#endif + #include #include #include @@ -557,6 +563,40 @@ void JackClient::SetupRealTime() if (fThread.AcquireSelfRealTime(GetEngineControl()->fClientPriority) < 0) { jack_error("JackClient::AcquireSelfRealTime error"); } + +#ifdef __APPLE__ + /* + Join the backend device's CoreAudio workgroup. Plain time-constraint + realtime is not enough on Apple Silicon: an unjoined thread can still be + preempted by WindowServer right before the cycle deadline, which is what + makes netmanager's client miss its slot under GUI load. + + Order matters. os_workgroup_join returns EINVAL on a thread that Mach + does not consider realtime, so this must come after AcquireSelfRealTime, + and it must run here -- on the client's own realtime thread -- because a + join cannot be performed on another thread's behalf. + + JACK_NO_WORKGROUP lets a client opt out. A client that already belongs + to some other device's workgroup (JackBridge's daemon holds its HAL + device's, because it publishes that device's timeline) sets this so we + do not fight over its membership. + + Failure is never fatal: the thread simply keeps the realtime scheduling + it already had, which is the pre-existing behaviour. + */ + if (getenv("JACK_NO_WORKGROUP") == NULL) { + if (JackWorkgroupJoinSelfForDevice(GetEngineControl()->fCoreAudioDeviceID) == 0) { + /* + Leave the workgroup before this thread ends. This is not + optional tidiness: libdispatch stops the process when a thread + ends while it is a member. See JackPosixThread::ThreadHandler. + */ + JackSetThreadExitHook(JackWorkgroupLeaveSelf); + } + } else { + jack_info("JackClient::SetupRealTime : JACK_NO_WORKGROUP set, staying out of the backend workgroup"); + } +#endif } int JackClient::StartThread() diff --git a/common/JackConstants.h b/common/JackConstants.h index 25afd3de..a1e67720 100644 --- a/common/JackConstants.h +++ b/common/JackConstants.h @@ -72,7 +72,7 @@ #define ALL_CLIENTS -1 // for notification -#define JACK_PROTOCOL_VERSION 9 +#define JACK_PROTOCOL_VERSION 10 #define SOCKET_TIME_OUT 2 // in sec #define DRIVER_OPEN_TIMEOUT 5 // in sec diff --git a/common/JackControlAPI.cpp b/common/JackControlAPI.cpp index c13640a9..7b6aadc5 100644 --- a/common/JackControlAPI.cpp +++ b/common/JackControlAPI.cpp @@ -698,6 +698,19 @@ jackctl_wait_signals(jackctl_sigmask_t * sigmask) // driver exit waiting = false; break; + case SIGPIPE: + /* + A write to a peer that went away. Not a reason to stop the + server. The signal stays blocked in every thread, thus the + write returns EPIPE and the caller handles it: netJACK2 + logs "connection lost" and drops that master. Before this, + a cable fault reached the default case and stopped jackd, + which then hung in ClientDeactivate for the client that had + just lost its peer. + */ + jack_info("Jack main ignores SIGPIPE from a lost peer"); + break; + case SIGTTOU: break; default: diff --git a/common/JackEngineControl.h b/common/JackEngineControl.h index 90d7ca48..c4942ff9 100644 --- a/common/JackEngineControl.h +++ b/common/JackEngineControl.h @@ -88,6 +88,13 @@ struct SERVER_EXPORT JackEngineControl : public JackShmMem // Timer alignas(UInt32) alignas(JackFrameTimer) JackFrameTimer fFrameTimer; + // AudioDeviceID of the backend's CoreAudio device, or 0 when the backend + // is not coreaudio (or has not opened yet). Published by + // JackCoreAudioDriver::Open so that client processes -- which never see + // the driver object -- can look up that device's os_workgroup and join it + // from their realtime threads. + UInt32 fCoreAudioDeviceID; + #ifdef JACK_MONITOR JackEngineProfiling fProfiler; #endif @@ -131,6 +138,7 @@ struct SERVER_EXPORT JackEngineControl : public JackShmMem fXrunDelayedUsecs = 0.f; fClockSource = clock; fDriverNum = 0; + fCoreAudioDeviceID = 0; } ~JackEngineControl() diff --git a/common/JackLibSampleRateResampler.cpp b/common/JackLibSampleRateResampler.cpp index f4bdbc88..cfa8a8d8 100644 --- a/common/JackLibSampleRateResampler.cpp +++ b/common/JackLibSampleRateResampler.cpp @@ -85,13 +85,8 @@ unsigned int JackLibSampleRateResampler::ReadResample(jack_default_audio_sample_ int res; jack_ringbuffer_get_read_vector(fRingBuffer, ring_buffer_data); - unsigned int available_frames = (ring_buffer_data[0].len + ring_buffer_data[1].len) / sizeof(jack_default_audio_sample_t); - jack_log("Output available = %ld", available_frames); - for (int j = 0; j < 2; j++) { - if (ring_buffer_data[j].len > 0) { - src_data.data_in = (jack_default_audio_sample_t*)ring_buffer_data[j].buf; src_data.data_out = &buffer[written_frames]; src_data.input_frames = ring_buffer_data[j].len / sizeof(jack_default_audio_sample_t); @@ -101,28 +96,15 @@ unsigned int JackLibSampleRateResampler::ReadResample(jack_default_audio_sample_ res = src_process(fResampler, &src_data); if (res != 0) { - jack_error("JackLibSampleRateResampler::ReadResample ratio = %f err = %s", fRatio, src_strerror(res)); return 0; } frames_to_write -= src_data.output_frames_gen; written_frames += src_data.output_frames_gen; - - if ((src_data.input_frames_used == 0 || src_data.output_frames_gen == 0) && j == 0) { - jack_log("Output : j = %d input_frames_used = %ld output_frames_gen = %ld frames1 = %lu frames2 = %lu" - , j, src_data.input_frames_used, src_data.output_frames_gen, ring_buffer_data[0].len, ring_buffer_data[1].len); - } - - jack_log("Output : j = %d input_frames_used = %ld output_frames_gen = %ld", j, src_data.input_frames_used, src_data.output_frames_gen); jack_ringbuffer_read_advance(fRingBuffer, src_data.input_frames_used * sizeof(jack_default_audio_sample_t)); } } - if (written_frames < frames) { - jack_error("Output available = %ld", available_frames); - jack_error("JackLibSampleRateResampler::ReadResample error written_frames = %ld", written_frames); - } - return written_frames; } @@ -135,13 +117,8 @@ unsigned int JackLibSampleRateResampler::WriteResample(jack_default_audio_sample int res; jack_ringbuffer_get_write_vector(fRingBuffer, ring_buffer_data); - unsigned int available_frames = (ring_buffer_data[0].len + ring_buffer_data[1].len) / sizeof(jack_default_audio_sample_t); - jack_log("Input available = %ld", available_frames); - for (int j = 0; j < 2; j++) { - if (ring_buffer_data[j].len > 0) { - src_data.data_in = &buffer[read_frames]; src_data.data_out = (jack_default_audio_sample_t*)ring_buffer_data[j].buf; src_data.input_frames = frames_to_read; @@ -151,28 +128,15 @@ unsigned int JackLibSampleRateResampler::WriteResample(jack_default_audio_sample res = src_process(fResampler, &src_data); if (res != 0) { - jack_error("JackLibSampleRateResampler::WriteResample ratio = %f err = %s", fRatio, src_strerror(res)); return 0; } frames_to_read -= src_data.input_frames_used; read_frames += src_data.input_frames_used; - - if ((src_data.input_frames_used == 0 || src_data.output_frames_gen == 0) && j == 0) { - jack_log("Input : j = %d input_frames_used = %ld output_frames_gen = %ld frames1 = %lu frames2 = %lu" - , j, src_data.input_frames_used, src_data.output_frames_gen, ring_buffer_data[0].len, ring_buffer_data[1].len); - } - - jack_log("Input : j = %d input_frames_used = %ld output_frames_gen = %ld", j, src_data.input_frames_used, src_data.output_frames_gen); jack_ringbuffer_write_advance(fRingBuffer, src_data.output_frames_gen * sizeof(jack_default_audio_sample_t)); } } - if (read_frames < frames) { - jack_error("Input available = %ld", available_frames); - jack_error("JackLibSampleRateResampler::WriteResample error read_frames = %ld", read_frames); - } - return read_frames; } diff --git a/common/JackNetAPI.cpp b/common/JackNetAPI.cpp index 7836d3da..66d47cf6 100644 --- a/common/JackNetAPI.cpp +++ b/common/JackNetAPI.cpp @@ -352,11 +352,15 @@ struct JackNetExtMaster : public JackNetMasterInterface { /// Network init if (!JackNetMasterInterface::Init()) { + // Release the socket immediately so a caller's retry does not + // build on a wedged fd (same rationale as JackNetMaster::Init). + fSocket.Close(); return -1; } // Set global parameters if (!SetParams()) { + fSocket.Close(); return -1; } diff --git a/common/JackNetInterface.cpp b/common/JackNetInterface.cpp index a1a6b46f..c85d929b 100644 --- a/common/JackNetInterface.cpp +++ b/common/JackNetInterface.cpp @@ -415,22 +415,25 @@ namespace Jack void JackNetMasterInterface::FatalRecvError() { - // fatal connection issue, exit + // fatal connection issue jack_error("Recv connection lost error = %s, '%s' exiting", StrError(NET_ERROR_CODE), fParams.fName); - // ask to the manager to properly remove the master + // stop the process loop and send the multicast euthanasia request Exit(); - // UGLY temporary way to be sure the thread does not call code possibly causing a deadlock in JackEngine. - ThreadExit(); + // Flag for the manager thread to reap. Do NOT ThreadExit() here: this + // runs as the JACK process callback on the RT graph thread, and killing + // that thread leaves the client registered with nothing to service it — + // every following audio cycle then waits out the full PACKET_TIMEOUT. + // The manager polls IsDead() and destroys the master off the RT thread. + fDead.store(true, std::memory_order_release); } void JackNetMasterInterface::FatalSendError() { - // fatal connection issue, exit + // fatal connection issue jack_error("Send connection lost error = %s, '%s' exiting", StrError(NET_ERROR_CODE), fParams.fName); - // ask to the manager to properly remove the master Exit(); - // UGLY temporary way to be sure the thread does not call code possibly causing a deadlock in JackEngine. - ThreadExit(); + // See FatalRecvError: reap from the manager thread, never ThreadExit() here. + fDead.store(true, std::memory_order_release); } int JackNetMasterInterface::Recv(size_t size, int flags) diff --git a/common/JackNetInterface.h b/common/JackNetInterface.h index 3c421022..8073e89f 100644 --- a/common/JackNetInterface.h +++ b/common/JackNetInterface.h @@ -22,6 +22,7 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. #include "JackNetTool.h" #include +#include namespace Jack { @@ -156,7 +157,17 @@ namespace Jack bool fRunning; int fCurrentCycleOffset; int fMaxCycleOffset; - bool fSynched; + // Written on the RT thread when the sync offset is reached, + // read on the manager thread by IsSynched() (supersede guard). + // Atomic for the same reason fDead is: cross-thread access. + std::atomic fSynched; + // Raised by FatalRecvError / FatalSendError on the RT process + // thread. The manager thread polls IsDead() and reaps the master + // off the RT thread — the old code called ThreadExit() from inside + // the process callback, leaving the JACK client registered with a + // dead RT thread, which made every subsequent audio cycle wait out + // the full PACKET_TIMEOUT. See NETJACK-REAPING.md. + std::atomic fDead; bool Init(); bool SetParams(); @@ -183,21 +194,33 @@ namespace Jack JackNetMasterInterface() : JackNetInterface(), - fRunning(false), - fCurrentCycleOffset(0), - fMaxCycleOffset(0), - fSynched(false) + fRunning(false), + fCurrentCycleOffset(0), + fMaxCycleOffset(0), + fSynched(false), + fDead(false) {} JackNetMasterInterface(session_params_t& params, JackNetSocket& socket, const char* multicast_ip) - : JackNetInterface(params, socket, multicast_ip), - fRunning(false), - fCurrentCycleOffset(0), - fMaxCycleOffset(0), - fSynched(false) + : JackNetInterface(params, socket, multicast_ip), + fRunning(false), + fCurrentCycleOffset(0), + fMaxCycleOffset(0), + fSynched(false), + fDead(false) {} virtual~JackNetMasterInterface() {} + + // True once a fatal socket error has torn the link down. Polled by + // JackNetMasterManager to reap the master off the RT thread. + bool IsDead() const { return fDead.load(std::memory_order_acquire); } + // True once the sync exchange with this slave completed and the + // RT cycle offset reached fMaxCycleOffset — i.e. the session is + // exchanging packets, not merely registered. InitMaster uses this + // to decide whether an incoming announce is duplicate discovery + // traffic (ignore) or a new incarnation (supersede). + bool IsSynched() const { return fSynched.load(std::memory_order_acquire); } }; /** diff --git a/common/JackNetManager.cpp b/common/JackNetManager.cpp index 7d832aa2..a05f5804 100644 --- a/common/JackNetManager.cpp +++ b/common/JackNetManager.cpp @@ -112,12 +112,24 @@ namespace Jack //network init if (!JackNetMasterInterface::Init()) { jack_error("JackNetMasterInterface::Init() error..."); + // Close the failed socket now, not at destruction. A failed + // handshake (typically ECONNREFUSED from the slave's previous + // incarnation, whose connected peer no longer has a listener) + // leaves the socket wedged with a queued ICMP error. The manager + // retries on the slave's next announce; releasing the fd here + // guarantees that retry builds on clean kernel state instead of + // inheriting the error queue (docs/plan-replug-recovery.md, + // Correction 2b: "must not continue with a socket that does not + // operate"). + fSocket.Close(); return false; } //set global parameters if (!SetParams()) { jack_error("SetParams error..."); + // Same discipline: release the socket before the manager retries. + fSocket.Close(); return false; } @@ -239,25 +251,36 @@ namespace Jack { jack_log("JackNetMaster::FreePorts ID = %u", fParams.fID); + // Null each slot as its port is unregistered. jack_port_unregister + // triggers a graph latency recomputation, which re-enters our + // LatencyCallback mid-loop; if the array kept the stale pointer, + // that callback fed it to jack_port_set_latency_range and jackd + // logged "called with an incorrect port " — a use-after- + // free visible on every master teardown (docs/plan-replug-recovery.md, + // "Also seen"). LatencyCallback skips NULL slots for the same reason. int port_index; for (port_index = 0; port_index < fParams.fSendAudioChannels; port_index++) { if (fAudioCapturePorts[port_index]) { jack_port_unregister(fClient, fAudioCapturePorts[port_index]); + fAudioCapturePorts[port_index] = NULL; } } for (port_index = 0; port_index < fParams.fReturnAudioChannels; port_index++) { if (fAudioPlaybackPorts[port_index]) { jack_port_unregister(fClient, fAudioPlaybackPorts[port_index]); + fAudioPlaybackPorts[port_index] = NULL; } } for (port_index = 0; port_index < fParams.fSendMidiChannels; port_index++) { if (fMidiCapturePorts[port_index]) { jack_port_unregister(fClient, fMidiCapturePorts[port_index]); + fMidiCapturePorts[port_index] = NULL; } } for (port_index = 0; port_index < fParams.fReturnMidiChannels; port_index++) { if (fMidiPlaybackPorts[port_index]) { jack_port_unregister(fClient, fMidiPlaybackPorts[port_index]); + fMidiPlaybackPorts[port_index] = NULL; } } } @@ -395,31 +418,38 @@ namespace Jack JackNetMaster* obj = static_cast(arg); jack_nframes_t port_latency = jack_get_buffer_size(obj->fClient); jack_latency_range_t range; - + + // FreePorts() nulls each slot as it unregisters, and unregistering + // re-enters this callback mid-loop; a NULL slot means the port is + // already gone and must be skipped, not passed on. + //audio for (int i = 0; i < obj->fParams.fSendAudioChannels; i++) { + if (!obj->fAudioCapturePorts[i]) continue; //port latency range.min = range.max = float(obj->fParams.fNetworkLatency * port_latency) / 2.f; jack_port_set_latency_range(obj->fAudioCapturePorts[i], JackPlaybackLatency, &range); } - + //audio for (int i = 0; i < obj->fParams.fReturnAudioChannels; i++) { + if (!obj->fAudioPlaybackPorts[i]) continue; //port latency range.min = range.max = float(obj->fParams.fNetworkLatency * port_latency) / 2.f + ((obj->fParams.fSlaveSyncMode) ? 0 : port_latency); jack_port_set_latency_range(obj->fAudioPlaybackPorts[i], JackCaptureLatency, &range); } - + //midi for (int i = 0; i < obj->fParams.fSendMidiChannels; i++) { + if (!obj->fMidiCapturePorts[i]) continue; //port latency range.min = range.max = float(obj->fParams.fNetworkLatency * port_latency) / 2.f; jack_port_set_latency_range(obj->fMidiCapturePorts[i], JackPlaybackLatency, &range); } - + //midi for (int i = 0; i < obj->fParams.fReturnMidiChannels; i++) { - //port latency + if (!obj->fMidiPlaybackPorts[i]) continue; range.min = range.max = obj->fParams.fNetworkLatency * port_latency + ((obj->fParams.fSlaveSyncMode) ? 0 : port_latency); jack_port_set_latency_range(obj->fMidiPlaybackPorts[i], JackCaptureLatency, &range); } @@ -448,14 +478,81 @@ namespace Jack } } + void JackNetMaster::RecordDiagnosticStage(uint64_t& max_usecs, + jack_time_t start, + jack_time_t end) + { + const uint64_t elapsed = (end >= start) ? (end - start) : 0; + if (elapsed > max_usecs) max_usecs = elapsed; + } + + void JackNetMaster::FinishDiagnosticCycle(jack_time_t start, jack_time_t end) + { + const uint64_t elapsed = (end >= start) ? (end - start) : 0; + if (elapsed > fDiagMaxProcessUsecs) fDiagMaxProcessUsecs = elapsed; + + const uint64_t period_usecs = fParams.fSampleRate + ? (1000000ULL * fParams.fPeriodSize / fParams.fSampleRate) + : 0; + if (period_usecs && elapsed > period_usecs) ++fDiagSlowCycles; + + ReportDiagnosticsIfDue(end); + } + + void JackNetMaster::ReportDiagnosticsIfDue(jack_time_t now) + { + static const jack_time_t kReportIntervalUsecs = 5000000; + if (fDiagLastReportUsecs == 0) { + fDiagLastReportUsecs = now; + return; + } + if (now - fDiagLastReportUsecs < kReportIntervalUsecs) return; + + // This is intentionally one interval summary, not a per-packet log. + // The counters are written by this JACK process callback only; keeping + // the report here avoids locks or cross-thread reads in the RT path. + jack_info("NetMaster diagnostics slave=%s cycles=%llu " + "dataPacketErrors=%llu syncPacketErrors=%llu " + "socketErrors=%llu slowCycles=%llu " + "maxProcessUsecs=%llu maxSyncSendUsecs=%llu " + "maxDataSendUsecs=%llu maxSyncRecvUsecs=%llu " + "maxDataRecvUsecs=%llu", + fParams.fName, + (unsigned long long)fDiagCycles, + (unsigned long long)fDiagDataPacketErrors, + (unsigned long long)fDiagSyncPacketErrors, + (unsigned long long)fDiagSocketErrors, + (unsigned long long)fDiagSlowCycles, + (unsigned long long)fDiagMaxProcessUsecs, + (unsigned long long)fDiagMaxSyncSendUsecs, + (unsigned long long)fDiagMaxDataSendUsecs, + (unsigned long long)fDiagMaxSyncRecvUsecs, + (unsigned long long)fDiagMaxDataRecvUsecs); + + fDiagCycles = 0; + fDiagDataPacketErrors = 0; + fDiagSyncPacketErrors = 0; + fDiagSocketErrors = 0; + fDiagSlowCycles = 0; + fDiagMaxProcessUsecs = 0; + fDiagMaxSyncSendUsecs = 0; + fDiagMaxDataSendUsecs = 0; + fDiagMaxSyncRecvUsecs = 0; + fDiagMaxDataRecvUsecs = 0; + fDiagLastReportUsecs = now; + } + int JackNetMaster::Process() { if (!fRunning) { return 0; } + const jack_time_t process_start = GetMicroSeconds(); + ++fDiagCycles; + #ifdef JACK_MONITOR - jack_time_t begin_time = GetMicroSeconds(); + jack_time_t begin_time = process_start; fNetTimeMon->New(); #endif @@ -512,35 +609,56 @@ namespace Jack // encode the first packet EncodeSyncPacket(); - if (SyncSend() == SOCKET_ERROR) { - return SOCKET_ERROR; + jack_time_t stage_start = GetMicroSeconds(); + int result = SyncSend(); + jack_time_t stage_end = GetMicroSeconds(); + RecordDiagnosticStage(fDiagMaxSyncSendUsecs, stage_start, stage_end); + if (result == SOCKET_ERROR) { + ++fDiagSocketErrors; + FinishDiagnosticCycle(process_start, stage_end); + return result; } #ifdef JACK_MONITOR - fNetTimeMon->Add((((float)(GetMicroSeconds() - begin_time)) / (float) fPeriodUsecs) * 100.f); + fNetTimeMon->Add((((float)(stage_end - begin_time)) / (float) fPeriodUsecs) * 100.f); #endif // send data - if (DataSend() == SOCKET_ERROR) { - return SOCKET_ERROR; + stage_start = GetMicroSeconds(); + result = DataSend(); + stage_end = GetMicroSeconds(); + RecordDiagnosticStage(fDiagMaxDataSendUsecs, stage_start, stage_end); + if (result == SOCKET_ERROR) { + ++fDiagSocketErrors; + FinishDiagnosticCycle(process_start, stage_end); + return result; } #ifdef JACK_MONITOR - fNetTimeMon->Add((((float)(GetMicroSeconds() - begin_time)) / (float) fPeriodUsecs) * 100.f); + fNetTimeMon->Add((((float)(stage_end - begin_time)) / (float) fPeriodUsecs) * 100.f); #endif // receive sync - int res = SyncRecv(); - switch (res) { - + stage_start = GetMicroSeconds(); + result = SyncRecv(); + stage_end = GetMicroSeconds(); + RecordDiagnosticStage(fDiagMaxSyncRecvUsecs, stage_start, stage_end); + switch (result) { + case NET_SYNCHING: + FinishDiagnosticCycle(process_start, stage_end); + return result; + case SOCKET_ERROR: - return res; - + ++fDiagSocketErrors; + FinishDiagnosticCycle(process_start, stage_end); + return result; + case SYNC_PACKET_ERROR: + ++fDiagSyncPacketErrors; // Since sync packet is incorrect, don't decode it and continue with data break; - + default: // Decode sync int unused_frames; @@ -549,26 +667,35 @@ namespace Jack } #ifdef JACK_MONITOR - fNetTimeMon->Add((((float)(GetMicroSeconds() - begin_time)) / (float) fPeriodUsecs) * 100.f); + fNetTimeMon->Add((((float)(stage_end - begin_time)) / (float) fPeriodUsecs) * 100.f); #endif - + // receive data - res = DataRecv(); - switch (res) { - + stage_start = GetMicroSeconds(); + result = DataRecv(); + stage_end = GetMicroSeconds(); + RecordDiagnosticStage(fDiagMaxDataRecvUsecs, stage_start, stage_end); + switch (result) { + case 0: + break; + case SOCKET_ERROR: - return res; - + ++fDiagSocketErrors; + FinishDiagnosticCycle(process_start, stage_end); + return result; + case DATA_PACKET_ERROR: + ++fDiagDataPacketErrors; // Well not a real XRun... JackServerGlobals::fInstance->GetEngine()->NotifyClientXRun(ALL_CLIENTS); break; } #ifdef JACK_MONITOR - fNetTimeMon->AddLast((((float)(GetMicroSeconds() - begin_time)) / (float) fPeriodUsecs) * 100.f); + fNetTimeMon->AddLast((((float)(stage_end - begin_time)) / (float) fPeriodUsecs) * 100.f); #endif + FinishDiagnosticCycle(process_start, stage_end); return 0; } @@ -667,9 +794,12 @@ namespace Jack // wrong one. Empty/unset keeps the legacy INADDR_ANY behavior. const char* multicast_if = getenv("JACK_NETJACK_MULTICAST_IF"); fMulticastIF[0] = '\0'; + fBoundIF = 0; + fPinFromEnv = false; if (multicast_if) { strncpy(fMulticastIF, multicast_if, sizeof(fMulticastIF) - 1); fMulticastIF[sizeof(fMulticastIF) - 1] = '\0'; + fPinFromEnv = true; } for (node = params; node; node = jack_slist_next(node)) { @@ -818,6 +948,9 @@ namespace Jack jack_error("Can't set local loop : %s", StrError(NET_ERROR_CODE)); } + //record the arrival interface of each announce (for master egress pin) + fSocket.SetRecvIF(); + //set a timeout on the multicast receive (the thread can now be cancelled) if (fSocket.SetTimeOut(MANAGER_INIT_TIMEOUT) == SOCKET_ERROR) { jack_error("Can't set timeout : %s", StrError(NET_ERROR_CODE)); @@ -826,6 +959,11 @@ namespace Jack //main loop, wait for data, deal with it and wait again do { + // Reap masters whose RT link died since the last pass. The recv + // below has a MANAGER_INIT_TIMEOUT, so this runs at least every 2 s + // even when the network is silent. + ReapDeadMasters(); + session_params_t net_params; rx_bytes = fSocket.CatchHost(&net_params, sizeof(session_params_t), 0); SessionParamsNToH(&net_params, &host_params); @@ -872,6 +1010,33 @@ namespace Jack return NULL; } + // Dedupe by slave name. A live master for a slave means every + // further SLAVE_AVAILABLE from that name is duplicate discovery + // traffic — the pi announces continuously (~1/s) on the discovery + // group, on both its wired and wifi paths (dual-homed is the + // deployment standard), including while a session is running and + // while one is being established. + // + // ReapDeadMasters() ran at the top of this same Run() pass, so a + // name match here is a *live* master, never a dead one. The two + // things that legitimately end a session — FatalRecvError/ + // FatalSendError on the RT path, KILL_MASTER from the slave — + // remove the master before the next announce is processed. There + // is therefore no case where superseding from the announce path is + // required; every variant tried here (kill-on-announce, kill-if-not- + // yet-synched) turned into a livelock, because the slave's UDP + // socket stays connected to the killed master's port and answers + // every replacement's SETUP with ICMP port-unreachable until its + // own recv timeout (~10 s) restarts it — then the cycle repeats. + // Measured as hundreds of master recreations, a starved RT graph + // and no audio (docs/plan-replug-recovery.md, fault 2). + for (master_list_it_t it = fMasterList.begin(); it != fMasterList.end(); ++it) { + if (strcmp((*it)->fParams.fName, params.fName) == 0) { + jack_log("NetMaster '%s' already live; ignoring announce", params.fName); + return NULL; + } + } + //settings fSocket.GetName(params.fMasterNetName); params.fID = ++fGlobalID; @@ -898,6 +1063,31 @@ namespace Jack jack_info("Takes physical %d MIDI output(s) for slave", params.fReturnMidiChannels); } + // Pin the master's command and RT sockets to one interface. A host + // with a link-local address on both a direct cable and wifi has two + // routes to the slave; without a pin the master's reply can leave by + // the wrong one. Use the interface this announce arrived on, latched + // once and then kept (see fBoundIF). fSocket is copied into the + // master below, so SetBoundIF must run before the copy. + int pin_if = 0; + if (fPinFromEnv) { + pin_if = fSocket.IFNameToIndex(fMulticastIF); + if (pin_if == 0) { + jack_error("netJACK: interface '%s' not found; master egress not pinned", fMulticastIF); + } + } else { + if (fBoundIF != 0 && !fSocket.IFIndexValid(fBoundIF)) { + jack_info("netJACK: pinned interface (ifindex %d) is gone; re-latching", fBoundIF); + fBoundIF = 0; + } + if (fBoundIF == 0 && fSocket.GetLastRecvIF() != 0) { + fBoundIF = fSocket.GetLastRecvIF(); + jack_info("netJACK: pinning masters to ifindex %d", fBoundIF); + } + pin_if = fBoundIF; + } + fSocket.SetBoundIF(pin_if); + //create a new master and add it to the list JackNetMaster* master = new JackNetMaster(fSocket, params, fMulticastIP); if (master->Init(fAutoConnect)) { @@ -925,22 +1115,52 @@ namespace Jack return it; } + // Remove one master from the list and destroy it. Caller holds no lock — + // the manager is single-threaded apart from the RT process callbacks, and + // those never touch fMasterList. + void JackNetMasterManager::RemoveMaster(master_list_it_t master_it) + { + JackNetMaster* master = *master_it; + if (fAutoSave) { + fMasterConnectionList[master->fParams.fName].clear(); + master->SaveConnections(fMasterConnectionList[master->fParams.fName]); + } + // Capture the pointer BEFORE erasing: erase() invalidates the iterator, + // so the old "erase(it); delete (*it);" was a use-after-free that + // deleted whatever garbage the stale iterator dereferenced to. + fMasterList.erase(master_it); + delete master; + } + int JackNetMasterManager::KillMaster(session_params_t* params) { jack_log("JackNetMasterManager::KillMaster ID = %u", params->fID); master_list_it_t master_it = FindMaster(params->fID); if (master_it != fMasterList.end()) { - if (fAutoSave) { - fMasterConnectionList[params->fName].clear(); - (*master_it)->SaveConnections(fMasterConnectionList[params->fName]); - } - fMasterList.erase(master_it); - delete (*master_it); + RemoveMaster(master_it); return 1; } return 0; } + + // Reap any master whose RT link has died (FatalRecvError / FatalSendError). + // Called from the manager listener loop, i.e. off the RT thread. This is + // the path that recovers a yanked cable or a hard-killed slave, where the + // multicast KILL_MASTER packet never arrives. + void JackNetMasterManager::ReapDeadMasters() + { + master_list_it_t it = fMasterList.begin(); + while (it != fMasterList.end()) { + if ((*it)->IsDead()) { + jack_info("Reaping dead NetMaster '%s' (ID %u)", (*it)->fParams.fName, (*it)->fParams.fID); + master_list_it_t dead = it++; + RemoveMaster(dead); + } else { + ++it; + } + } + } }//namespace static Jack::JackNetMasterManager* master_manager = NULL; diff --git a/common/JackNetManager.h b/common/JackNetManager.h index 85fab6a3..b75f1b9a 100644 --- a/common/JackNetManager.h +++ b/common/JackNetManager.h @@ -61,6 +61,27 @@ namespace Jack //sync and transport int fLastTransportState; + // Five-second, interval-based diagnostics. These fields are written + // only by the JACK process callback; reporting is deliberately + // rate-limited so the normal cycle does not emit per-packet logs. + uint64_t fDiagCycles = 0; + uint64_t fDiagDataPacketErrors = 0; + uint64_t fDiagSyncPacketErrors = 0; + uint64_t fDiagSocketErrors = 0; + uint64_t fDiagSlowCycles = 0; + uint64_t fDiagMaxProcessUsecs = 0; + uint64_t fDiagMaxSyncSendUsecs = 0; + uint64_t fDiagMaxDataSendUsecs = 0; + uint64_t fDiagMaxSyncRecvUsecs = 0; + uint64_t fDiagMaxDataRecvUsecs = 0; + jack_time_t fDiagLastReportUsecs = 0; + + void RecordDiagnosticStage(uint64_t& max_usecs, + jack_time_t start, + jack_time_t end); + void FinishDiagnosticCycle(jack_time_t start, jack_time_t end); + void ReportDiagnosticsIfDue(jack_time_t now); + //monitoring #ifdef JACK_MONITOR jack_time_t fPeriodUsecs; @@ -116,6 +137,14 @@ namespace Jack // Set from JACK_NETJACK_MULTICAST_IF. Empty = legacy INADDR_ANY // behavior. See posix/JackNetUnixSocket.cpp::JoinMCastGroup. char fMulticastIF[16]; + // Interface index that master sockets pin unicast egress to. + // InitMaster() latches the first SLAVE_AVAILABLE arrival interface + // and keeps it. Do not re-latch on later packets: a stray announce + // on another interface must not move live masters. + int fBoundIF; + // True when JACK_NETJACK_MULTICAST_IF sets the pin. Then fBoundIF + // comes from fMulticastIF on each InitMaster() and no latch runs. + bool fPinFromEnv; JackNetSocket fSocket; jack_native_thread_t fThread; master_list_t fMasterList; @@ -128,6 +157,8 @@ namespace Jack void Run(); JackNetMaster* InitMaster(session_params_t& params); master_list_it_t FindMaster(uint32_t client_id); + void RemoveMaster(master_list_it_t master_it); + void ReapDeadMasters(); int KillMaster(session_params_t* params); int SyncCallback(jack_transport_state_t state, jack_position_t* pos); int CountIO(const char* type, int flags); diff --git a/common/JackResampler.cpp b/common/JackResampler.cpp index 04cc590d..ae33847f 100644 --- a/common/JackResampler.cpp +++ b/common/JackResampler.cpp @@ -19,6 +19,8 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. #include "JackResampler.h" #include "JackError.h" +#include "JackTime.h" +#include #include namespace Jack @@ -45,6 +47,41 @@ void JackRingBuffer::Reset(unsigned int new_size) jack_ringbuffer_read_advance(fRingBuffer, (sizeof(jack_default_audio_sample_t) * new_size/2)); } +namespace { + std::atomic gReadFailureCount(0); + std::atomic gWriteFailureCount(0); + std::atomic gLastReadFailureReport(0); + std::atomic gLastWriteFailureReport(0); + + static_assert(__atomic_always_lock_free(sizeof(uint64_t), 0), "ring failure counters must be lock-free"); + + uint64_t TakeFailureReportCount(std::atomic& count, + std::atomic& last_report) + { + count.fetch_add(1, std::memory_order_relaxed); + jack_time_t now = GetMicroSeconds(); + jack_time_t last = last_report.load(std::memory_order_relaxed); + if (last == 0 || now - last >= 1000000) { + if (last_report.compare_exchange_strong(last, now, + std::memory_order_relaxed, + std::memory_order_relaxed)) { + return count.exchange(0, std::memory_order_relaxed); + } + } + return 0; + } +} + +uint64_t JackRingBuffer::ReadFailureReportCount() +{ + return TakeFailureReportCount(gReadFailureCount, gLastReadFailureReport); +} + +uint64_t JackRingBuffer::WriteFailureReportCount() +{ + return TakeFailureReportCount(gWriteFailureCount, gLastWriteFailureReport); +} + unsigned int JackRingBuffer::ReadSpace() { return (jack_ringbuffer_read_space(fRingBuffer) / sizeof(jack_default_audio_sample_t)); @@ -58,10 +95,12 @@ unsigned int JackRingBuffer::WriteSpace() unsigned int JackRingBuffer::Read(jack_default_audio_sample_t* buffer, unsigned int frames) { size_t len = jack_ringbuffer_read_space(fRingBuffer); - jack_log("JackRingBuffer::Read input available = %ld", len / sizeof(jack_default_audio_sample_t)); if (len < frames * sizeof(jack_default_audio_sample_t)) { - jack_error("JackRingBuffer::Read : producer too slow, missing frames = %d", frames); + uint64_t failure_count = ReadFailureReportCount(); + if (failure_count > 0) { + jack_error("JackRingBuffer::Read : producer too slow, missing frames = %d; failures since last report = %llu", frames, (unsigned long long)failure_count); + } return 0; } else { jack_ringbuffer_read(fRingBuffer, (char*)buffer, frames * sizeof(jack_default_audio_sample_t)); @@ -72,10 +111,12 @@ unsigned int JackRingBuffer::Read(jack_default_audio_sample_t* buffer, unsigned unsigned int JackRingBuffer::Write(jack_default_audio_sample_t* buffer, unsigned int frames) { size_t len = jack_ringbuffer_write_space(fRingBuffer); - jack_log("JackRingBuffer::Write output available = %ld", len / sizeof(jack_default_audio_sample_t)); if (len < frames * sizeof(jack_default_audio_sample_t)) { - jack_error("JackRingBuffer::Write : consumer too slow, skip frames = %d", frames); + uint64_t failure_count = WriteFailureReportCount(); + if (failure_count > 0) { + jack_error("JackRingBuffer::Write : consumer too slow, skip frames = %d; failures since last report = %llu", frames, (unsigned long long)failure_count); + } return 0; } else { jack_ringbuffer_write(fRingBuffer, (char*)buffer, frames * sizeof(jack_default_audio_sample_t)); @@ -86,10 +127,12 @@ unsigned int JackRingBuffer::Write(jack_default_audio_sample_t* buffer, unsigned unsigned int JackRingBuffer::Read(void* buffer, unsigned int bytes) { size_t len = jack_ringbuffer_read_space(fRingBuffer); - jack_log("JackRingBuffer::Read input available = %ld", len); if (len < bytes) { - jack_error("JackRingBuffer::Read : producer too slow, missing bytes = %d", bytes); + uint64_t failure_count = ReadFailureReportCount(); + if (failure_count > 0) { + jack_error("JackRingBuffer::Read : producer too slow, missing bytes = %d; failures since last report = %llu", bytes, (unsigned long long)failure_count); + } return 0; } else { jack_ringbuffer_read(fRingBuffer, (char*)buffer, bytes); @@ -100,10 +143,12 @@ unsigned int JackRingBuffer::Read(void* buffer, unsigned int bytes) unsigned int JackRingBuffer::Write(void* buffer, unsigned int bytes) { size_t len = jack_ringbuffer_write_space(fRingBuffer); - jack_log("JackRingBuffer::Write output available = %ld", len); if (len < bytes) { - jack_error("JackRingBuffer::Write : consumer too slow, skip bytes = %d", bytes); + uint64_t failure_count = WriteFailureReportCount(); + if (failure_count > 0) { + jack_error("JackRingBuffer::Write : consumer too slow, skip bytes = %d; failures since last report = %llu", bytes, (unsigned long long)failure_count); + } return 0; } else { jack_ringbuffer_write(fRingBuffer, (char*)buffer, bytes); diff --git a/common/JackResampler.h b/common/JackResampler.h index 74c75a26..57f4c2e0 100644 --- a/common/JackResampler.h +++ b/common/JackResampler.h @@ -46,6 +46,8 @@ class JackRingBuffer jack_ringbuffer_t* fRingBuffer; unsigned int fRingBufferSize; + uint64_t ReadFailureReportCount(); + uint64_t WriteFailureReportCount(); public: JackRingBuffer(int size = DEFAULT_RB_SIZE); diff --git a/common/wscript b/common/wscript index 178d4c1d..6fb880e3 100644 --- a/common/wscript +++ b/common/wscript @@ -138,6 +138,7 @@ def build(bld): '../macosx/JackMachThread.mm', '../macosx/JackMachSemaphore.mm', '../macosx/JackMachSemaphoreServer.mm', + '../macosx/JackWorkgroup.mm', '../posix/JackSocket.cpp', '../macosx/JackMachTime.c', ] @@ -166,6 +167,10 @@ def build(bld): clientlib = bld(features=['c', 'cxx', 'cxxshlib', 'cshlib']) if bld.env['IS_MACOSX']: clientlib.framework = ['CoreAudio', 'Accelerate'] + # JackWorkgroup.mm calls os_release on an os_workgroup_t, which is an + # os_object and lowers to objc_release. The other .mm files here never + # touch the ObjC runtime, so libobjc was not linked before. + clientlib.env.append_value('LINKFLAGS', ['-lobjc']) clientlib.defines = 'HAVE_CONFIG_H' clientlib.includes = includes clientlib.name = 'clientlib' @@ -250,6 +255,8 @@ def build(bld): serverlib = bld(features=['c', 'cxx', 'cxxshlib', 'cshlib']) if bld.env['IS_MACOSX']: serverlib.framework = ['CoreAudio', 'CoreFoundation', 'Accelerate'] + # See the clientlib note: JackWorkgroup.mm needs the ObjC runtime. + serverlib.env.append_value('LINKFLAGS', ['-lobjc']) serverlib.defines = ['HAVE_CONFIG_H', 'SERVER_SIDE'] serverlib.includes = includes serverlib.name = 'serverlib' diff --git a/macosx/JackWorkgroup.h b/macosx/JackWorkgroup.h new file mode 100644 index 00000000..831c6551 --- /dev/null +++ b/macosx/JackWorkgroup.h @@ -0,0 +1,81 @@ +/* +Copyright (C) 2026 Treefall Sound + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +*/ + +/* + CoreAudio workgroup membership for JACK client realtime threads. + + Every CoreAudio device publishes an os_workgroup_t via + kAudioDevicePropertyIOThreadOSWorkgroup. Threads that join it are scheduled + by the kernel as co-deadline with that device's IO thread, which is + materially stronger protection against unrelated CPU pressure (WindowServer, + a hypervisor) than plain time-constraint realtime alone. + + jackd's coreaudio backend cycle runs on an AudioUnit render callback, so + CoreAudio has already placed *that* thread in the backend device's + workgroup. It does not propagate to the graph's client threads, which jack2 + spawns itself -- so every JACK client (netmanager's per-slave clients + included) runs unprotected and can miss the cycle deadline under load. + This module closes that gap. + + Two undocumented constraints shape the API: + + 1. os_workgroup_join returns EINVAL on a thread that is not realtime by + Mach's definition. THREAD_TIME_CONSTRAINT_POLICY must already be set. + Call this only after JackThread::AcquireSelfRealTime has run. + 2. The join is per-thread and must happen ON the thread that will + participate. A different thread cannot join on its behalf. + + The join token is kept in thread-local storage so callers need no state and + no Apple headers leak into the cross-platform sources. +*/ + +#ifndef __JackWorkgroup__ +#define __JackWorkgroup__ + +#include + +#ifdef __cplusplus +extern "C" +{ +#endif + +/*! +\brief Join the calling thread to the CoreAudio workgroup of device_id. + +\param device_id An AudioDeviceID, valid process-wide. 0 means "not published + yet" and is treated as a no-op failure. +\return 0 on success, non-zero on any failure (no device, property absent, + OS too old, already joined, join refused). + +Never fatal: a failure means the thread keeps the scheduling it already had. +*/ +int JackWorkgroupJoinSelfForDevice(uint32_t device_id); + +/*! +\brief Leave the workgroup this thread joined, if any. Idempotent. + +Must run on the thread that joined, and must run before that thread ends. +Membership does NOT drop by itself: a thread that ends while it is a member +makes libdispatch raise EXC_BREAKPOINT in _os_workgroup_tsd_cleanup, which +stops the process. JackPosixThread::ThreadHandler calls this from a +cancellation handler for that reason. +*/ +void JackWorkgroupLeaveSelf(void); + +#ifdef __cplusplus +} +#endif + +#endif diff --git a/macosx/JackWorkgroup.mm b/macosx/JackWorkgroup.mm new file mode 100644 index 00000000..3926996f --- /dev/null +++ b/macosx/JackWorkgroup.mm @@ -0,0 +1,88 @@ +/* +Copyright (C) 2026 Treefall Sound + +This program is free software; you can redistribute it and/or modify +it under the terms of the GNU Lesser General Public License as published by +the Free Software Foundation; either version 2.1 of the License, or +(at your option) any later version. + +This program is distributed in the hope that it will be useful, +but WITHOUT ANY WARRANTY; without even the implied warranty of +MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +GNU Lesser General Public License for more details. + +*/ + +#include "JackWorkgroup.h" +#include "JackError.h" + +#include +#include + +/* + Per-thread membership state. A JACK client thread joins once, in + SetupRealTime, and holds the membership for its whole life. +*/ +static __thread os_workgroup_t gWorkgroup = NULL; +static __thread os_workgroup_join_token_s gJoinToken; +static __thread int gJoined = 0; + +extern "C" int JackWorkgroupJoinSelfForDevice(uint32_t device_id) +{ + if (gJoined) { + return EALREADY; + } + if (device_id == 0) { + // The backend has not published its device yet, or the backend is not + // coreaudio. Nothing to join; the caller keeps plain realtime. + return ENODEV; + } + + if (__builtin_available(macOS 11.0, *)) { + AudioObjectPropertyAddress addr = { + kAudioDevicePropertyIOThreadOSWorkgroup, + kAudioObjectPropertyScopeGlobal, + kAudioObjectPropertyElementMain + }; + + os_workgroup_t wg = NULL; + UInt32 size = sizeof(wg); + OSStatus err = AudioObjectGetPropertyData((AudioObjectID)device_id, + &addr, 0, NULL, &size, &wg); + if (err != noErr || wg == NULL) { + jack_error("JackWorkgroup: device %u has no IOThreadOSWorkgroup (OSStatus = %d)", + (unsigned)device_id, (int)err); + return err != noErr ? (int)err : ENOENT; + } + + // Requires THREAD_TIME_CONSTRAINT_POLICY on this thread already; the + // caller guarantees it by joining after AcquireSelfRealTime. + int rc = os_workgroup_join(wg, &gJoinToken); + if (rc != 0) { + jack_error("JackWorkgroup: os_workgroup_join failed rc = %d", rc); + os_release(wg); + return rc; + } + + gWorkgroup = wg; + gJoined = 1; + jack_info("JackWorkgroup: realtime thread joined the workgroup of device %u", + (unsigned)device_id); + return 0; + } + + return ENOTSUP; +} + +extern "C" void JackWorkgroupLeaveSelf(void) +{ + if (!gJoined) { + return; + } + if (__builtin_available(macOS 11.0, *)) { + os_workgroup_leave(gWorkgroup, &gJoinToken); + os_release(gWorkgroup); + } + gWorkgroup = NULL; + gJoined = 0; +} diff --git a/macosx/coreaudio/JackCoreAudioDriver.mm b/macosx/coreaudio/JackCoreAudioDriver.mm index 5fbaab72..4c03d941 100644 --- a/macosx/coreaudio/JackCoreAudioDriver.mm +++ b/macosx/coreaudio/JackCoreAudioDriver.mm @@ -2161,7 +2161,14 @@ static void ParseChannelList(const string& list, vector& result, int max_ch if (AddListeners() < 0) { goto error; } - + + // Publish the device for client realtime threads. + // Our own cycle is an AudioUnit render callback, so CoreAudio has + // already put *this* thread in that workgroup; the graph's client + // threads are spawned by jack2 and get nothing without this. + GetEngineControl()->fCoreAudioDeviceID = (UInt32)fDeviceID; + jack_info("JackCoreAudioDriver::Open : published CoreAudio device %d for client workgroup joins", fDeviceID); + return noErr; error: diff --git a/posix/JackNetUnixSocket.cpp b/posix/JackNetUnixSocket.cpp index 0c15306c..941b2a3f 100644 --- a/posix/JackNetUnixSocket.cpp +++ b/posix/JackNetUnixSocket.cpp @@ -46,6 +46,10 @@ namespace Jack fSockfd = 0; fPort = 0; fTimeOut = 0; + fMcastIF[0] = '\0'; + fBoundIF = 0; + fRecvIF = false; + fLastRecvIF = 0; fSendAddr.sin_family = AF_INET; fSendAddr.sin_addr.s_addr = htonl(INADDR_ANY); memset(&fSendAddr.sin_zero, 0, 8); @@ -59,6 +63,10 @@ namespace Jack fSockfd = 0; fPort = port; fTimeOut = 0; + fMcastIF[0] = '\0'; + fBoundIF = 0; + fRecvIF = false; + fLastRecvIF = 0; fSendAddr.sin_family = AF_INET; fSendAddr.sin_port = htons(port); inet_aton(ip, &fSendAddr.sin_addr); @@ -76,6 +84,10 @@ namespace Jack fPort = socket.fPort; fSendAddr = socket.fSendAddr; fRecvAddr = socket.fRecvAddr; + strcpy(fMcastIF, socket.fMcastIF); + fBoundIF = socket.fBoundIF; + fRecvIF = false; + fLastRecvIF = 0; } JackNetUnixSocket::~JackNetUnixSocket() @@ -90,6 +102,10 @@ namespace Jack fPort = socket.fPort; fSendAddr = socket.fSendAddr; fRecvAddr = socket.fRecvAddr; + strcpy(fMcastIF, socket.fMcastIF); + fBoundIF = socket.fBoundIF; + fRecvIF = false; + fLastRecvIF = 0; } return *this; } @@ -131,9 +147,41 @@ namespace Jack res = getsockopt(fSockfd, IPPROTO_IP, IP_TOS, &tos, &len); - tos = 46 * 4; // see + tos = 46 * 4; // see res = setsockopt(fSockfd, IPPROTO_IP, IP_TOS, &tos, sizeof(tos)); + // Re-pin the outgoing multicast interface on every fresh socket. The + // slave's reconnect loop tears down and rebuilds this socket on each + // attempt (SendAvailableToMaster -> NewSocket); without re-applying, + // only the very first socket was pinned and a leaked netadapter would + // start announcing over whatever interface holds the default route + // (i.e. Wi-Fi) once the cable's kernel route was deleted. + // + // Fail closed: if a pin was requested (JACK_NETJACK_MULTICAST_IF set) + // and cannot be applied, refuse the socket rather than let the slave + // fall back to the default route. The caller propagates SOCKET_ERROR + // and the reconnect loop retries. Only the adapter/slave path ever + // sets fMcastIF; the master pins via JoinMCastGroup and is unaffected. + if (fMcastIF[0] != '\0' && ApplyMulticastIF() == SOCKET_ERROR) { + jack_error("NewSocket: can't pin multicast to '%s' (%s) - refusing socket", + fMcastIF, strerror(NET_ERROR_CODE)); + Close(); + return SOCKET_ERROR; + } + + // Fail closed, as for fMcastIF: a master whose egress cannot be pinned + // must not fall back to the default route. The caller retries. + if (fBoundIF != 0 && ApplyBoundIF() == SOCKET_ERROR) { + jack_error("NewSocket: can't pin egress to ifindex %d (%s) - refusing socket", + fBoundIF, strerror(NET_ERROR_CODE)); + Close(); + return SOCKET_ERROR; + } + + if (fRecvIF) { + ApplyRecvIF(); + } + return fSockfd; } @@ -308,12 +356,34 @@ namespace Jack int JackNetUnixSocket::SetMulticastIF(const char* ifname) { - if (ifname == NULL || ifname[0] == '\0') { + // Record the choice. Callers set this before NewSocket(), so the + // actual setsockopt has to be deferred (and re-done on every socket). + if (ifname == NULL) { + fMcastIF[0] = '\0'; + } else { + strncpy(fMcastIF, ifname, sizeof(fMcastIF) - 1); + fMcastIF[sizeof(fMcastIF) - 1] = '\0'; + } + + // Pin unicast egress to the same interface. IP_MULTICAST_IF only + // steers multicast sendto(); the slave's later connect() to the + // master and its RT stream are unicast and would otherwise follow + // the default route. + fBoundIF = (fMcastIF[0] != '\0') ? (int)if_nametoindex(fMcastIF) : 0; + + if (fMcastIF[0] == '\0') { // No env var set; let the kernel pick the outgoing interface for // multicast sendto via its multicast route table. return 0; } + // Apply now too, if the socket already exists. + return (fSockfd > 0) ? ApplyMulticastIF() : 0; + } + + int JackNetUnixSocket::ApplyMulticastIF() + { + const char* ifname = fMcastIF; int idx = if_nametoindex(ifname); if (idx == 0) { NET_ERROR_CODE = ENODEV; @@ -342,6 +412,66 @@ namespace Jack #endif } + int JackNetUnixSocket::SetBoundIF(int ifindex) + { + fBoundIF = ifindex; + return 0; + } + + int JackNetUnixSocket::ApplyBoundIF() + { + if (!IFIndexValid(fBoundIF)) { + NET_ERROR_CODE = ENXIO; + return SOCKET_ERROR; + } +#if defined(__linux__) + #if defined(IP_UNICAST_IF) + // IP_UNICAST_IF takes the index as an int in network byte order. + int idx = htonl(fBoundIF); + return SetOption(IPPROTO_IP, IP_UNICAST_IF, &idx, sizeof(idx)); + #else + // No socket-level unicast pin. The kernel route table still applies. + return 0; + #endif +#else + // macOS / BSD: IP_BOUND_IF takes the index as an unsigned int. + unsigned int idx = fBoundIF; + return SetOption(IPPROTO_IP, IP_BOUND_IF, &idx, sizeof(idx)); +#endif + } + + int JackNetUnixSocket::SetRecvIF() + { + fRecvIF = true; + return (fSockfd > 0) ? ApplyRecvIF() : 0; + } + + int JackNetUnixSocket::ApplyRecvIF() + { + int on = 1; +#if defined(IP_RECVPKTINFO) + return SetOption(IPPROTO_IP, IP_RECVPKTINFO, &on, sizeof(on)); +#else + return SetOption(IPPROTO_IP, IP_PKTINFO, &on, sizeof(on)); +#endif + } + + int JackNetUnixSocket::GetLastRecvIF() + { + return fLastRecvIF; + } + + int JackNetUnixSocket::IFNameToIndex(const char* ifname) + { + return (ifname && ifname[0]) ? (int)if_nametoindex(ifname) : 0; + } + + bool JackNetUnixSocket::IFIndexValid(int ifindex) + { + char name[IF_NAMESIZE]; + return ifindex > 0 && if_indextoname(ifindex, name) != NULL; + } + //options************************************************************************************************************ int JackNetUnixSocket::SetOption(int level, int optname, const void* optval, socklen_t optlen) { @@ -543,10 +673,42 @@ namespace Jack } #endif int res; - if ((res = recvfrom(fSockfd, buffer, nbytes, flags, reinterpret_cast(&fSendAddr), &addr_len)) < 0) { + + if (!fRecvIF) { + if ((res = recvfrom(fSockfd, buffer, nbytes, flags, reinterpret_cast(&fSendAddr), &addr_len)) < 0) { + jack_log("CatchHost fd = %ld err = %s", fSockfd, strerror(errno)); + } + return res; + } + + struct iovec iov; + iov.iov_base = buffer; + iov.iov_len = nbytes; + + char control[CMSG_SPACE(sizeof(struct in_pktinfo))]; + struct msghdr msg; + memset(&msg, 0, sizeof(msg)); + msg.msg_name = &fSendAddr; + msg.msg_namelen = sizeof(socket_address_t); + msg.msg_iov = &iov; + msg.msg_iovlen = 1; + msg.msg_control = control; + msg.msg_controllen = sizeof(control); + + fLastRecvIF = 0; + if ((res = recvmsg(fSockfd, &msg, flags)) < 0) { jack_log("CatchHost fd = %ld err = %s", fSockfd, strerror(errno)); + return res; } - return res; + + for (struct cmsghdr* cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) { + if (cmsg->cmsg_level == IPPROTO_IP && cmsg->cmsg_type == IP_PKTINFO) { + struct in_pktinfo info; + memcpy(&info, CMSG_DATA(cmsg), sizeof(info)); + fLastRecvIF = info.ipi_ifindex; + } + } + return res; } net_error_t JackNetUnixSocket::GetError() diff --git a/posix/JackNetUnixSocket.h b/posix/JackNetUnixSocket.h index 28502d02..de28fb0c 100644 --- a/posix/JackNetUnixSocket.h +++ b/posix/JackNetUnixSocket.h @@ -47,6 +47,26 @@ namespace Jack struct sockaddr_in fSendAddr; struct sockaddr_in fRecvAddr; + + // Interface name to pin outgoing multicast to. Stored here rather + // than applied immediately: callers (JackNetAdapter) set it before + // the socket exists, so the old code did the setsockopt on fd 0 and + // never re-applied it. Empty = legacy behavior. Re-applied by every + // NewSocket(). + char fMcastIF[16]; + int ApplyMulticastIF(); + + // Interface index for unicast egress. 0 disables the pin. + // NewSocket() applies it. The copy constructor must copy it. + int fBoundIF; + int ApplyBoundIF(); + + // If true, CatchHost() uses recvmsg() and records the arrival + // interface index in fLastRecvIF. If false, CatchHost() uses + // recvfrom(). + bool fRecvIF; + int fLastRecvIF; + int ApplyRecvIF(); #if defined(__sun__) || defined(sun) int WaitRead(); int WaitWrite(); @@ -101,6 +121,19 @@ namespace Jack // preserved when the env var is unset. int SetMulticastIF(const char* ifname); + // Pin unicast egress (connect(), send()) to an interface index. + // Pass 0 to remove the pin. NewSocket() applies the pin. + int SetBoundIF(int ifindex); + // Make CatchHost() record the arrival interface index. + int SetRecvIF(); + // Arrival interface index of the last CatchHost() datagram. + // 0 if not known. + int GetLastRecvIF(); + // Interface index for the given name. 0 if the name is unknown. + int IFNameToIndex(const char* ifname); + // True if the interface index refers to a current interface. + bool IFIndexValid(int ifindex); + //options management int SetOption(int level, int optname, const void* optval, socklen_t optlen); int GetOption(int level, int optname, void* optval, socklen_t* optlen); diff --git a/posix/JackPosixThread.cpp b/posix/JackPosixThread.cpp index 86bf729d..19b043e8 100644 --- a/posix/JackPosixThread.cpp +++ b/posix/JackPosixThread.cpp @@ -25,6 +25,7 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. #include // for memset #include // for _POSIX_PRIORITY_SCHEDULING check + //#define JACK_SCHED_POLICY SCHED_RR #define JACK_SCHED_POLICY SCHED_FIFO @@ -35,6 +36,45 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. namespace Jack { +static __thread void (*gThreadExitHook)(void) = NULL; + +SERVER_EXPORT void JackSetThreadExitHook(void (*hook)(void)) +{ + gThreadExitHook = hook; +} + +/* + Run the thread's exit hook before the thread ends. + + On macOS JackClient::SetupRealTime joins the backend device's CoreAudio + workgroup on this thread and sets the hook to the matching leave. A thread + that ends while it is a member makes libdispatch stop the process: + _os_workgroup_tsd_cleanup raises EXC_BREAKPOINT during the pthread_exit of + that thread. + + JackClient::SetupRealTime joins the backend device's workgroup on this + thread. A thread that ends while it is a member makes libdispatch stop the + process: _os_workgroup_tsd_cleanup raises EXC_BREAKPOINT during the + pthread_exit of that thread. The comment in JackWorkgroup.h that says + membership drops by itself at thread exit is wrong. + + This is a cancellation handler because cancellation is how the thread + usually ends. JackEngine::ClientDeactivate cancels the client thread, the + cancel is taken at the condition wait in JackPosixProcessSync, and no + ordinary return path runs. A cable fault does exactly this to the netJACK2 + master client, and jackd stopped with SIGTRAP on every cable fault. + + Cancellation handlers run before the thread-specific-data destructors, thus + the membership is gone before libdispatch looks at it. +*/ +static void JackThreadExitCleanup(void* /*arg*/) +{ + if (gThreadExitHook != NULL) { + gThreadExitHook(); + gThreadExitHook = NULL; + } +} + void* JackPosixThread::ThreadHandler(void* arg) { JackPosixThread* obj = (JackPosixThread*)arg; @@ -49,18 +89,39 @@ void* JackPosixThread::ThreadHandler(void* arg) jack_log("JackPosixThread::ThreadHandler : start"); obj->fStatus = kIniting; + /* + Init joins the workgroup, thus the handler must cover Init as well as + the loop. pthread_cleanup_push and pthread_cleanup_pop are one lexical + block, and a return between them leaves a handler on a dead frame. + Thus the init failure exits after the pop, and init_ok is declared + before the push: the block ends at the pop. + + The hook covers Init as well as the loop, because Init is where + JackClient::SetupRealTime joins the workgroup and sets the hook. + */ + bool init_ok = false; + + pthread_cleanup_push(JackThreadExitCleanup, NULL); + // Call Init method - if (!runnable->Init()) { + init_ok = runnable->Init(); + + if (init_ok) { + obj->fStatus = kRunning; + + // If Init succeed, start the thread loop + bool res = true; + while (obj->fStatus == kRunning && res) { + res = runnable->Execute(); + } + } else { jack_error("Thread init fails: thread quits"); - return 0; } - obj->fStatus = kRunning; + pthread_cleanup_pop(1); - // If Init succeed, start the thread loop - bool res = true; - while (obj->fStatus == kRunning && res) { - res = runnable->Execute(); + if (!init_ok) { + return 0; } jack_log("JackPosixThread::ThreadHandler : exit"); diff --git a/posix/JackPosixThread.h b/posix/JackPosixThread.h index 599bf498..2c87688d 100644 --- a/posix/JackPosixThread.h +++ b/posix/JackPosixThread.h @@ -27,6 +27,20 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. namespace Jack { +/*! +\brief Register a function to run on this thread just before it ends. + +Thread-local: the hook belongs to the thread that sets it, and runs from a +cancellation handler in JackPosixThread::ThreadHandler. Cancellation is the +usual way a client thread ends, and cancellation handlers run before the +thread-specific-data destructors. + +This exists so that JackPosixThread needs no link-time dependency on the +macOS-only JackWorkgroup sources: this file is compiled into libraries that +do not contain them. NULL, the default, means there is nothing to do. +*/ +SERVER_EXPORT void JackSetThreadExitHook(void (*hook)(void)); + /* use 512KB stack per thread - the default is way too high to be feasible * with mlockall() on many systems */ #define THREAD_STACK 524288 diff --git a/windows/JackNetWinSocket.cpp b/windows/JackNetWinSocket.cpp index ff9e51ed..b35a96c4 100644 --- a/windows/JackNetWinSocket.cpp +++ b/windows/JackNetWinSocket.cpp @@ -301,6 +301,14 @@ namespace Jack return 0; } + // No-op on Windows. fBoundIF stays 0 in the manager, so masters keep the + // legacy route-table egress. + int JackNetWinSocket::SetBoundIF(int ifindex) { (void)ifindex; return 0; } + int JackNetWinSocket::SetRecvIF() { return 0; } + int JackNetWinSocket::GetLastRecvIF() { return 0; } + int JackNetWinSocket::IFNameToIndex(const char* ifname) { (void)ifname; return 0; } + bool JackNetWinSocket::IFIndexValid(int ifindex) { (void)ifindex; return false; } + //options************************************************************************************************************ int JackNetWinSocket::SetOption(int level, int optname, const void* optval, SOCKLEN optlen) { diff --git a/windows/JackNetWinSocket.h b/windows/JackNetWinSocket.h index e6191a38..61abb916 100644 --- a/windows/JackNetWinSocket.h +++ b/windows/JackNetWinSocket.h @@ -93,6 +93,14 @@ namespace Jack // No-op on Windows; the legacy kernel-picks behavior is preserved. int SetMulticastIF(const char* ifname); + // Unicast egress interface pinning. No-op on Windows; the socket + // keeps the legacy kernel-picks-route behavior. + int SetBoundIF(int ifindex); + int SetRecvIF(); + int GetLastRecvIF(); + int IFNameToIndex(const char* ifname); + bool IFIndexValid(int ifindex); + //options management int SetOption(int level, int optname, const void* optval, SOCKLEN optlen); int GetOption(int level, int optname, void* optval, SOCKLEN* optlen);