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.

Sign up for GitHub

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

Jump to bottom

Crash in the videoroom plugin #2582

Closed
cppdev-1 opened this issue Mar 16, 2021 · 8 comments
Closed

Crash in the videoroom plugin #2582

cppdev-1 opened this issue Mar 16, 2021 · 8 comments
Labels
pr-exists

Comments

@cppdev-1
Copy link

cppdev-1 commented Mar 16, 2021 •
edited by atoppi
Loading 

Program terminated with signal SIGABRT, Aborted.
#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
50      ../sysdeps/unix/sysv/linux/raise.c: No such file or directory.
[Current thread is 1 (Thread 0x7f548e7fc700 (LWP 32457))]
(gdb) bt
#0  0x00007f550b1d27bb in __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:50
#1  0x00007f550b1bd535 in __GI_abort () at abort.c:79
#2  0x00007f550bec2e0f in g_mutex_clear () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#3  0x00007f55040be9d9 in janus_videoroom_publisher_free (p_ref=0x7f54cc0038d8) at plugins/janus_videoroom.c:1761
#4  0x00007f55040cb5d9 in janus_videoroom_destroy_session (handle=<optimized out>, error=<optimized out>) at plugins/janus_videoroom.c:2731
#5  0x00005625e84af463 in janus_ice_outgoing_traffic_handle (handle=0x7f54f0005650, pkt=0x5625e8523f20 <janus_ice_detach_handle>) at ice.c:4290
#6  0x00005625e84b1ac4 in janus_ice_outgoing_traffic_dispatch (source=0x7f54f0005860, callback=<optimized out>, user_data=<optimized out>) at ice.c:411
#7  0x00007f550be78f2e in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#8  0x00007f550be791c8 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#9  0x00007f550be794c2 in g_main_loop_run () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#10 0x00005625e84a33eb in janus_ice_handle_thread (data=0x7f54f0005650) at ice.c:1208
#11 0x00007f550bea1415 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#12 0x00007f550b363fa3 in start_thread (arg=<optimized out>) at pthread_create.c:486
#13 0x00007f550b2944cf in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:95

Steps to reproduce

  1. The patch reproduce.patch have to be applied to sources;
  2. Create a video room with two participants;
  3. Start video for them;
  4. Close browser for the 2nd participant;
  5. Quickly (<3 seconds) stop video for 1st participant;
  6. Wait a little for the crash.

reproduce.zip
videoroom.zip

Provided patch fixes it. Also it fixes a crash in "switch" request.
@lminiero
Copy link
Member

If you have a fix, please submit a PR and document what's wrong and why there. That will make it easier for us to look at the changes. I'm not going to manually go through patches, sorry.

@cppdev-1
Copy link
Author

PR - #2584

@cppdev-1
Copy link
Author

cppdev-1 commented Mar 24, 2021 •
edited
Loading

@lminiero I don't mind if you don't accept my PR. The main thing is that I'd like you to have a look at reproduce patch, because I believe it reveals an architectural problem in Janus.

@lminiero
Copy link
Member

I don't think that can happen: hangup_media is triggered by the handle loop and can't go dormant for such a long time, and a g_slist_remove will never take that long either. Did you notice or experience behaviours at runtime that seemed to suggest similar scenarios could happen? Or is it mostly meant to highlight a potential race condition, that the patch makes more likely to occur?

@cppdev-1
Copy link
Author

cppdev-1 commented Mar 24, 2021 •
edited
Loading

Yes, the patch makes the race condition (and crash) more likely to occur with the lower number of participants. I've created this patch because Janus really crashed.

lminiero referenced this issue Apr 7, 2021
@lminiero
Added extra reference to VideoRoom
9bf29f9
lminiero added a commit that referenced this issue Apr 16, 2021
@lminiero
Fix for race condition between VideoRoom publisher leaving and subscr…
1dcb4e1 
…iber hanging up (fixes #2582)
@lminiero lminiero mentioned this issue Apr 16, 2021
Merged
@lminiero
Copy link
Member

@cppdev-1 sorry if this took a while, but I managed to find some time to use your code to replicate the issue and work on a fix. You can find it referenced above, please let me know if it fixes it for you.

@cppdev-1
Copy link
Author

@lminiero I've tested your patch, it works fine for me. I think the issue has been fixed.

@lminiero
Copy link
Member

Cool, thanks for the feedback!

heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Apr 29, 2021
@imhao183
Merge branch 'meetecho_master'
262e4c7 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Jul 29, 2021
@imhao183 @heavyrain2012
Merge branch 'meetecho_master'
3529b8d 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Jul 29, 2021
@imhao183 @heavyrain2012
Merge branch 'meetecho_master'
feec490 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Jul 29, 2021
@imhao183 @heavyrain2012
Merge branch 'meetecho_master'
dab3e25 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Aug 2, 2021
@imhao183 @heavyrain2012
Merge branch 'meetecho_master'
e231d7a 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Nov 14, 2021
@imhao183 @heavyrain2012
Merge branch 'meetecho_master'
1827315 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Dec 7, 2021
@imhao183
Merge branch 'meetecho_master'
b9318b7 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c

(cherry picked from commit 1827315)
heavyrain2012 pushed a commit to heavyrain2012/janus-gateway that referenced this issue Jan 29, 2022
@imhao183
Merge branch 'meetecho_master'
cf53cb9 
* meetecho_master: (345 commits)
  Remove support for framemarking RTP extension (meetecho#2640)
  Prevent race conditions on socket close in SIP and NoSIP plugins (meetecho#2599)
  Fixed overflow runtime error
  Fix for race condition between VideoRoom publisher leaving and subscriber hanging up (fixes meetecho#2582) (meetecho#2637)
  Send PLI when starting a paused stream (meetecho#2645)
  Prevent too high shift exponent
  Fixed type of seq/ts in file-based Streaming mountpoint threads
  Fix missing g_thread_unref when a streaming helper thread quits.
  Added custom headers for SIP INFO request (meetecho#2644)
  Free participant->user_id_str in case of opus enc/decoder error.
  Reject flexfec when offered, as still unsupported (see meetecho#2639)
  Don't add rtx ssrc if m-line is recvonly/inactive (see meetecho#2639)
  Added NULL checks for json_dumps (see meetecho#2629)
  Parse custom headers, if required, in successful REGISTER response (fixes meetecho#2636)
  Timestamp correction for janus-pp-rec (meetecho#2573)
  Don't chain error handler to success handler in Janus.httpAPICall (meetecho#2569)
  Add missing library link for WS event handler (fixes meetecho#2628)
  Fixed broken switch in Streaming plugin when using helper threads
  Resolves meetecho#2624 jansson double referencing (meetecho#2634)
  Unlock mountpoints mutex after the spawning of helper threads.
  ...

# Conflicts:
#	transports/janus_mqtt.c

(cherry picked from commit 1827315)
(cherry picked from commit b9318b7)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
pr-exists
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@atoppi @lminiero @cppdev-1