Skip to content

pipewire: fix thread-loop lifetime & recursive-lock crashes in the shared context#226

Open
ogauthiersat wants to merge 4 commits into
celtera:masterfrom
ogauthiersat:fix/pipewire-context-concurrency
Open

pipewire: fix thread-loop lifetime & recursive-lock crashes in the shared context#226
ogauthiersat wants to merge 4 commits into
celtera:masterfrom
ogauthiersat:fix/pipewire-context-concurrency

Conversation

@ogauthiersat

Copy link
Copy Markdown

Summary

Fixes three related crashes/deadlocks in the shared PipeWire context
(backends/linux/pipewire/context.hpp) that surface when the thread-loop
backend is used from real applications — driving the loop from foreign threads,
reconnecting a broken context, and using several MIDI objects (or MIDI + an
audio filter) on the shared context at once.

All three were found integrating this layer into
ossia score (audio + MIDI sharing one
libremidi::pipewire::context), tested against PipeWire 1.4.2.

Bugs & fixes

1. synchronize() iterated the thread-loop from two threads

For loop_kind::thread, do_sync_locked() called pw_loop_iterate() on the
shared pw_loop from the calling thread while the pw_thread_loop worker was
iterating the same loop
. Two threads iterating one pw_loop corrupt its
per-source dispatch bookkeeping (s->priv / s->rmask, with a per-thread stack
ep[]) and double-dispatch the core-socket callback, ending in
pw_protocol_native_connection_flush(conn=0x0) on the worker thread (segfault
at process exit, when teardown floods the socket).

Fix: for the thread loop, issue pw_core_sync and wait on a private
std::condition_variable instead of iterating — the single worker drives the
loop and on_core_done / on_core_error wake the waiter.

2. reconnect() tore the loop down from inside itself

reconnect() wrapped tear_down() + build_connection() in invoke_sync(),
so they ran on the worker thread. But tear_down() stops and destroys that same
loop: pthread_join(self) is a silent no-op and pw_thread_loop_destroy() then
unlocks a loop it is running inside → 'this->recurse > 0' failed in
do_unlock() and the blocking invoke never returns (hang).

Fix: run the tear-down/rebuild/sync on the calling thread, and bail out early if
called from the loop thread.

3. Recursive thread-loop lock misused in invoke_sync() and synchronize()

Two more 'recurse > 0' failures + deadlocks, both stemming from PipeWire's
thread-loop mutex being PTHREAD_MUTEX_RECURSIVE:

  • invoke_sync() (used by unsubscribe()) called
    pw_loop_invoke(block=true) without holding the thread-loop lock.
    PipeWire's blocking invoke wraps its wait in the loop control hooks
    (do_unlock before / do_lock after), so it requires the caller to hold the
    lock exactly once. Unlocked, do_unlock underflows the recurse counter and
    the stray do_lock leaves the mutex owned by the wrong thread, deadlocking
    the next pw_thread_loop_stop() join. Fix: hold the lock around the blocking
    invoke.

  • synchronize() held the recursive lock across
    pw_thread_loop_timed_wait(); pthread_cond_wait cannot fully release a
    recursively-held mutex, so nested/concurrent syncs starved the worker and
    corrupted the lock. Fix: the private-CV wait never holds the
    loop lock across the wait, and synchronize() now guards is_in_loop_thread()
    (drives iteration directly on the worker rather than waiting on itself).

pw_thread_loop_timed_wait is no longer used and is dropped from the
thread_available gate.

Verification

Standalone reproducers against a live PipeWire 1.4.2 daemon (each reproduced the
failure before the fix, clean after):

  • Create/destroy context + repeated synchronize() (was: flush(conn=0x0) at exit).
  • reconnect() from the main thread, ×N (was: recurse > 0 + SIGSEGV).
  • MIDI-style filter + port + synchronize_node() + port subscriptions, teardown
    (was: recurse > 0 at unsubscribe, then pw_thread_loop_stop deadlock).
  • Audio-style filter created/destroyed repeatedly while a MIDI filter +
    subscriptions stay live on the shared context.
  • Sync stress (tens of thousands of round-trips), reconnect loops, nested and
    loop-thread synchronize().

Regression tests

Added under examples/backends/ (built when LIBREMIDI_HAS_PIPEWIRE):

  • pipewire_context_sync — create/synchronize/destroy cycles + a sustained
    synchronize() loop.
  • pipewire_context_reconnect — repeated reconnect() from the calling thread.
  • pipewire_context_subscriptions — subscribe/unsubscribe churn, MIDI-style
    filter teardown with live subscriptions, and synchronize() from a subscriber
    callback.

Each skips (exit 0) when no PipeWire daemon is reachable and arms a watchdog so a
lock-corruption regression fails instead of hanging. Against the pre-fix code,
pipewire_context_reconnect and pipewire_context_subscriptions reproduce
'this->recurse > 0' failed + SIGSEGV; all three pass after the fix.

