netJACK2: reap dead masters, fix KillMaster UAF, dedupe slaves, pin slave multicast - #1
Open
sastraxi wants to merge 14 commits into
Open
netJACK2: reap dead masters, fix KillMaster UAF, dedupe slaves, pin slave multicast#1sastraxi wants to merge 14 commits into
sastraxi wants to merge 14 commits into
Conversation
…lave 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137tergcV2UfSv81GpryaKT
…dupe 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0137tergcV2UfSv81GpryaKT
JackNetMaster is built through JackNetMasterInterface(params, socket, multicast_ip), which left fDead uninitialized — the atomic<bool> 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.
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.
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KRptcXkk6KFdX1KzPnbak9
…roup
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01DHztJXcFXtPMyPGbtKKLgC
…IGPIPE 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. d48aaa0 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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ABgaVgUV7CjDt9arjy4vNx
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four defects that together made a departed netJACK2 slave a permanent, silent stall on the master side. Full write-up in
NETJACK-REAPING.md.FatalRecvError/FatalSendErrorcalledThreadExit()from the JACK process callback, leaving the client registered with a dead RT thread. Now they set an atomicfDead;JackNetMasterManager::Runreaps via the newReapDeadMasters()off the RT thread.KillMasteruse-after-free — dereferenced an erased iterator todelete. ExtractedRemoveMaster()which captures the pointer before erasing.InitMasterspawned a fresh master perSLAVE_AVAILABLE, producingpistomp-01/-02duplicates. It now reaps any master already holding that name first.JackNetAdapterset the multicast interface on fd 0 before the socket existed andNewSocket()never re-applied it, so a leaked netadapter announced over Wi-Fi.JackNetUnixSocketnow stores the ifname and re-pins it (checked) on everyNewSocket().Builds clean (
jack_net,jack_netone). Not yet hardware-verified against a live pi-Stomp.🤖 Generated with Claude Code