From 35b404cb06cdd3721874d9c3b52ab8e7998c726c Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Thu, 27 Aug 2026 23:50:19 -0400 Subject: [PATCH 01/13] netJACK2: reap dead masters, fix KillMaster UAF, dedupe slaves, pin slave multicast Four defects that together made a departed netJACK2 slave a permanent, silent stall on the master side (see NETJACK-REAPING.md): 1. FatalRecvError/FatalSendError called ThreadExit() from the RT process callback, leaving the JACK client registered with a dead RT thread. Now they set an atomic fDead flag; JackNetMasterManager::Run reaps via the new ReapDeadMasters() off the RT thread. 2. KillMaster dereferenced an erased iterator to delete the master. Extracted RemoveMaster() which captures the pointer before erasing. 3. InitMaster created a fresh master for every SLAVE_AVAILABLE with no existing-slave check, producing pistomp-01/-02 duplicates. It now reaps any master already holding that name first. 4. JackNetAdapter set the multicast interface on fd 0 before the socket existed and NewSocket never re-applied it, so a leaked netadapter announced over Wi-Fi. JackNetUnixSocket now stores the ifname and re-pins it (checked) on every NewSocket(). Builds clean (jack_net, jack_netone). Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_0137tergcV2UfSv81GpryaKT --- NETJACK-REAPING.md | 114 ++++++++++++++++++++++++++++++++++++ common/JackNetInterface.cpp | 19 +++--- common/JackNetInterface.h | 21 +++++-- common/JackNetManager.cpp | 63 ++++++++++++++++++-- common/JackNetManager.h | 2 + posix/JackNetUnixSocket.cpp | 35 ++++++++++- posix/JackNetUnixSocket.h | 8 +++ 7 files changed, 242 insertions(+), 20 deletions(-) create mode 100644 NETJACK-REAPING.md diff --git a/NETJACK-REAPING.md b/NETJACK-REAPING.md new file mode 100644 index 00000000..f9ce084b --- /dev/null +++ b/NETJACK-REAPING.md @@ -0,0 +1,114 @@ +# 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. + +### 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. + +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()`, and +logs an error if the interface is not found. + +## Note + +Defect 1 also returns `SOCKET_ERROR` from the process callback. The server +may deactivate the client for that. This is acceptable: `fRunning` is already +false, so the next cycle returns 0 at once, and the manager reaps the master +within one loop pass. + +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/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..fb973bf9 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 { @@ -157,6 +158,13 @@ namespace Jack int fCurrentCycleOffset; int fMaxCycleOffset; bool 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,10 +191,11 @@ 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), @@ -198,6 +207,10 @@ namespace Jack 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); } }; /** diff --git a/common/JackNetManager.cpp b/common/JackNetManager.cpp index 7d832aa2..6722cc35 100644 --- a/common/JackNetManager.cpp +++ b/common/JackNetManager.cpp @@ -826,6 +826,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 +877,22 @@ namespace Jack return NULL; } + // Dedupe by slave name. A slave that restarts — or a rapid Ethernet + // Audio toggle on the pedal — sends a fresh SLAVE_AVAILABLE while its + // previous master may still be in the list (its KILL_MASTER lost, or + // its link not yet declared dead). Without this, each announcement + // spawns another JACK client and the graph fills with pistomp-01, + // pistomp-02, ... all fighting for the same ports. + for (master_list_it_t it = fMasterList.begin(); it != fMasterList.end(); ) { + if (strcmp((*it)->fParams.fName, params.fName) == 0) { + jack_info("NetMaster '%s' already present — reaping the stale one before re-init", params.fName); + master_list_it_t stale = it++; + RemoveMaster(stale); + } else { + ++it; + } + } + //settings fSocket.GetName(params.fMasterNetName); params.fID = ++fGlobalID; @@ -925,22 +946,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..18798a6f 100644 --- a/common/JackNetManager.h +++ b/common/JackNetManager.h @@ -128,6 +128,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/posix/JackNetUnixSocket.cpp b/posix/JackNetUnixSocket.cpp index 0c15306c..b6b03d34 100644 --- a/posix/JackNetUnixSocket.cpp +++ b/posix/JackNetUnixSocket.cpp @@ -46,6 +46,7 @@ namespace Jack fSockfd = 0; fPort = 0; fTimeOut = 0; + fMcastIF[0] = '\0'; fSendAddr.sin_family = AF_INET; fSendAddr.sin_addr.s_addr = htonl(INADDR_ANY); memset(&fSendAddr.sin_zero, 0, 8); @@ -59,6 +60,7 @@ namespace Jack fSockfd = 0; fPort = port; fTimeOut = 0; + fMcastIF[0] = '\0'; fSendAddr.sin_family = AF_INET; fSendAddr.sin_port = htons(port); inet_aton(ip, &fSendAddr.sin_addr); @@ -76,6 +78,7 @@ namespace Jack fPort = socket.fPort; fSendAddr = socket.fSendAddr; fRecvAddr = socket.fRecvAddr; + strcpy(fMcastIF, socket.fMcastIF); } JackNetUnixSocket::~JackNetUnixSocket() @@ -90,6 +93,7 @@ namespace Jack fPort = socket.fPort; fSendAddr = socket.fSendAddr; fRecvAddr = socket.fRecvAddr; + strcpy(fMcastIF, socket.fMcastIF); } return *this; } @@ -131,9 +135,20 @@ 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. + if (fMcastIF[0] != '\0' && ApplyMulticastIF() == SOCKET_ERROR) { + jack_error("NewSocket: can't pin multicast to '%s': %s", + fMcastIF, strerror(NET_ERROR_CODE)); + } + return fSockfd; } @@ -308,12 +323,28 @@ 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'; + } + + 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; diff --git a/posix/JackNetUnixSocket.h b/posix/JackNetUnixSocket.h index 28502d02..811b3ff4 100644 --- a/posix/JackNetUnixSocket.h +++ b/posix/JackNetUnixSocket.h @@ -47,6 +47,14 @@ 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(); #if defined(__sun__) || defined(sun) int WaitRead(); int WaitWrite(); From cf82e56e66b17ffca89cbb317d0b01dffd592b61 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 00:05:55 -0400 Subject: [PATCH 02/13] netJACK2: fail closed on multicast pin failure; clarify InitMaster dedupe MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-ups: - NewSocket() now closes the socket and returns SOCKET_ERROR when a requested multicast pin (JACK_NETJACK_MULTICAST_IF) can't be applied, instead of logging and continuing. A slave that can't pin its interface retries in its reconnect loop rather than announcing over the default route, so the "leaked netadapter over Wi-Fi" case is fully contained in the fork and the pi teardown needs no kernel-route choreography. Only the adapter/slave path sets fMcastIF; the master (JoinMCastGroup) is unaffected. All five NewSocket() call sites already check SOCKET_ERROR. - InitMaster dedupe: keep the by-name reap but document that ReapDeadMasters already cleared self-declared-dead masters this pass, so a name match is always a live master being deliberately superseded — commonly one that can't self-declare dead because the restarted slave is now feeding it. Builds clean. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_0137tergcV2UfSv81GpryaKT --- NETJACK-REAPING.md | 19 +++++++++++++++++-- common/JackNetManager.cpp | 19 ++++++++++++++----- posix/JackNetUnixSocket.cpp | 10 +++++++++- 3 files changed, 40 insertions(+), 8 deletions(-) diff --git a/NETJACK-REAPING.md b/NETJACK-REAPING.md index f9ce084b..6dbab7f9 100644 --- a/NETJACK-REAPING.md +++ b/NETJACK-REAPING.md @@ -48,6 +48,12 @@ 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 @@ -89,6 +95,10 @@ erases, then deletes: `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. @@ -96,8 +106,13 @@ this defect. It can be removed once this correction is in the field. `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()`, and -logs an error if the interface is not found. +`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 diff --git a/common/JackNetManager.cpp b/common/JackNetManager.cpp index 6722cc35..700e91fd 100644 --- a/common/JackNetManager.cpp +++ b/common/JackNetManager.cpp @@ -879,13 +879,22 @@ namespace Jack // Dedupe by slave name. A slave that restarts — or a rapid Ethernet // Audio toggle on the pedal — sends a fresh SLAVE_AVAILABLE while its - // previous master may still be in the list (its KILL_MASTER lost, or - // its link not yet declared dead). Without this, each announcement - // spawns another JACK client and the graph fills with pistomp-01, - // pistomp-02, ... all fighting for the same ports. + // previous master may still be in the list. Without this, each + // announcement spawns another JACK client and the graph fills with + // pistomp-01, pistomp-02, ... all fighting for the same ports. + // + // ReapDeadMasters() ran at the top of this same Run() pass, so every + // master that has already declared itself dead (FatalRecvError) is + // gone by now. A name match here is therefore always a *live* master + // that we are deliberately superseding: the old link isn't reachable + // any more but hasn't timed out — commonly because the restarted slave + // is now feeding the old master its packets, so that master would + // never self-declare dead. Reaping by name is the only way out; the + // KILL_MASTER path can't help (one lost multicast packet) and upstream + // FindMaster only matches fID, which a re-announcing slave doesn't have. for (master_list_it_t it = fMasterList.begin(); it != fMasterList.end(); ) { if (strcmp((*it)->fParams.fName, params.fName) == 0) { - jack_info("NetMaster '%s' already present — reaping the stale one before re-init", params.fName); + jack_info("NetMaster '%s' already present — superseding the live one", params.fName); master_list_it_t stale = it++; RemoveMaster(stale); } else { diff --git a/posix/JackNetUnixSocket.cpp b/posix/JackNetUnixSocket.cpp index b6b03d34..13c3bc6c 100644 --- a/posix/JackNetUnixSocket.cpp +++ b/posix/JackNetUnixSocket.cpp @@ -144,9 +144,17 @@ namespace Jack // 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", + jack_error("NewSocket: can't pin multicast to '%s' (%s) - refusing socket", fMcastIF, strerror(NET_ERROR_CODE)); + Close(); + return SOCKET_ERROR; } return fSockfd; From 37e97aa63534eebdad1600ed210c96f931dcc76b Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 00:15:43 -0400 Subject: [PATCH 03/13] netJACK2: initialize fDead in the params constructor too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit JackNetMaster is built through JackNetMasterInterface(params, socket, multicast_ip), which left fDead uninitialized — the atomic was only zeroed in the default constructor. IsDead() on a freshly created master was formally UB; in practice it read stack garbage from the manager thread's ReapDeadMasters() scan. --- common/JackNetInterface.h | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/common/JackNetInterface.h b/common/JackNetInterface.h index fb973bf9..55cf5fa7 100644 --- a/common/JackNetInterface.h +++ b/common/JackNetInterface.h @@ -198,11 +198,12 @@ namespace Jack 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() From 66087c5fcb6bedec53d2e68ff265347db8a01ac2 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 00:16:01 -0400 Subject: [PATCH 04/13] netJACK2 docs: trace the single SOCKET_ERROR return to ground Replace the guessed 'the server may deactivate the client' with the actual path: JackClient::CycleSignalAux sees status != 0 and calls End(), clearing fActive and deactivating the internal client. Graph stops scheduling it; manager reaps within one 2 s loop pass. --- NETJACK-REAPING.md | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/NETJACK-REAPING.md b/NETJACK-REAPING.md index 6dbab7f9..2159c9fc 100644 --- a/NETJACK-REAPING.md +++ b/NETJACK-REAPING.md @@ -116,10 +116,13 @@ not need to add or remove kernel routes. ## Note -Defect 1 also returns `SOCKET_ERROR` from the process callback. The server -may deactivate the client for that. This is acceptable: `fRunning` is already -false, so the next cycle returns 0 at once, and the manager reaps the master -within one loop pass. +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 From 4e34e6bdc68cd9bc10572c7776aa806342bd193f Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 16:29:18 -0400 Subject: [PATCH 05/13] Add colour --- .vscode/settings.json | 12 ++++++++++++ 1 file changed, 12 insertions(+) create mode 100644 .vscode/settings.json 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" + } +} From 2a4520f9d1cd14eaec065e3c980e150b03104b23 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 17:12:35 -0400 Subject: [PATCH 06/13] IF binding --- common/JackNetManager.cpp | 31 +++++++++ common/JackNetManager.h | 8 +++ posix/JackNetUnixSocket.cpp | 127 ++++++++++++++++++++++++++++++++++- posix/JackNetUnixSocket.h | 25 +++++++ windows/JackNetWinSocket.cpp | 8 +++ windows/JackNetWinSocket.h | 8 +++ 6 files changed, 205 insertions(+), 2 deletions(-) diff --git a/common/JackNetManager.cpp b/common/JackNetManager.cpp index 700e91fd..7e1fbb33 100644 --- a/common/JackNetManager.cpp +++ b/common/JackNetManager.cpp @@ -667,9 +667,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 +821,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)); @@ -928,6 +934,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)) { diff --git a/common/JackNetManager.h b/common/JackNetManager.h index 18798a6f..5318ba12 100644 --- a/common/JackNetManager.h +++ b/common/JackNetManager.h @@ -116,6 +116,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; diff --git a/posix/JackNetUnixSocket.cpp b/posix/JackNetUnixSocket.cpp index 13c3bc6c..941b2a3f 100644 --- a/posix/JackNetUnixSocket.cpp +++ b/posix/JackNetUnixSocket.cpp @@ -47,6 +47,9 @@ namespace Jack 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); @@ -61,6 +64,9 @@ namespace Jack 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); @@ -79,6 +85,9 @@ namespace Jack fSendAddr = socket.fSendAddr; fRecvAddr = socket.fRecvAddr; strcpy(fMcastIF, socket.fMcastIF); + fBoundIF = socket.fBoundIF; + fRecvIF = false; + fLastRecvIF = 0; } JackNetUnixSocket::~JackNetUnixSocket() @@ -94,6 +103,9 @@ namespace Jack fSendAddr = socket.fSendAddr; fRecvAddr = socket.fRecvAddr; strcpy(fMcastIF, socket.fMcastIF); + fBoundIF = socket.fBoundIF; + fRecvIF = false; + fLastRecvIF = 0; } return *this; } @@ -157,6 +169,19 @@ namespace Jack 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; } @@ -340,6 +365,12 @@ namespace Jack 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. @@ -381,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) { @@ -582,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; + } + + 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; + return res; } net_error_t JackNetUnixSocket::GetError() diff --git a/posix/JackNetUnixSocket.h b/posix/JackNetUnixSocket.h index 811b3ff4..de28fb0c 100644 --- a/posix/JackNetUnixSocket.h +++ b/posix/JackNetUnixSocket.h @@ -55,6 +55,18 @@ namespace Jack // 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(); @@ -109,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/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); From 22cac4fc4d98eb05724d9cb0a8adf844fbbd4dad Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 17:29:22 -0400 Subject: [PATCH 07/13] netJACK2: catch ChangeLog up to branch HEAD; retire sastraxi naming ChangeLog.rst Unreleased section was missing the master-reaping / KillMaster UAF / name-dedupe / fDead-init / fail-closed / unicast-egress-pin work. Add them and rename the header to the TreeFallSound fork. build-macos-pkg.sh: stamp com.treefallsound.jack2 and +treefall.N, matching the remote rename. Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01KRptcXkk6KFdX1KzPnbak9 --- ChangeLog.rst | 36 +++++++++++++++++++++++++++++++++++- build-macos-pkg.sh | 8 ++++---- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/ChangeLog.rst b/ChangeLog.rst index fde1d3ef..9c99de25 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 @@ -32,6 +32,40 @@ ChangeLog 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/build-macos-pkg.sh b/build-macos-pkg.sh index 9a63bb52..82c9ed36 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,7 +77,7 @@ 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 From 582fce929940273bce08ed0368a7737af3627807 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 18:20:22 -0400 Subject: [PATCH 08/13] Simpler --- ChangeLog.rst | 2 +- build-macos-pkg.sh | 19 ++++++++----------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/ChangeLog.rst b/ChangeLog.rst index 9c99de25..d1ae4ed0 100644 --- a/ChangeLog.rst +++ b/ChangeLog.rst @@ -27,7 +27,7 @@ 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 diff --git a/build-macos-pkg.sh b/build-macos-pkg.sh index 82c9ed36..d52c67e0 100755 --- a/build-macos-pkg.sh +++ b/build-macos-pkg.sh @@ -80,21 +80,19 @@ echo "==> build the .pkg" 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" From 9bd453dae4f411002e657e1f080175b313431693 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 21:52:02 -0400 Subject: [PATCH 09/13] Close failed sockets instantly --- common/JackNetAPI.cpp | 4 ++ common/JackNetInterface.h | 11 +++++- common/JackNetManager.cpp | 82 +++++++++++++++++++++++++++------------ 3 files changed, 71 insertions(+), 26 deletions(-) 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.h b/common/JackNetInterface.h index 55cf5fa7..8073e89f 100644 --- a/common/JackNetInterface.h +++ b/common/JackNetInterface.h @@ -157,7 +157,10 @@ 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 @@ -212,6 +215,12 @@ namespace Jack // 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 7e1fbb33..60eda462 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); } @@ -883,28 +913,30 @@ namespace Jack return NULL; } - // Dedupe by slave name. A slave that restarts — or a rapid Ethernet - // Audio toggle on the pedal — sends a fresh SLAVE_AVAILABLE while its - // previous master may still be in the list. Without this, each - // announcement spawns another JACK client and the graph fills with - // pistomp-01, pistomp-02, ... all fighting for the same ports. + // 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 every - // master that has already declared itself dead (FatalRecvError) is - // gone by now. A name match here is therefore always a *live* master - // that we are deliberately superseding: the old link isn't reachable - // any more but hasn't timed out — commonly because the restarted slave - // is now feeding the old master its packets, so that master would - // never self-declare dead. Reaping by name is the only way out; the - // KILL_MASTER path can't help (one lost multicast packet) and upstream - // FindMaster only matches fID, which a re-announcing slave doesn't have. - for (master_list_it_t it = fMasterList.begin(); it != fMasterList.end(); ) { + // 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_info("NetMaster '%s' already present — superseding the live one", params.fName); - master_list_it_t stale = it++; - RemoveMaster(stale); - } else { - ++it; + jack_log("NetMaster '%s' already live; ignoring announce", params.fName); + return NULL; } } From d48aaa0116d568c2141cfb41bc0d8ed2d3b7f572 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Fri, 28 Aug 2026 22:53:50 -0400 Subject: [PATCH 10/13] macOS: join client RT threads to the backend device's CoreAudio workgroup jackd's coreaudio backend cycle runs on an AudioUnit render callback, so CoreAudio has already placed that thread in the backend device's os_workgroup. It never propagates to the graph's client threads, which jack2 spawns itself -- those get THREAD_TIME_CONSTRAINT_POLICY and nothing else. On Apple Silicon that is not enough: an unjoined realtime thread can still be descheduled by WindowServer immediately before the cycle deadline. netJACK2's master client is the visible casualty. Under GUI load it misses its slot and the rest of the graph xruns behind it: JackEngine::XRun: client = pistomp was not finished, state = Running Measured on an M1 Pro driving a 4K panel, a mission-control swipe (full repaint of every window) produced xruns at hundreds per second before this change and roughly 100 over 15 seconds after. The audible result goes from dropout to a barely perceptible spike. Implementation: - macosx/JackWorkgroup.{h,mm}: fetch kAudioDevicePropertyIOThreadOSWorkgroup for a device and join the calling thread. The join token lives in thread-local storage, so no Apple headers leak into common/. - JackCoreAudioDriver::Open publishes its AudioDeviceID into JackEngineControl. AudioObjectIDs are valid machine-wide, so client processes need no UID string and libjack needs no CoreFoundation. - JackClient::SetupRealTime joins after AcquireSelfRealTime. Two undocumented constraints force that placement: os_workgroup_join returns EINVAL on a thread Mach does not consider realtime, and a join cannot be performed on another thread's behalf. - JACK_NO_WORKGROUP lets a client process opt out, for a client that already holds a different device's membership. Failure is never fatal. A client that cannot join keeps exactly the realtime scheduling it had before. JackEngineControl gained a field, so JACK_PROTOCOL_VERSION moves 9 -> 10: a mismatched jackd/libjack pair must refuse to talk rather than misinterpret the shared layout. libobjc is now linked into clientlib and serverlib. os_release on an os_workgroup_t lowers to objc_release, and the other .mm sources in those libraries never touched the ObjC runtime. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DHztJXcFXtPMyPGbtKKLgC --- common/JackClient.cpp | 32 +++++++++ common/JackConstants.h | 2 +- common/JackEngineControl.h | 8 +++ common/wscript | 7 ++ macosx/JackWorkgroup.h | 78 ++++++++++++++++++++++ macosx/JackWorkgroup.mm | 88 +++++++++++++++++++++++++ macosx/coreaudio/JackCoreAudioDriver.mm | 9 ++- 7 files changed, 222 insertions(+), 2 deletions(-) create mode 100644 macosx/JackWorkgroup.h create mode 100644 macosx/JackWorkgroup.mm diff --git a/common/JackClient.cpp b/common/JackClient.cpp index 74a9dca8..8aa9a788 100644 --- a/common/JackClient.cpp +++ b/common/JackClient.cpp @@ -29,6 +29,11 @@ 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 +#endif + #include #include #include @@ -557,6 +562,33 @@ 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) { + JackWorkgroupJoinSelfForDevice(GetEngineControl()->fCoreAudioDeviceID); + } 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/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/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..66239ae4 --- /dev/null +++ b/macosx/JackWorkgroup.h @@ -0,0 +1,78 @@ +/* +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. Membership also drops when the thread +exits, so this is only needed for a clean teardown. +*/ +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: From 33b4c28a4b7ac5933212cb6d5e20c7dd7acfe07f Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Sat, 29 Aug 2026 20:22:23 -0400 Subject: [PATCH 11/13] macOS: leave the CoreAudio workgroup before the thread ends; ignore SIGPIPE Two faults that stopped jackd on every netJACK2 peer loss. A cable pull on a pi-Stomp link killed the server, and everything downstream of that -- the daemon restart storm, the LaunchAgent restart, a full cold start -- was fallout. Recovery took 80-140 s; it is now bounded by the interface's own IPv4 link-local probing. 1. The workgroup join had no leave. d48aaa01 joins the backend device's os_workgroup on each client's realtime thread. os_workgroup membership does not drop by itself at thread exit: libdispatch raises EXC_BREAKPOINT in _os_workgroup_tsd_cleanup during pthread_exit. JackWorkgroupLeaveSelf existed for this and was never called, and its header comment said the leave was only needed for tidiness. JackEngine::ClientDeactivate cancels the client thread, and the cancel is taken at the condition wait in JackPosixProcessSync, so no ordinary return path runs and a call at the end of the run loop would never execute. The leave therefore runs from a pthread_cleanup_push handler in JackPosixThread::ThreadHandler; cancellation handlers run before the thread-specific-data destructors, which is the ordering that matters. The hook is thread-local and set through JackSetThreadExitHook rather than called directly, because JackPosixThread.cpp is compiled into libraries that do not contain JackWorkgroup.mm -- netlib fails to link a direct reference, and weak_import does not help for a symbol absent at static link time. SetupRealTime registers it only when the join succeeded. init_ok is declared before pthread_cleanup_push and the init-failure path returns after the pop: the macros are one lexical block, and a return between them leaves a handler on a dead frame. 2. SIGPIPE stopped the server. SIGPIPE is in the sigwait set and had no case in jackctl_wait_signals, so it reached "default: waiting = false" and began shutdown -- for a write to a peer that went away, which is routine for a server doing network I/O and which the caller already handles (netJACK2 logs "connection lost" and drops the master). The shutdown then hung in ClientDeactivate for the client that had just lost its peer, leaving jackd neither running nor exited. It now logs and keeps waiting. The signal stays blocked in every thread, so writes still return EPIPE and nothing else about signal handling changes. Fault 2 was only reachable once fault 1 was fixed: before that, jackd crashed first. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ABgaVgUV7CjDt9arjy4vNx --- common/JackClient.cpp | 10 +++++- common/JackControlAPI.cpp | 13 +++++++ macosx/JackWorkgroup.h | 7 ++-- posix/JackPosixThread.cpp | 75 +++++++++++++++++++++++++++++++++++---- posix/JackPosixThread.h | 14 ++++++++ 5 files changed, 109 insertions(+), 10 deletions(-) diff --git a/common/JackClient.cpp b/common/JackClient.cpp index 8aa9a788..57209c09 100644 --- a/common/JackClient.cpp +++ b/common/JackClient.cpp @@ -31,6 +31,7 @@ Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. #ifdef __APPLE__ #include "JackWorkgroup.h" +#include "JackPosixThread.h" #include #endif @@ -584,7 +585,14 @@ void JackClient::SetupRealTime() it already had, which is the pre-existing behaviour. */ if (getenv("JACK_NO_WORKGROUP") == NULL) { - JackWorkgroupJoinSelfForDevice(GetEngineControl()->fCoreAudioDeviceID); + 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"); } 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/macosx/JackWorkgroup.h b/macosx/JackWorkgroup.h index 66239ae4..831c6551 100644 --- a/macosx/JackWorkgroup.h +++ b/macosx/JackWorkgroup.h @@ -66,8 +66,11 @@ int JackWorkgroupJoinSelfForDevice(uint32_t device_id); /*! \brief Leave the workgroup this thread joined, if any. Idempotent. -Must run on the thread that joined. Membership also drops when the thread -exits, so this is only needed for a clean teardown. +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); 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 From 50a480176f486d72779fc6ddbc72f7abf7a5e24b Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Mon, 31 Aug 2026 15:36:06 -0400 Subject: [PATCH 12/13] jack2: rate-limit realtime ring failure logs --- common/JackAudioAdapterInterface.cpp | 20 +++- common/JackAudioAdapterInterface.h | 12 ++- common/JackLibSampleRateResampler.cpp | 36 ------- common/JackNetManager.cpp | 139 ++++++++++++++++++++++---- common/JackNetManager.h | 21 ++++ common/JackResampler.cpp | 53 ++++++++-- common/JackResampler.h | 6 ++ 7 files changed, 216 insertions(+), 71 deletions(-) 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/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/JackNetManager.cpp b/common/JackNetManager.cpp index 60eda462..a05f5804 100644 --- a/common/JackNetManager.cpp +++ b/common/JackNetManager.cpp @@ -478,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 @@ -542,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; @@ -579,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; } diff --git a/common/JackNetManager.h b/common/JackNetManager.h index 5318ba12..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; diff --git a/common/JackResampler.cpp b/common/JackResampler.cpp index 04cc590d..f55474ed 100644 --- a/common/JackResampler.cpp +++ b/common/JackResampler.cpp @@ -19,12 +19,13 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. #include "JackResampler.h" #include "JackError.h" +#include "JackTime.h" #include namespace Jack { -JackRingBuffer::JackRingBuffer(int size):fRingBufferSize(size) +JackRingBuffer::JackRingBuffer(int size):fRingBufferSize(size), fReadFailureCount(0), fWriteFailureCount(0), fLastReadFailureReport(0), fLastWriteFailureReport(0) { fRingBuffer = jack_ringbuffer_create(sizeof(jack_default_audio_sample_t) * fRingBufferSize); Reset(fRingBufferSize); @@ -45,6 +46,32 @@ void JackRingBuffer::Reset(unsigned int new_size) jack_ringbuffer_read_advance(fRingBuffer, (sizeof(jack_default_audio_sample_t) * new_size/2)); } +uint64_t JackRingBuffer::ReadFailureReportCount() +{ + ++fReadFailureCount; + jack_time_t now = GetMicroSeconds(); + if (fLastReadFailureReport != 0 && now - fLastReadFailureReport < 1000000) { + return 0; + } + fLastReadFailureReport = now; + uint64_t count = fReadFailureCount; + fReadFailureCount = 0; + return count; +} + +uint64_t JackRingBuffer::WriteFailureReportCount() +{ + ++fWriteFailureCount; + jack_time_t now = GetMicroSeconds(); + if (fLastWriteFailureReport != 0 && now - fLastWriteFailureReport < 1000000) { + return 0; + } + fLastWriteFailureReport = now; + uint64_t count = fWriteFailureCount; + fWriteFailureCount = 0; + return count; +} + unsigned int JackRingBuffer::ReadSpace() { return (jack_ringbuffer_read_space(fRingBuffer) / sizeof(jack_default_audio_sample_t)); @@ -58,10 +85,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 +101,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 +117,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 +133,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..8ce00755 100644 --- a/common/JackResampler.h +++ b/common/JackResampler.h @@ -45,7 +45,13 @@ class JackRingBuffer jack_ringbuffer_t* fRingBuffer; unsigned int fRingBufferSize; + uint64_t fReadFailureCount; + uint64_t fWriteFailureCount; + jack_time_t fLastReadFailureReport; + jack_time_t fLastWriteFailureReport; + uint64_t ReadFailureReportCount(); + uint64_t WriteFailureReportCount(); public: JackRingBuffer(int size = DEFAULT_RB_SIZE); From e799bc04d899f87f1293f9612cb2d6a851657ff8 Mon Sep 17 00:00:00 2001 From: Cam Gorrie Date: Mon, 31 Aug 2026 15:40:29 -0400 Subject: [PATCH 13/13] jack2: cap ring reports per process --- common/JackResampler.cpp | 48 ++++++++++++++++++++++++---------------- common/JackResampler.h | 4 ---- 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/common/JackResampler.cpp b/common/JackResampler.cpp index f55474ed..ae33847f 100644 --- a/common/JackResampler.cpp +++ b/common/JackResampler.cpp @@ -20,12 +20,13 @@ Foundation, Inc., 675 Mass Ave, Cambridge, MA 02139, USA. #include "JackResampler.h" #include "JackError.h" #include "JackTime.h" +#include #include namespace Jack { -JackRingBuffer::JackRingBuffer(int size):fRingBufferSize(size), fReadFailureCount(0), fWriteFailureCount(0), fLastReadFailureReport(0), fLastWriteFailureReport(0) +JackRingBuffer::JackRingBuffer(int size):fRingBufferSize(size) { fRingBuffer = jack_ringbuffer_create(sizeof(jack_default_audio_sample_t) * fRingBufferSize); Reset(fRingBufferSize); @@ -46,30 +47,39 @@ void JackRingBuffer::Reset(unsigned int new_size) jack_ringbuffer_read_advance(fRingBuffer, (sizeof(jack_default_audio_sample_t) * new_size/2)); } -uint64_t JackRingBuffer::ReadFailureReportCount() -{ - ++fReadFailureCount; - jack_time_t now = GetMicroSeconds(); - if (fLastReadFailureReport != 0 && now - fLastReadFailureReport < 1000000) { +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; } - fLastReadFailureReport = now; - uint64_t count = fReadFailureCount; - fReadFailureCount = 0; - return count; +} + +uint64_t JackRingBuffer::ReadFailureReportCount() +{ + return TakeFailureReportCount(gReadFailureCount, gLastReadFailureReport); } uint64_t JackRingBuffer::WriteFailureReportCount() { - ++fWriteFailureCount; - jack_time_t now = GetMicroSeconds(); - if (fLastWriteFailureReport != 0 && now - fLastWriteFailureReport < 1000000) { - return 0; - } - fLastWriteFailureReport = now; - uint64_t count = fWriteFailureCount; - fWriteFailureCount = 0; - return count; + return TakeFailureReportCount(gWriteFailureCount, gLastWriteFailureReport); } unsigned int JackRingBuffer::ReadSpace() diff --git a/common/JackResampler.h b/common/JackResampler.h index 8ce00755..57f4c2e0 100644 --- a/common/JackResampler.h +++ b/common/JackResampler.h @@ -45,10 +45,6 @@ class JackRingBuffer jack_ringbuffer_t* fRingBuffer; unsigned int fRingBufferSize; - uint64_t fReadFailureCount; - uint64_t fWriteFailureCount; - jack_time_t fLastReadFailureReport; - jack_time_t fLastWriteFailureReport; uint64_t ReadFailureReportCount(); uint64_t WriteFailureReportCount();