Notes / not addressed here

reconnect() still rebuilds the loop and core, which invalidates pw_filters
that other holders created on the old ones — inherent to PipeWire (a lost core
connection kills all proxies). It's only reachable on a genuine daemon
disconnect. Coordinating dependent filter recreation across a reconnect is left
for a follow-up.

Disclosure

The implementation in this PR was produced with an AI coding
agent (Anthropic's Claude Opus 4.8) using omp.
I directed the work, reviewed every change, and ran the verification
described above; I take responsibility for its correctness and licensing.

ogauthiersat and others added 4 commits July 9, 2026 17:01
In thread-loop mode context::do_sync_locked() called pw_loop_iterate() on
the shared pw_loop from the caller while the pw_thread_loop worker iterates
the same loop. Two threads iterating one pw_loop corrupt the per-source
dispatch bookkeeping (s->priv/s->rmask, stack-local ep[]) and double-fire
socket callbacks, crashing in on_remote_data -> pw_protocol_native_connection_flush
on a freed/NULL connection (segfault at process exit, during audio filter
teardown which calls synchronize() amid protocol traffic).

Use the idiomatic pattern instead: for the thread loop, issue pw_core_sync
and block on pw_thread_loop_timed_wait, letting the single worker drive the
loop; on_core_done/on_core_error wake the waiter via pw_thread_loop_signal.
The main-loop path (no worker) keeps driving iteration itself. Require
pw_thread_loop_timed_wait in the thread_available gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
reconnect() wrapped tear_down()+build_connection() in invoke_sync(), running
them on the pw_thread_loop worker. But tear_down() stops and destroys that
same loop: pthread_join(self) returns EDEADLK (silent no-op) and
pw_thread_loop_destroy() then unlocks a loop it is running inside, tripping
'this->recurse > 0' in do_unlock() and hanging the blocking invoke. score's
PipeWireAudioFactory::make_engine() calls reconnect() directly when the shared
context is broken, so this fires even with auto_reconnect off.

Run the tear-down/rebuild/sync directly on the calling thread instead, and
bail out if called from the loop thread. tear_down() then joins the worker
from a foreign thread (correct) before recreating the loop and core.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two related recursive-lock bugs surfaced by using a MIDI device and audio
together on the shared context:

1. invoke_sync() (used by unsubscribe) called pw_loop_invoke(block=true)
   WITHOUT holding the thread-loop lock. pipewire's blocking invoke wraps its
   wait in the loop control hooks (do_unlock before / do_lock after), so it
   requires the caller to hold the lock exactly once. Unlocked, do_unlock
   underflows the recurse counter ('recurse > 0' failed at thread-loop.c:62)
   and the stray do_lock leaves the recursive mutex owned by the wrong thread,
   deadlocking the next pw_thread_loop_stop() join. Take the lock around the
   blocking invoke.

2. synchronize() held pipewire's recursive loop lock across
   pw_thread_loop_timed_wait(); pthread_cond_wait cannot fully release a
   recursively-held mutex, so nested/concurrent syncs starved the worker and
   corrupted the lock. Replace with a private std::condition_variable: take the
   loop lock only briefly to issue pw_core_sync, then wait off the loop lock;
   on_core_done/on_core_error notify it. Also guard is_in_loop_thread() (drive
   iteration directly on the worker instead of waiting on ourselves).

pw_thread_loop_timed_wait is no longer used; drop it from the thread gate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Three standalone programs under examples/backends/ that exercise the shared
thread-loop context fixes:

- pipewire_context_sync: many create/synchronize/destroy cycles plus a sustained
  synchronize() loop (foreign-thread sync must not iterate the loop concurrently
  with the worker).
- pipewire_context_reconnect: repeated reconnect() from the calling thread
  (must not self-destruct the loop from the worker thread).
- pipewire_context_subscriptions: subscribe/unsubscribe churn, MIDI-style filter
  teardown with live subscriptions, and synchronize() from a subscriber callback
  (recursive thread-loop lock must stay balanced).

Each skips (exit 0) when no PipeWire daemon is reachable and arms a watchdog so a
lock-corruption regression fails loudly instead of hanging. Built against the
pre-fix headers, pipewire_context_reconnect and pipewire_context_subscriptions
reproduce 'recurse > 0' + SIGSEGV.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@jcelerier

Copy link
Copy Markdown
Member

Heads-up: #229 contains this PR verbatim (authorship preserved) plus #227's format.hpp buffer-param ranges and a reconciliation of the two invoke_sync fixes — merging #226 and #227 independently would auto-merge cleanly but deadlock (double lock on the recursive mutex; the loop-control hooks release exactly one level). Reviewed against pipewire's thread-loop.c/loop.c; validated with ossia score's 28-cell PipeWire round-trip harness on 1.3.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants