FS-10231: [freeswitch-core] Some media bugs not fully cleaned up when session is…

FS-10231: [freeswitch-core] Some media bugs not fully cleaned up when session is destroyed #comment Regression causing deadlock when holding write lock and close/destroying all the bugs but the video recording thread tries to read lock.  Separating destroy out so it can be called outside the lock after the bugs are detached.

src/include/switch_core.h

@@ -396,7 +396,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_callback(switch_cor
   \param bug bug to remove
   \return SWITCH_STATUS_SUCCESS if the operation was a success
 */
-SWITCH_DECLARE(switch_status_t) switch_core_media_bug_close(_Inout_ switch_media_bug_t **bug);
+SWITCH_DECLARE(switch_status_t) switch_core_media_bug_close(_Inout_ switch_media_bug_t **bug, switch_bool_t destroy);
 /*!
   \brief Remove all media bugs from the session
   \param session the session to remove the bugs from

src/switch_core_media_bug.c

@@ -35,22 +35,51 @@
 #include "switch.h"
 #include "private/switch_core_pvt.h"
 
-static void switch_core_media_bug_destroy(switch_media_bug_t *bug)
+static void switch_core_media_bug_destroy(switch_media_bug_t **bug)
 {
 	switch_event_t *event = NULL;
+	switch_media_bug_t *bp = *bug;
 
-	if (bug->raw_read_buffer) {
-		switch_buffer_destroy(&bug->raw_read_buffer);
+	*bug = NULL;
+
+	switch_img_free(&bp->spy_img[0]);
+	switch_img_free(&bp->spy_img[1]);
+
+	if (bp->video_bug_thread) {
+		switch_status_t st;
+		int i;
+
+		for (i = 0; i < 2; i++) {
+			void *pop;
+			switch_image_t *img;
+
+			if (bp->spy_video_queue[i]) {
+				while (switch_queue_trypop(bp->spy_video_queue[i], &pop) == SWITCH_STATUS_SUCCESS && pop) {
+					img = (switch_image_t *) pop;
+					switch_img_free(&img);
+				}
+			}
+		}
+
+		switch_thread_join(&st, bp->video_bug_thread);
 	}
 
-	if (bug->raw_write_buffer) {
-		switch_buffer_destroy(&bug->raw_write_buffer);
+	if (switch_test_flag(bp, SMBF_READ_VIDEO_PATCH) && bp->session->video_read_codec) {
+		switch_clear_flag(bp->session->video_read_codec, SWITCH_CODEC_FLAG_VIDEO_PATCHING);
+	}
+
+	if (bp->raw_read_buffer) {
+		switch_buffer_destroy(&bp->raw_read_buffer);
+	}
+
+	if (bp->raw_write_buffer) {
+		switch_buffer_destroy(&bp->raw_write_buffer);
 	}
 
 	if (switch_event_create(&event, SWITCH_EVENT_MEDIA_BUG_STOP) == SWITCH_STATUS_SUCCESS) {
-		switch_event_add_header(event, SWITCH_STACK_BOTTOM, "Media-Bug-Function", "%s", bug->function);
-		switch_event_add_header(event, SWITCH_STACK_BOTTOM, "Media-Bug-Target", "%s", bug->target);
-		if (bug->session) switch_channel_event_set_data(bug->session->channel, event);
+		switch_event_add_header(event, SWITCH_STACK_BOTTOM, "Media-Bug-Function", "%s", bp->function);
+		switch_event_add_header(event, SWITCH_STACK_BOTTOM, "Media-Bug-Target", "%s", bp->target);
+		if (bp->session) switch_channel_event_set_data(bp->session->channel, event);
 		switch_event_fire(&event);
 	}
 }
@@ -887,7 +916,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_add(switch_core_session_t
 	if (bug->callback) {
 		switch_bool_t result = bug->callback(bug, bug->user_data, SWITCH_ABC_TYPE_INIT);
 		if (result == SWITCH_FALSE) {
-			switch_core_media_bug_destroy(bug);
+			switch_core_media_bug_destroy(&bug);
 			switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(session), SWITCH_LOG_ERROR, "Error attaching BUG to %s\n",
 							  switch_channel_get_name(session->channel));
 			return SWITCH_STATUS_GENERR;
@@ -1031,7 +1060,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_transfer_callback(switch_c
 			switch_core_media_bug_add(new_session, cur->function, cur->target, cur->callback,
 									  user_data_dup_func(new_session, cur->user_data),
 									  cur->stop_time, cur->flags, &new_bug);
-			switch_core_media_bug_destroy(cur);
+			switch_core_media_bug_destroy(&cur);
 			total++;
 		} else {
 			last = cur;
@@ -1181,6 +1210,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_all_function(switch
 {
 	switch_media_bug_t *bp, *last = NULL;
 	switch_status_t status = SWITCH_STATUS_FALSE;
+	switch_media_bug_t *closed = NULL;
 
 	if (session->bugs) {
 		switch_thread_rwlock_wrlock(session->bug_rwlock);
@@ -1203,12 +1233,21 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_all_function(switch
 				session->bugs = bp->next;
 			}
 
-			switch_core_media_bug_close(&bp);
+			bp->next = closed;
+			closed = bp;
+
+			switch_core_media_bug_close(&bp, SWITCH_FALSE);
 		}
 		switch_thread_rwlock_unlock(session->bug_rwlock);
 		status = SWITCH_STATUS_SUCCESS;
 	}
 
+	if (closed) {
+		for (bp = session->bugs; bp; bp = bp->next) {
+			switch_core_media_bug_destroy(&bp);
+		}
+	}
+
 	if (switch_core_codec_ready(&session->bug_codec)) {
 		switch_core_codec_destroy(&session->bug_codec);
 	}
@@ -1216,7 +1255,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_all_function(switch
 	return status;
 }
 
-SWITCH_DECLARE(switch_status_t) switch_core_media_bug_close(switch_media_bug_t **bug)
+SWITCH_DECLARE(switch_status_t) switch_core_media_bug_close(switch_media_bug_t **bug, switch_bool_t destroy)
 {
 	switch_media_bug_t *bp = *bug;
 
@@ -1236,44 +1275,21 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_close(switch_media_bug_t *
 
 		bp->ready = 0;
 
-		switch_img_free(&bp->spy_img[0]);
-		switch_img_free(&bp->spy_img[1]);
-
-		if (bp->video_bug_thread) {
-			switch_status_t st;
-			int i;
-
-			for (i = 0; i < 2; i++) {
-				void *pop;
-				switch_image_t *img;
-
-				if (bp->spy_video_queue[i]) {
-					while (switch_queue_trypop(bp->spy_video_queue[i], &pop) == SWITCH_STATUS_SUCCESS && pop) {
-						img = (switch_image_t *) pop;
-						switch_img_free(&img);
-					}
-				}
-			}
-
-			if (bp->read_video_queue) {
-				switch_queue_push(bp->read_video_queue, NULL);
-			}
-			
-			if (bp->write_video_queue) {
-				switch_queue_push(bp->write_video_queue, NULL);
-			}
-
-			switch_thread_join(&st, bp->video_bug_thread);
+		if (bp->read_video_queue) {
+			switch_queue_push(bp->read_video_queue, NULL);
 		}
-
-		if (switch_test_flag(bp, SMBF_READ_VIDEO_PATCH) && bp->session->video_read_codec) {
-			switch_clear_flag(bp->session->video_read_codec, SWITCH_CODEC_FLAG_VIDEO_PATCHING);
+			
+		if (bp->write_video_queue) {
+			switch_queue_push(bp->write_video_queue, NULL);
 		}
 
-		switch_core_media_bug_destroy(bp);
 		switch_log_printf(SWITCH_CHANNEL_SESSION_LOG(switch_core_media_bug_get_session(*bug)), SWITCH_LOG_DEBUG, "Removing BUG from %s\n",
 						  switch_channel_get_name(bp->session->channel));
-		*bug = NULL;
+
+		if (destroy) {
+			switch_core_media_bug_destroy(bug);
+		}
+
 		return SWITCH_STATUS_SUCCESS;
 	}
 
@@ -1327,7 +1343,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove(switch_core_session
 	switch_thread_rwlock_unlock(session->bug_rwlock);
 
 	if (bp) {
-		status = switch_core_media_bug_close(&bp);
+		status = switch_core_media_bug_close(&bp, SWITCH_TRUE);
 	}
 	
 	return status;
@@ -1367,7 +1383,7 @@ SWITCH_DECLARE(uint32_t) switch_core_media_bug_prune(switch_core_session_t *sess
 	if (bp) {
 		switch_clear_flag(bp, SMBF_LOCK);
 		bp->thread_id = 0;
-		switch_core_media_bug_close(&bp);
+		switch_core_media_bug_close(&bp, SWITCH_TRUE);
 		ttl++;
 		goto top;
 	}
@@ -1378,7 +1394,7 @@ SWITCH_DECLARE(uint32_t) switch_core_media_bug_prune(switch_core_session_t *sess
 
 SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_callback(switch_core_session_t *session, switch_media_bug_callback_t callback)
 {
-	switch_media_bug_t *cur = NULL, *bp = NULL, *last = NULL;
+	switch_media_bug_t *cur = NULL, *bp = NULL, *last = NULL, *closed = NULL;
 	int total = 0;
 
 	switch_thread_rwlock_wrlock(session->bug_rwlock);
@@ -1394,15 +1410,25 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_callback(switch_cor
 				} else {
 					session->bugs = cur->next;
 				}
-				if (switch_core_media_bug_close(&cur) == SWITCH_STATUS_SUCCESS) {
+				if (switch_core_media_bug_close(&cur, SWITCH_FALSE) == SWITCH_STATUS_SUCCESS) {
 					total++;
 				}
+
+				cur->next = closed;
+				closed = cur;
+
 			} else {
 				last = cur;
 			}
 		}
 	}
 
+	if (closed) {
+		for (bp = session->bugs; bp; bp = bp->next) {
+			switch_core_media_bug_destroy(&bp);
+		}
+	}
+
 	if (!session->bugs && switch_core_codec_ready(&session->bug_codec)) {
 		switch_core_codec_destroy(&session->bug_codec);
 	}