Skip to content

Do not hold the egress SSRC lock while the send timer emits RTCP - #2237

Closed
jleal52 wants to merge 1 commit into
sipwise:masterfrom
jleal52:send-timer-rtcp-lock
Closed

jleal52 wants to merge 1 commit into
sipwise:masterfrom
jleal52:send-timer-rtcp-lock

Conversation

@jleal52

@jleal52 jleal52 commented Nov 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

__send_timer_rtcp() held ssrc_out->h.lock across rtcp_send_report(), which locks other SSRC entries: the receiver-report sources of the sink media and the sender-report bookkeeping entry on the far media, i.e. ingress entries. The transcoder locks ingress first and egress second (__ssrc_lock_both).

The two orders met on a call with a transcoded leg and the daemon deadlocked: all pollers and send timers in futex, control socket unread. Owner graph from the core dump: a poller in __ssrc_lock_both holding the ingress entry and waiting for the egress one, a send timer in rtcp_sender_report holding that egress entry and waiting for the ingress one.

Change

Take the lock only to check and claim next_rtcp (so that another send timer sharing the SSRC does not send a second report), then send the report without it, as the RTCP_GEN timer in codec.c already does.

The inversion is easiest to hit when an egress entry is shared (fixed egress SSRC, audio player; see #2195), but the lock order is the same without it. With #2195 applied and without this fix, auto-daemon-tests-inject.pl (RTPENGINE_EXTENDED_TESTS=1) deadlocked in 1 of 3 runs with exactly this owner graph (gdb: the media player thread in __send_timer_rtcp → rtcp_send_report → ssrc_sender_report waiting on an ingress entry, two pollers in __ssrc_lock_both waiting on the egress one); with this fix, in none.

Testing

make asan-check passes on this branch alone, and on top of #2195, #2238 and #2239.

__send_timer_rtcp() kept ssrc_out->h.lock across rtcp_send_report(), which
locks other SSRC entries: the receiver-report sources of the sink media and
the sender-report bookkeeping entry on the far media, i.e. ingress entries.
The transcoder locks ingress first and egress second (__ssrc_lock_both).
The two orders met on a call with a transcoded leg and the daemon
deadlocked: a poller in __ssrc_lock_both held the ingress entry and waited
for the egress one, while a send timer in rtcp_sender_report held that
egress entry and waited for the ingress one. Every other thread queued
behind them, including the control socket.

Take the lock only to check and claim next_rtcp, then send the report
without it, as the RTCP_GEN timer in codec.c already does.
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