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.

This commit is contained in:
Anthony Minessale 2017-05-23 12:30:50 -05:00
parent 49468d480f
commit a14651110b
2 changed files with 80 additions and 54 deletions

View File

@ -398,7 +398,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

View File

@ -35,22 +35,56 @@
#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;
if (bp->text_buffer) {
switch_buffer_destroy(&bp->text_buffer);
switch_safe_free(bp->text_framedata);
}
if (bug->raw_write_buffer) {
switch_buffer_destroy(&bug->raw_write_buffer);
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 (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);
}
}
@ -899,7 +933,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;
@ -1043,7 +1077,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;
@ -1193,6 +1227,7 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_remove_all_function(switch
{
switch_media_bug_t *bp, *last = NULL, *next = NULL;
switch_status_t status = SWITCH_STATUS_FALSE;
switch_media_bug_t *closed = NULL;
if (session->bugs) {
switch_thread_rwlock_wrlock(session->bug_rwlock);
@ -1217,12 +1252,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);
}
@ -1230,7 +1274,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;
@ -1244,55 +1288,27 @@ SWITCH_DECLARE(switch_status_t) switch_core_media_bug_close(switch_media_bug_t *
bp->callback(bp, bp->user_data, SWITCH_ABC_TYPE_CLOSE);
}
if (bp->text_buffer) {
switch_buffer_destroy(&bp->text_buffer);
switch_safe_free(bp->text_framedata);
}
if (switch_test_flag(bp, SMBF_READ_VIDEO_STREAM) || switch_test_flag(bp, SMBF_WRITE_VIDEO_STREAM) || switch_test_flag(bp, SMBF_READ_VIDEO_PING) || switch_test_flag(bp, SMBF_WRITE_VIDEO_PING)) {
switch_channel_clear_flag_recursive(bp->session->channel, CF_VIDEO_DECODED_READ);
}
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;
}
@ -1346,7 +1362,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;
@ -1386,7 +1402,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;
}
@ -1397,7 +1413,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);
@ -1413,15 +1429,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);
}