fix(audio): stop the audio pump re-taking the lock its caller already holds

[datagutt]

Alternative to #11, fixing the same deadlock at its source rather than by relaxing mutex semantics.

The bug is a shipped regression

irl_audio_thread holds audio_state_lock across the whole of irl_pump_audio_once (src/receiver.c:37-39), but two places inside the pump took it again:

• src/receiver-audio.c:766 — the audio_fill_peak_ms publish, added by c57d4f0
• src/receiver-audio.c:642 — the offset re-anchor in irl_audio_maybe_reanchor_offset, added by d5bd5cc

A POSIX mutex is not recursive, so the second acquire hangs the audio thread. The video thread then blocks on the same lock, nothing drains the demuxer, and OBS waits on both — the "No room to store incoming packet" flood @iamconorwilson reported. Windows never showed it because CRITICAL_SECTION is recursive, so the re-acquire is a no-op there.

git tag --contains c57d4f0 returns v1.3.0 and v1.3.1. The fill-peak acquire is on the pump's unconditional path (the only early return above it needs audio_out_primed, which is false at startup), so the audio thread deadlocks within about a millisecond of the first audio frame creating the buffer — before playback ever primes. Both releases hang on Linux and macOS for any stream with audio. Worth checking against #9.

What this does

fix(audio) — removes the two inner acquires. The caller's hold already covered those writes, so this is a strict reduction in lock operations, and the critical section gets wider, not narrower: no reader can observe anything it could not before. The comments claiming "nothing is nested here" described the callee in isolation and were wrong about the caller; the contract is now stated at the declaration, the definition, the call site, and in the threading section of CLAUDE.md.

I audited every holder of audio_state_lock and video_queue_lock and traced what each region calls. These two were the only nested acquisitions in the tree.

fix(threading) — irl_mutex_init returned an error nobody read. irl_source_create now frees and returns NULL; audio_buffer_init returns bool and creates the lock before setting sample_rate, which audio_buffer_free uses as its "init ran" marker, so a failed init cannot leave free() destroying a mutex that does not exist. This is the hardening from #11, which stands on its own.

feat(threading) — -DIRL_CHECKED_LOCKS=ON, automatic in Debug, turns a lock-contract violation into an immediate abort naming the offending line instead of a frozen stream. POSIX mutexes become PTHREAD_MUTEX_ERRORCHECK; Win32 reads CRITICAL_SECTION's RecursionCount, which is the half Windows otherwise cannot see at all — and not seeing it is why this shipped. irl_mutex_lock/unlock become function-like macros so the abort reports the caller's location rather than a line in the header.

Default builds are unchanged: plain pthread_mutex_init, no branch on the lock path.

Why not make the mutexes recursive

That also stops the hang, and it is what #11 does. The cost is that it relaxes all three mutexes permanently to tolerate one bug in one call path, and video_queue_lock is paired with a condition variable — pthread_cond_wait on a recursively-held mutex releases only one level, so a future nested acquire on that path would hold the lock through the sleep and reintroduce the same hang, now with "our mutexes are recursive, nesting is fine" as the documented design. Checked locks go the other way: they make the discipline enforceable.

Testing

CI covers the build on all three platforms. Locally I verified the checked-lock machinery against real libobs:

build	case	result
IRL_CHECKED_LOCKS	nested acquire	aborts, names the caller's line, EDEADLK
IRL_CHECKED_LOCKS	unlock not held	aborts, EPERM
IRL_CHECKED_LOCKS	normal lock/unlock	unaffected
default	nested acquire	hangs — the shipped behaviour

Not yet exercised against a live stream. The direct confirmation would be reverting the first commit under a checked build and watching it abort at receiver-audio.c:766 instead of freezing.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved audio and video initialization error handling, including cleanup after synchronization failures.
  • Prevented potential audio-thread deadlocks caused by recursive lock acquisition.
  • Audio buffer initialization now reports failures reliably.

• Development

  • Added optional checked-lock diagnostics for detecting invalid or recursive mutex operations.
  • Expanded development guidance for audio-thread lock usage and ordering.