From c917ee6d10d53a55a127be8d02108d537d4a1e68 Mon Sep 17 00:00:00 2001 From: Alessandro Toppi Date: Tue, 12 Nov 2024 11:31:53 +0100 Subject: [PATCH] Fix references counting for remote publisher and streams (see #3359) (#3475) --- src/plugins/janus_videoroom.c | 45 ++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-) diff --git a/src/plugins/janus_videoroom.c b/src/plugins/janus_videoroom.c index 9385013b9c..77c67d876e 100644 --- a/src/plugins/janus_videoroom.c +++ b/src/plugins/janus_videoroom.c @@ -2799,7 +2799,10 @@ static void janus_videoroom_session_destroy(janus_videoroom_session *session) { static void janus_videoroom_session_free(const janus_refcount *session_ref) { janus_videoroom_session *session = janus_refcount_containerof(session_ref, janus_videoroom_session, ref); /* Remove the reference to the core plugin session */ - janus_refcount_decrease(&session->handle->ref); + if(session->handle) { + /* Could be NULL for dummy publishers */ + janus_refcount_decrease(&session->handle->ref); + } /* This session can be destroyed, free all the resources */ janus_mutex_destroy(&session->mutex); g_free(session); @@ -7290,6 +7293,7 @@ static json_t *janus_videoroom_process_synchronous_request(janus_videoroom_sessi json_object_set_new(response, "id", string_ids ? json_string(publisher->user_id_str) : json_integer(publisher->user_id)); json_object_set_new(response, "remote_id", json_string(remote_id)); janus_refcount_decrease(&publisher->ref); /* This is just to handle the request for now */ + janus_refcount_decrease(&videoroom->ref); goto prepare_response; } else if(!strcasecmp(request_text, "unpublish_remotely")) { /* Configure a local publisher to stop restreaming to a remote VideoRomm instance */ @@ -7833,8 +7837,6 @@ static json_t *janus_videoroom_process_synchronous_request(janus_videoroom_sessi mindex++; } /* Done, spawn a thread for this remote publisher */ - janus_refcount_increase(&publisher->ref); - janus_refcount_increase(&publisher->session->ref); GError *error = NULL; char tname[16]; g_snprintf(tname, sizeof(tname), "vremote %s", publisher->user_id_str); @@ -7843,9 +7845,15 @@ static json_t *janus_videoroom_process_synchronous_request(janus_videoroom_sessi /* Something went wrong */ janus_mutex_unlock(&videoroom->mutex); janus_refcount_decrease(&videoroom->ref); - janus_videoroom_publisher_destroy(publisher); - janus_refcount_decrease(&publisher->ref); + janus_mutex_lock(&publisher->streams_mutex); + g_list_free_full(publisher->streams, (GDestroyNotify)(janus_videoroom_publisher_stream_unref)); + publisher->streams = NULL; + g_hash_table_remove_all(publisher->streams_byid); + g_hash_table_remove_all(publisher->streams_bymid); + janus_mutex_unlock(&publisher->streams_mutex); + janus_videoroom_leave_or_unpublish(publisher, TRUE, FALSE); janus_refcount_decrease(&publisher->session->ref); + janus_videoroom_publisher_destroy(publisher); JANUS_LOG(LOG_ERR, "Could not spawn thread for remote publisher, %d (%s)\n", errno, g_strerror(errno)); error_code = JANUS_VIDEOROOM_ERROR_UNKNOWN_ERROR; @@ -13491,6 +13499,11 @@ static void *janus_videoroom_remote_publisher_thread(void *user_data) { JANUS_LOG(LOG_VERB, "[%s/%s] Joining remote publisher thread...\n", publisher->room->room_id_str, publisher->user_id_str); + janus_videoroom *videoroom = publisher->room; + janus_refcount_increase(&videoroom->ref); + janus_refcount_increase(&publisher->ref); + janus_refcount_increase(&publisher->session->ref); + /* File descriptors */ socklen_t addrlen; struct sockaddr_storage remote = { 0 }; @@ -13504,12 +13517,10 @@ static void *janus_videoroom_remote_publisher_thread(void *user_data) { * and/or we may never be notified about sessions being closed, so give up */ JANUS_LOG(LOG_WARN, "[%s/%s] Leaving remote publisher thread, no pipe file descriptor...\n", publisher->room->room_id_str, publisher->user_id_str); - janus_videoroom_publisher_destroy(publisher); - janus_refcount_decrease(&publisher->session->ref); - janus_refcount_decrease(&publisher->ref); - g_thread_unref(g_thread_self()); - return NULL; + janus_videoroom_publisher_dereference(publisher); + goto cleanup; } + /* RTP stuff */ janus_rtp_header *rtp = NULL; uint32_t ssrc = 0, diff = 0; @@ -13520,10 +13531,6 @@ static void *janus_videoroom_remote_publisher_thread(void *user_data) { GList *temp = NULL; /* As the first thing, we add the remote publisher to the list */ - janus_refcount_increase(&publisher->ref); - janus_refcount_increase(&publisher->session->ref); - janus_videoroom *videoroom = publisher->room; - janus_refcount_increase(&videoroom->ref); janus_mutex_lock(&videoroom->mutex); g_hash_table_insert(videoroom->participants, string_ids ? (gpointer)g_strdup(publisher->user_id_str) : (gpointer)janus_uint64_dup(publisher->user_id), @@ -13714,6 +13721,7 @@ static void *janus_videoroom_remote_publisher_thread(void *user_data) { } } } +cleanup: /* If we got here, the remote publisher has been removed from the * room: let's notify all other publishers in the room */ janus_mutex_lock(&publisher->rec_mutex); @@ -13810,21 +13818,20 @@ static void *janus_videoroom_remote_publisher_thread(void *user_data) { temp = temp->next; } } + JANUS_LOG(LOG_VERB, "[%s/%s] Leaving remote publisher thread...\n", + videoroom->room_id_str, publisher->user_id_str); g_list_free(subscribers); /* Free streams */ - g_list_free(publisher->streams); + g_list_free_full(publisher->streams, (GDestroyNotify)(janus_videoroom_publisher_stream_unref)); publisher->streams = NULL; g_hash_table_remove_all(publisher->streams_byid); g_hash_table_remove_all(publisher->streams_bymid); janus_mutex_unlock(&publisher->streams_mutex); janus_videoroom_leave_or_unpublish(publisher, TRUE, FALSE); + janus_refcount_decrease(&publisher->session->ref); janus_videoroom_publisher_destroy(publisher); /* Done */ - JANUS_LOG(LOG_VERB, "[%s/%s] Leaving remote publisher thread...\n", - videoroom->room_id_str, publisher->user_id_str); janus_refcount_decrease(&videoroom->ref); - janus_refcount_decrease(&publisher->session->ref); - janus_refcount_decrease(&publisher->ref); g_thread_unref(g_thread_self()); return NULL; }