From 02539c6c4faedbc2e6d978e949273f7851174a42 Mon Sep 17 00:00:00 2001 From: Brian West Date: Tue, 21 Jul 2015 15:43:06 -0500 Subject: [PATCH] FS-7861, FS-7862: [mod_conference] fix crash and other issues caused by multi canvas feature --- .../autoload_configs/conference.conf.xml | 31 +++++++++++ .../conference_layouts.conf.xml | 4 ++ conf/testing/dialplan/default/0019_conf.xml | 6 +++ .../mod_conference/conference_api.c | 40 ++++++--------- .../mod_conference/conference_video.c | 51 +++++++++++++++++++ .../mod_conference/mod_conference.h | 3 ++ 6 files changed, 110 insertions(+), 25 deletions(-) diff --git a/conf/testing/autoload_configs/conference.conf.xml b/conf/testing/autoload_configs/conference.conf.xml index 00c20af3de..6891466f71 100644 --- a/conf/testing/autoload_configs/conference.conf.xml +++ b/conf/testing/autoload_configs/conference.conf.xml @@ -261,6 +261,37 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/conf/testing/autoload_configs/conference_layouts.conf.xml b/conf/testing/autoload_configs/conference_layouts.conf.xml index 3620f64011..a074a2e3cb 100644 --- a/conf/testing/autoload_configs/conference_layouts.conf.xml +++ b/conf/testing/autoload_configs/conference_layouts.conf.xml @@ -12,6 +12,10 @@ + + + + diff --git a/conf/testing/dialplan/default/0019_conf.xml b/conf/testing/dialplan/default/0019_conf.xml index 41189353e2..e09949843e 100644 --- a/conf/testing/dialplan/default/0019_conf.xml +++ b/conf/testing/dialplan/default/0019_conf.xml @@ -88,4 +88,10 @@ + + + + + + diff --git a/src/mod/applications/mod_conference/conference_api.c b/src/mod/applications/mod_conference/conference_api.c index db93f0abbb..f84d0a7518 100644 --- a/src/mod/applications/mod_conference/conference_api.c +++ b/src/mod/applications/mod_conference/conference_api.c @@ -488,11 +488,11 @@ switch_status_t conference_api_sub_unvmute(conference_member_t *member, switch_s return SWITCH_STATUS_SUCCESS; } - if (member->conference->canvas) { - switch_mutex_lock(member->conference->canvas->mutex); - layer = &member->conference->canvas->layers[member->video_layer_id]; + layer = conference_video_get_layer_locked(member); + + if (layer) { conference_video_clear_layer(layer); - switch_mutex_unlock(member->conference->canvas->mutex); + conference_video_release_layer(&layer); } conference_utils_member_set_flag_locked(member, MFLAG_CAN_BE_SEEN); @@ -1419,14 +1419,13 @@ switch_status_t conference_api_sub_vid_mute_img(conference_member_t *member, swi return SWITCH_STATUS_FALSE; } - switch_mutex_lock(layer->canvas->mutex); + layer = conference_video_get_layer_locked(member); - if (member->video_layer_id == -1 || !layer->canvas) { + if (!layer) { goto end; } member->video_mute_png = NULL; - layer = &layer->canvas->layers[member->video_layer_id]; if (text) { switch_img_free(&layer->mute_img); @@ -1436,11 +1435,11 @@ switch_status_t conference_api_sub_vid_mute_img(conference_member_t *member, swi member->video_mute_png = switch_core_strdup(member->pool, text); } - end: +end: stream->write_function(stream, "%s\n", member->video_mute_png ? member->video_mute_png : "_undef_"); - switch_mutex_unlock(layer->canvas->mutex); + conference_video_release_layer(&layer); return SWITCH_STATUS_SUCCESS; @@ -1459,16 +1458,12 @@ switch_status_t conference_api_sub_vid_logo_img(conference_member_t *member, swi return SWITCH_STATUS_FALSE; } - if (member->video_layer_id == -1 || !member->conference->canvas) { + layer = conference_video_get_layer_locked(member); + + if (!layer) { goto end; } - - - layer = &member->conference->canvas->layers[member->video_layer_id]; - - switch_mutex_lock(layer->canvas->mutex); - if (strcasecmp(text, "clear")) { member->video_logo = switch_core_strdup(member->pool, text); } @@ -1479,7 +1474,7 @@ switch_status_t conference_api_sub_vid_logo_img(conference_member_t *member, swi stream->write_function(stream, "+OK\n"); - switch_mutex_unlock(layer->canvas->mutex); + conference_video_release_layer(&layer); return SWITCH_STATUS_SUCCESS; @@ -1488,7 +1483,6 @@ switch_status_t conference_api_sub_vid_logo_img(conference_member_t *member, swi switch_status_t conference_api_sub_vid_res_id(conference_member_t *member, switch_stream_handle_t *stream, void *data) { char *text = (char *) data; - //mcu_layer_t *layer = NULL; if (member == NULL) return SWITCH_STATUS_GENERR; @@ -1509,8 +1503,6 @@ switch_status_t conference_api_sub_vid_res_id(conference_member_t *member, switc switch_mutex_lock(member->conference->canvas->mutex); - //layer = &member->conference->canvas->layers[member->video_layer_id]; - if (!strcasecmp(text, "clear") || (member->video_reservation_id && !strcasecmp(text, member->video_reservation_id))) { member->video_reservation_id = NULL; stream->write_function(stream, "+OK reservation_id cleared\n"); @@ -1542,9 +1534,9 @@ switch_status_t conference_api_sub_vid_banner(conference_member_t *member, switc return SWITCH_STATUS_SUCCESS; } - switch_mutex_lock(member->conference->mutex); + layer = conference_video_get_layer_locked(member); - if (member->video_layer_id == -1 || !member->conference->canvas) { + if (!layer) { stream->write_function(stream, "Channel %s is not in a video layer\n", switch_channel_get_name(member->channel)); goto end; } @@ -1554,8 +1546,6 @@ switch_status_t conference_api_sub_vid_banner(conference_member_t *member, switc goto end; } - layer = &member->conference->canvas->layers[member->video_layer_id]; - member->video_banner_text = switch_core_strdup(member->pool, text); conference_video_layer_set_banner(member, layer, NULL); @@ -1564,7 +1554,7 @@ switch_status_t conference_api_sub_vid_banner(conference_member_t *member, switc end: - switch_mutex_unlock(member->conference->mutex); + conference_video_release_layer(&layer); return SWITCH_STATUS_SUCCESS; } diff --git a/src/mod/applications/mod_conference/conference_video.c b/src/mod/applications/mod_conference/conference_video.c index 106b7fdc3b..805f374b95 100644 --- a/src/mod/applications/mod_conference/conference_video.c +++ b/src/mod/applications/mod_conference/conference_video.c @@ -521,6 +521,57 @@ void conference_video_check_used_layers(mcu_canvas_t *canvas) } } +mcu_layer_t *conference_video_get_layer_locked(conference_member_t *member) +{ + mcu_layer_t *layer = NULL; + mcu_canvas_t *canvas = NULL; + + if (!member || member->canvas_id < 0 || member->video_layer_id < 0) return NULL; + + switch_mutex_lock(member->conference->canvas_mutex); + + canvas = member->conference->canvases[member->canvas_id]; + + if (!canvas) { + goto end; + } + + switch_mutex_lock(canvas->mutex); + layer = &canvas->layers[member->video_layer_id]; + + if (!layer) { + switch_mutex_unlock(canvas->mutex); + } + + end: + + if (!layer) { + switch_mutex_unlock(member->conference->canvas_mutex); + } + + return layer; +} + +void conference_video_release_layer(mcu_layer_t **layer) +{ + mcu_canvas_t *canvas = NULL; + + if (!layer || !*layer) return; + + canvas = (*layer)->canvas; + + if (!canvas) return; + + switch_mutex_unlock(canvas->mutex); + + switch_assert(canvas->conference); + + switch_mutex_unlock(canvas->conference->canvas_mutex); + + *layer = NULL; +} + + void conference_video_detach_video_layer(conference_member_t *member) { mcu_layer_t *layer = NULL; diff --git a/src/mod/applications/mod_conference/mod_conference.h b/src/mod/applications/mod_conference/mod_conference.h index bbaa615159..0c4c15412b 100644 --- a/src/mod/applications/mod_conference/mod_conference.h +++ b/src/mod/applications/mod_conference/mod_conference.h @@ -866,6 +866,9 @@ void conference_utils_clear_flag(conference_obj_t *conference, conference_flag_t void conference_utils_clear_flag_locked(conference_obj_t *conference, conference_flag_t flag); switch_status_t conference_loop_dmachine_dispatcher(switch_ivr_dmachine_match_t *match); +mcu_layer_t *conference_video_get_layer_locked(conference_member_t *member); +void conference_video_release_layer(mcu_layer_t **layer); + int conference_member_setup_media(conference_member_t *member, conference_obj_t *conference); al_handle_t *conference_al_create(switch_memory_pool_t *pool);