Skip to content

Never block the control thread indefinitely on a stalled render thread#635

Open
cmzy wants to merge 2 commits into
orottier:mainfrom
cmzy:fix/control-thread-render-liveness
Open

Never block the control thread indefinitely on a stalled render thread#635
cmzy wants to merge 2 commits into
orottier:mainfrom
cmzy:fix/control-thread-render-liveness

Conversation

@cmzy

@cmzy cmzy commented Jul 16, 2026

Copy link
Copy Markdown

This PR groups two commits that make the control thread resilient to a stalled render thread; the sink-change fix reuses the liveness rule introduced by the first commit, so they are kept together.

Never block the control thread indefinitely on a stalled render thread

Two related hang surfaces, both triggered by a render thread that stops
draining its channels - in practice: the output device was unplugged or
the driver died, upon which cpal only invokes err_fn (which logs) and
stops issuing callbacks.

  1. send_control_msg() performed a blocking send() into the bounded(256)
    control channel. Once the render callback stops draining, the 257th
    message blocks the sending thread forever. Applications hosting a
    scripting engine send control messages from their main thread, which
    then hangs wholesale.

  2. suspend_sync()/resume_sync()/close_sync() send a oneshot notify
    through the same machinery and then wait with a bare recv() for the
    render thread to acknowledge. With the render thread gone the notify
    never arrives. close() is exactly the API an application calls right
    after a device disappears, and it hung forever.

Manual reproduction: play audio through a USB audio interface, unplug
it, then keep using the context (schedule >256 param changes, or call
ctx.close()). The control thread never returns.

Fix: keep the channel bounded (the receiving side lives on the render
thread; a bounded channel is allocation-free and realtime-safe) and keep
blocking back-pressure semantics (single FIFO: no drops, no reordering),
but replace the indefinite waits with waits on render-thread liveness.
While the channel is full (or the ack is pending), poll every 50 ms and
check whether frames_played advanced - the render thread bumps it by 128
per rendered quantum. Progress means plain back-pressure: keep waiting,
exactly like before. Zero progress across a 2 s window (~700 missed
quanta - not busyness) means the render side is gone: mark the context
Closed, drop the in-flight message / stop waiting, and let the existing
Closed short-circuit discard all subsequent messages. Dropping is safe
at that point: a Closed context discards control messages anyway, and
there is no render thread left to execute them.

Liveness (playhead progress) is used instead of a fixed send timeout so
that healthy-but-busy devices and device startup cannot misfire the
detection. Offline contexts are unaffected (their control channel is
unbounded, and their playhead only advances under explicit rendering
calls on the control thread itself).

A regression test guards the false-positive direction: a suspended
context keeps draining its channel without advancing the playhead, so
flooding it with thousands of messages must neither hang nor close it.
The stall direction requires actual device removal and remains a manual
test.

Do not hang or silently kill the context when a sink change fails

set_sink_id_sync had two failure paths that misbehaved when the audio
device disappears mid-protocol (unplugged USB interface, dead driver -
cpal only reports through err_fn and stops issuing callbacks):

  1. After sending CloseAndRecycle it waited for the render thread to
    hand back the audio graph with a bare graph_recv.recv().unwrap().
    With the render thread stalled the graph never arrives and the
    control thread blocks forever. "Device disappeared, application
    switches to another sink" is precisely the sequence that hits this.

  2. If io::build_output for the new sink failed after the old backend
    had already been closed, the error was returned but the context kept
    claiming its previous state - typically Running - while the graph
    had been taken out of the render thread and dropped. Nothing can
    produce audio at that point; every later call quietly operates on a
    dead context.

The graph wait now uses the same frames_played liveness rule introduced
for send_control_msg and the suspend/resume/close acknowledgements:
poll every 50 ms and only declare the render thread gone after 2 s
without playhead progress. Progress means plain back-pressure and the
wait continues exactly as before. On a stall (or a disconnected reply
channel) the context is marked Closed and the call returns an error
instead of hanging. A build_output failure after teardown also marks
the context Closed so the observable state reflects reality.

