Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed deadlock between stream and ICE #3801

Merged
merged 3 commits into from
Dec 14, 2023
Merged

Fixed deadlock between stream and ICE #3801

merged 3 commits into from
Dec 14, 2023

Conversation

sauwming
Copy link
Member

It is reported that a deadlock can occur between stream and ICE:

SIPMediaControl!pj_mutex_lock_ex+156 [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 1124]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 1124
SIPMediaControl!on_rx_rtp+411 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 2077]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 2077
SIPMediaControl!srtp_rtp_cb+6d [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_srtp.c @ 1552]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_srtp.c @ 1552
SIPMediaControl!ice_on_rx_data+f2 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_ice.c @ 2572]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_ice.c @ 2572
SIPMediaControl!ice_rx_data+34 [C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 2287]	  C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 2287
SIPMediaControl!pj_ice_sess_on_rx_pkt+1dd [C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_session.c @ 3742]	  C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_session.c @ 3742
SIPMediaControl!stun_on_rx_data+b5 [C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 2369]	  C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 2369
SIPMediaControl!on_data_recvfrom+104 [C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\stun_sock.c @ 1009]	  C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\stun_sock.c @ 1009
SIPMediaControl!ioqueue_on_read_complete+fc [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\activesock.c @ 520 + 23]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\activesock.c @ 520 + 23
SIPMediaControl!ioqueue_dispatch_read_event+21e [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\ioqueue_common_abs.c @ 629]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\ioqueue_common_abs.c @ 629
SIPMediaControl!pj_ioqueue_poll+3ac [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\ioqueue_select.c @ 1128]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\ioqueue_select.c @ 1128
SIPMediaControl!pjsip_endpt_handle_events2+8e [C:\UCS\External\pjsip\pjproject-2.14\pjsip\src\pjsip\sip_endpoint.c @ 746]	  C:\UCS\External\pjsip\pjproject-2.14\pjsip\src\pjsip\sip_endpoint.c @ 746
SIPMediaControl!worker_thread+50 [C:\UCS\External\pjsip\pjproject-2.14\pjsip\src\pjsua-lib\pjsua_core.c @ 807 + 30]	  C:\UCS\External\pjsip\pjproject-2.14\pjsip\src\pjsua-lib\pjsua_core.c @ 807 + 30
SIPMediaControl!thread_main+4a [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 538 + 7]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 538 + 7
SIPMediaControl!pj_mutex_lock_ex+156 [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 1124]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 1124
SIPMediaControl!grp_lock_acquire+36 [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\lock.c @ 300]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\lock.c @ 300
SIPMediaControl!send_data+b2 [C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 1837]	  C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 1837
SIPMediaControl!pj_ice_strans_sendto2+35 [C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 1988]	  C:\UCS\External\pjsip\pjproject-2.14\pjnath\src\pjnath\ice_strans.c @ 1988
SIPMediaControl!transport_send_rtcp2+7d [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_ice.c @ 2512]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_ice.c @ 2512
SIPMediaControl!transport_send_rtcp+32 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_srtp.c @ 1417]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\transport_srtp.c @ 1417
SIPMediaControl!send_rtcp+26c [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1211 + 15]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1211 + 15
SIPMediaControl!check_tx_rtcp+46 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1270]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1270
SIPMediaControl!put_frame_imp+3a0 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1556]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1556
SIPMediaControl!put_frame+16b [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1725]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\stream.c @ 1725
SIPMediaControl!write_port+1dd [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\conference.c @ 1862 + 1c]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\conference.c @ 1862 + 1c
SIPMediaControl!get_frame+5f8 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\conference.c @ 2230]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\conference.c @ 2230
SIPMediaControl!clock_callback+9d [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\master_port.c @ 195]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\master_port.c @ 195
SIPMediaControl!clock_thread+c5 [C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\clock_thread.c @ 395]	  C:\UCS\External\pjsip\pjproject-2.14\pjmedia\src\pjmedia\clock_thread.c @ 395
SIPMediaControl!thread_main+4a [C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 538 + 7]	  C:\UCS\External\pjsip\pjproject-2.14\pjlib\src\pj\os_core_win32.c @ 538 + 7

The issue occurs because of the added locking when sending RTCP in #3528 .

@nanangizz
Copy link
Member

Is this deadlock scenario still possible?

  • send side thread:
    1. got lock(rtcp_mutex)
    2. want lock(ice_mutex)
  • receive side thread:
    1. got lock(ice_mutex)
    2. want lock(rtcp_mutex)

@sauwming
Copy link
Member Author

You're right. I have checked that on_rx_rtcp() does not acquire lock, but missed the part that on_rx_rtp() can call send_rtcp() as well.

I haven't found a proper solution though. Some ideas:

@nanangizz
Copy link
Member

* Revert [Prevent data race in stream when sending RTCP #3528](https://github.com/pjsip/pjproject/pull/3528) (and reopen [Data races in pjmedia stream #1973](https://github.com/pjsip/pjproject/issues/1973)). The data race seems to be a relatively minor issue, especially if compared to the potential deadlock.

Perhaps only consider this if there is no 'easy' solution.

* Move `send_rtcp()` from `on_rx_rtp()` to `check_tx_rtcp()`. I'm not sure if there can be potential consequences of this though.

Not sure if I understand this correctly, If you mean removing send_rtp() from on_rx_rtp(), then it sounds like a major change, it may not be good for receive-only stream.

Btw, how about using transport group lock for RTCP?

@sauwming sauwming merged commit 378e97c into master Dec 14, 2023
30 of 35 checks passed
@sauwming sauwming deleted the rtcp-mutex branch December 14, 2023 04:16
xhit pushed a commit to xhit/pjproject that referenced this pull request Dec 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants