meetecho / janus-gateway

Janus WebRTC Server
https://janus.conf.meetecho.com
GNU General Public License v3.0
8.25k stars 2.48k forks source link

[1.x] Streaming plugin #3288

Closed amnonbb closed 1 year ago

amnonbb commented 1 year ago

What version of Janus is this happening on? 1.2.1 0ae1c6d286bd59efe41d85efd5186dbad880a6eb

Have you tested a more recent version of Janus too? It's latest master

Was this working before? This crash i got in all version i tried

Is there a gdb or libasan trace of the issue? https://gist.github.com/amnonbb/8e88f82859eede4e44a69f3f5555642d

Additional context It's for sure cause when client do ICE restart. (When i turn off this on client, crash is gone) But i it's in some unknown condition. It's not possible to reproduce directly to use ICE restart.

atoppi commented 1 year ago

The previously allocated and freed by sections in the asan dump look inverted:

Nov 09 12:06:44 str1 janus[110770]: previously allocated by thread T6 here:
Nov 09 12:06:44 str1 janus[110770]:     #3 0x7f2fde0e0c98  (/lib64/libglib-2.0.so.0+0x3bc98)
Nov 09 12:06:44 str1 janus[110770]:     #2 0x7f2fd0d4eae7 in janus_streaming_session_destroy plugins/janus_streaming.c:1563
Nov 09 12:06:44 str1 janus[110770]:     #1 0x7f2fde0f86f1 in g_free (/lib64/libglib-2.0.so.0+0x536f1)
Nov 09 12:06:44 str1 janus[110770]:     #0 0x7f2fdef087f0 in __interceptor_free (/lib64/libasan.so.5+0xef7f0)
Nov 09 12:06:44 str1 janus[110770]: freed by thread T313 (hloop 845082543) here:
Nov 09 12:06:44 str1 janus[110770]: 0x60b0002617c0 is located 0 bytes inside of 112-byte region [0x60b0002617c0,0x60b000261830)
Nov 09 12:06:44 str1 janus[110770]:     #5 0x7f2fdc9f8e72 in __clone (/lib64/libc.so.6+0x39e72)
Nov 09 12:06:44 str1 janus[110770]:     #4 0x7f2fdcd8c1c9 in start_thread (/lib64/libpthread.so.0+0x81c9)
Nov 09 12:06:44 str1 janus[110770]:     #3 0x7f2fde11b4e9  (/lib64/libglib-2.0.so.0+0x764e9)
Nov 09 12:06:44 str1 janus[110770]:     #2 0x7f2fd0d86947 in janus_streaming_relay_thread plugins/janus_streaming.c:9584
Nov 09 12:06:44 str1 janus[110770]:     #1 0x7f2fde0eeda4 in g_list_foreach (/lib64/libglib-2.0.so.0+0x49da4)
Nov 09 12:06:44 str1 janus[110770]:     #0 0x7f2fd0d87feb in janus_streaming_relay_rtp_packet plugins/janus_streaming.c:10002

Q1: just for confirmation, are you using the API flag restart to force janus resending a JSEP offer (e.g. you are NOT sending an offer FROM the browser) ?

Anyway the crash has been caused by a freed session while trying to relay a packet to the same session. Since the dump mentions janus_streaming_session_destroy, this means that something has requested the plugin session teardown.

Q2: could your client be sending by mistake a destroy session or closing the transport while trying to perform an ICE restart?

atoppi commented 1 year ago

@amnonbb can you try again after applying this patch?

diff --git a/src/plugins/janus_streaming.c b/src/plugins/janus_streaming.c
index 2cadc73d..4cca8d29 100644
--- a/src/plugins/janus_streaming.c
+++ b/src/plugins/janus_streaming.c
@@ -5843,6 +5843,27 @@ static void *janus_streaming_handler(void *data) {
            /* Check if this is a new viewer, or if an update is taking place (i.e., ICE restart) */
            gboolean audio = TRUE, video = TRUE, data = TRUE;
            if(do_restart) {
+               if(session->mountpoint == NULL) {
+                   JANUS_LOG(LOG_ERR, "Can't perform ICE restart: no mountpoint set\n");
+                   error_code = JANUS_STREAMING_ERROR_NO_SUCH_MOUNTPOINT;
+                   g_snprintf(error_cause, 512, "Can't perform ICE restart: no mountpoint set");
+                   janus_mutex_unlock(&session->mutex);
+                   janus_mutex_unlock(&mp->mutex);
+                   janus_mutex_unlock(&sessions_mutex);
+                   janus_refcount_decrease(&mp->ref);
+                   goto error;
+               }
+               if(session->mountpoint != mp) {
+                   /* Already watching something else */
+                   JANUS_LOG(LOG_ERR, "Already watching mountpoint %s\n", session->mountpoint->id_str);
+                   error_code = JANUS_STREAMING_ERROR_INVALID_STATE;
+                   g_snprintf(error_cause, 512, "Already watching mountpoint %s", session->mountpoint->id_str);
+                   janus_mutex_unlock(&session->mutex);
+                   janus_mutex_unlock(&mp->mutex);
+                   janus_mutex_unlock(&sessions_mutex);
+                   janus_refcount_decrease(&mp->ref);
+                   goto error;
+               }
                /* User asked for an ICE restart: provide a new offer */
                if(!g_atomic_int_compare_and_exchange(&session->renegotiating, 0, 1)) {
                    /* Already triggered a renegotiation, and still waiting for an answer */
amnonbb commented 1 year ago
  1. I send {request: 'watch', id, restart: true}. And it's actually work.
  2. I think it's can happen. ICE restart trigger on ICE state disconnected and case maybe bad network connection.

I'll try patch and give you feedback. Thank you!

amnonbb commented 1 year ago

Looks like patch solve issue. Two days without any crash. Before this i got 5-10 crash in one day. Thank you!