The stall direction requires physically removing a device and remains a
manual test. A regression test covers the ordering guarantee that must
not change: rejecting an invalid sinkId happens before any teardown and
must leave the context running and usable.

cmzy added 2 commits July 15, 2026 22:49
Two related hang surfaces, both triggered by a render thread that stops
draining its channels - in practice: the output device was unplugged or
the driver died, upon which cpal only invokes err_fn (which logs) and
stops issuing callbacks.

1. send_control_msg() performed a blocking send() into the bounded(256)
   control channel. Once the render callback stops draining, the 257th
   message blocks the sending thread forever. Applications hosting a
   scripting engine send control messages from their main thread, which
   then hangs wholesale.

2. suspend_sync()/resume_sync()/close_sync() send a oneshot notify
   through the same machinery and then wait with a bare recv() for the
   render thread to acknowledge. With the render thread gone the notify
   never arrives. close() is exactly the API an application calls right
   after a device disappears, and it hung forever.

Manual reproduction: play audio through a USB audio interface, unplug
it, then keep using the context (schedule >256 param changes, or call
ctx.close()). The control thread never returns.

Fix: keep the channel bounded (the receiving side lives on the render
thread; a bounded channel is allocation-free and realtime-safe) and keep
blocking back-pressure semantics (single FIFO: no drops, no reordering),
but replace the indefinite waits with waits on render-thread liveness.
While the channel is full (or the ack is pending), poll every 50 ms and
check whether frames_played advanced - the render thread bumps it by 128
per rendered quantum. Progress means plain back-pressure: keep waiting,
exactly like before. Zero progress across a 2 s window (~700 missed
quanta - not busyness) means the render side is gone: mark the context
Closed, drop the in-flight message / stop waiting, and let the existing
Closed short-circuit discard all subsequent messages. Dropping is safe
at that point: a Closed context discards control messages anyway, and
there is no render thread left to execute them.

Liveness (playhead progress) is used instead of a fixed send timeout so
that healthy-but-busy devices and device startup cannot misfire the
detection. Offline contexts are unaffected (their control channel is
unbounded, and their playhead only advances under explicit rendering
calls on the control thread itself).

A regression test guards the false-positive direction: a suspended
context keeps draining its channel without advancing the playhead, so
flooding it with thousands of messages must neither hang nor close it.
The stall direction requires actual device removal and remains a manual
test.
set_sink_id_sync had two failure paths that misbehaved when the audio
device disappears mid-protocol (unplugged USB interface, dead driver -
cpal only reports through err_fn and stops issuing callbacks):

1. After sending CloseAndRecycle it waited for the render thread to
   hand back the audio graph with a bare graph_recv.recv().unwrap().
   With the render thread stalled the graph never arrives and the
   control thread blocks forever. "Device disappeared, application
   switches to another sink" is precisely the sequence that hits this.

2. If io::build_output for the new sink failed after the old backend
   had already been closed, the error was returned but the context kept
   claiming its previous state - typically Running - while the graph
   had been taken out of the render thread and dropped. Nothing can
   produce audio at that point; every later call quietly operates on a
   dead context.

The graph wait now uses the same frames_played liveness rule introduced
for send_control_msg and the suspend/resume/close acknowledgements:
poll every 50 ms and only declare the render thread gone after 2 s
without playhead progress. Progress means plain back-pressure and the
wait continues exactly as before. On a stall (or a disconnected reply
channel) the context is marked Closed and the call returns an error
instead of hanging. A build_output failure after teardown also marks
the context Closed so the observable state reflects reality.

The stall direction requires physically removing a device and remains a
manual test. A regression test covers the ordering guarantee that must
not change: rejecting an invalid sinkId happens before any teardown and
must leave the context running and usable.
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.

1 participant