From 76ca5133e2ed75afcd9db521bdc71f319f9c215f Mon Sep 17 00:00:00 2001 From: Gilbok Lee Date: Thu, 19 Mar 2020 16:13:57 +0900 Subject: [PATCH] [0.6.219] Fix g_mutex_clear crash when destroying bus msg thread - Wait until the function registered in the gst_bus_watcher is terminated, when destroying the gst_bus_watcher Change-Id: I5cddb3c01517f5a0fbd0843147e19bd92adae55a --- packaging/libmm-player.spec | 2 +- src/include/mm_player_priv.h | 6 ++++++ src/include/mm_player_utils.h | 6 ++++++ src/mm_player.c | 2 ++ src/mm_player_gst.c | 23 ++++++++++++----------- src/mm_player_priv.c | 43 ++++++++++++++++++++++++++++++++++++++----- 6 files changed, 65 insertions(+), 17 deletions(-) diff --git a/packaging/libmm-player.spec b/packaging/libmm-player.spec index 0bbda67..c661ddb 100644 --- a/packaging/libmm-player.spec +++ b/packaging/libmm-player.spec @@ -1,6 +1,6 @@ Name: libmm-player Summary: Multimedia Framework Player Library -Version: 0.6.218 +Version: 0.6.219 Release: 0 Group: Multimedia/Libraries License: Apache-2.0 diff --git a/src/include/mm_player_priv.h b/src/include/mm_player_priv.h index 25c944c..4b141be 100644 --- a/src/include/mm_player_priv.h +++ b/src/include/mm_player_priv.h @@ -690,6 +690,10 @@ typedef struct { /* list of sink elements */ GList *sink_elements; + /* for destroy bus thread */ + GMutex bus_watcher_mutex; + GCond bus_watcher_cond; + /* signal notifiers */ GList *signals[MM_PLAYER_SIGNAL_TYPE_MAX]; guint bus_watcher; @@ -870,6 +874,8 @@ int _mmplayer_is_audio_control_available(MMHandleType hplayer, mmplayer_audio_co /* internal */ void _mmplayer_bus_msg_thread_destroy(MMHandleType hplayer); +void _mmplayer_bus_watcher_remove(MMHandleType hplayer); +void _mmplayer_watcher_removed_notify(gpointer data); void _mmplayer_set_state(mmplayer_t *player, int state); int _mmplayer_check_state(mmplayer_t *player, mmplayer_command_state_e command); gboolean _mmplayer_update_content_attrs(mmplayer_t *player, enum content_attr_flag flag); diff --git a/src/include/mm_player_utils.h b/src/include/mm_player_utils.h index 4f12889..6d09d1a 100644 --- a/src/include/mm_player_utils.h +++ b/src/include/mm_player_utils.h @@ -89,6 +89,12 @@ #define MMPLAYER_BUS_MSG_THREAD_WAIT_UNTIL(x_player, end_time) g_cond_wait_until(&((mmplayer_t *)x_player)->bus_msg_thread_cond, &((mmplayer_t *)x_player)->bus_msg_thread_mutex, end_time) #define MMPLAYER_BUS_MSG_THREAD_SIGNAL(x_player) g_cond_signal(&((mmplayer_t *)x_player)->bus_msg_thread_cond); +/* gst bus watcher thread */ +#define MMPLAYER_BUS_WATCHER_LOCK(x_player) g_mutex_lock(&((mmplayer_t *)x_player)->bus_watcher_mutex) +#define MMPLAYER_BUS_WATCHER_UNLOCK(x_player) g_mutex_unlock(&((mmplayer_t *)x_player)->bus_watcher_mutex) +#define MMPLAYER_BUS_WATCHER_WAIT_UNTIL(x_player, end_time) g_cond_wait_until(&((mmplayer_t *)x_player)->bus_watcher_cond, &((mmplayer_t *)x_player)->bus_watcher_mutex, end_time) +#define MMPLAYER_BUS_WATCHER_SIGNAL(x_player) g_cond_signal(&((mmplayer_t *)x_player)->bus_watcher_cond); + /* handling fakesink */ #define MMPLAYER_FSINK_LOCK(x_player) g_mutex_lock(&((mmplayer_t *)x_player)->fsink_lock) #define MMPLAYER_FSINK_UNLOCK(x_player) g_mutex_unlock(&((mmplayer_t *)x_player)->fsink_lock) diff --git a/src/mm_player.c b/src/mm_player.c index bcd2d7d..52feafe 100644 --- a/src/mm_player.c +++ b/src/mm_player.c @@ -102,6 +102,7 @@ int mm_player_destroy(MMHandleType player) MMPLAYER_RETURN_VAL_IF_FAIL(player, MM_ERROR_PLAYER_NOT_INITIALIZED); + _mmplayer_bus_watcher_remove(player); /* destroy the gst bus msg thread if it is remained. this funct have to be called before getting cmd lock. */ _mmplayer_bus_msg_thread_destroy(player); @@ -145,6 +146,7 @@ int mm_player_abort_pause(MMHandleType player) MMPLAYER_RETURN_VAL_IF_FAIL(player, MM_ERROR_PLAYER_NOT_INITIALIZED); + _mmplayer_bus_watcher_remove(player); /* destroy the gst bus msg thread not to be blocked in pause(without cmd lock). */ _mmplayer_bus_msg_thread_destroy(player); diff --git a/src/mm_player_gst.c b/src/mm_player_gst.c index d6f8567..7978600 100644 --- a/src/mm_player_gst.c +++ b/src/mm_player_gst.c @@ -2785,7 +2785,6 @@ __mmplayer_gst_msg_push(GstBus *bus, GstMessage *msg, gpointer data) g_return_val_if_fail(player, FALSE); g_return_val_if_fail(msg && GST_IS_MESSAGE(msg), FALSE); - gst_message_ref(msg); g_mutex_lock(&player->bus_msg_q_lock); @@ -2802,7 +2801,6 @@ static gpointer __mmplayer_gst_bus_msg_thread(gpointer data) { mmplayer_t *player = (mmplayer_t *)(data); GstMessage *msg = NULL; - GstBus *bus = NULL; MMPLAYER_FENTER(); MMPLAYER_RETURN_VAL_IF_FAIL(player && @@ -2811,12 +2809,6 @@ static gpointer __mmplayer_gst_bus_msg_thread(gpointer data) player->pipeline->mainbin[MMPLAYER_M_PIPE].gst, NULL); - bus = gst_pipeline_get_bus(GST_PIPELINE(player->pipeline->mainbin[MMPLAYER_M_PIPE].gst)); - if (!bus) { - LOGE("cannot get BUS from the pipeline"); - return NULL; - } - MMPLAYER_BUS_MSG_THREAD_LOCK(player); LOGD("[handle: %p] gst bus msg thread will be started.", player); @@ -2836,9 +2828,8 @@ static gpointer __mmplayer_gst_bus_msg_thread(gpointer data) } MMPLAYER_BUS_MSG_THREAD_UNLOCK(player); - gst_object_unref(GST_OBJECT(bus)); - MMPLAYER_FLEAVE(); + return NULL; } @@ -3899,7 +3890,17 @@ _mmplayer_gst_add_bus_watch(mmplayer_t *player) return MM_ERROR_PLAYER_INTERNAL; } - player->bus_watcher = gst_bus_add_watch(bus, (GstBusFunc)__mmplayer_gst_msg_push, player); + player->bus_watcher = gst_bus_add_watch_full(bus, G_PRIORITY_DEFAULT, + (GstBusFunc)__mmplayer_gst_msg_push, player, + (GDestroyNotify)_mmplayer_watcher_removed_notify); + if (player->bus_watcher == 0) { + LOGE("failed to add bus watch"); + return MM_ERROR_PLAYER_INTERNAL; + } + + g_mutex_init(&player->bus_watcher_mutex); + g_cond_init(&player->bus_watcher_cond); + player->context.thread_default = g_main_context_get_thread_default(); if (player->context.thread_default == NULL) { player->context.thread_default = g_main_context_default(); diff --git a/src/mm_player_priv.c b/src/mm_player_priv.c index 403cf19..b1559a8 100644 --- a/src/mm_player_priv.c +++ b/src/mm_player_priv.c @@ -756,6 +756,42 @@ __mmplayer_remove_g_source_from_context(GMainContext *context, guint source_id) } void +_mmplayer_watcher_removed_notify(gpointer data) +{ + mmplayer_t *player = (mmplayer_t *)data; + MMPLAYER_RETURN_IF_FAIL(player); + + MMPLAYER_BUS_WATCHER_LOCK(player); + player->bus_watcher = 0; + MMPLAYER_BUS_WATCHER_SIGNAL(player); + MMPLAYER_BUS_WATCHER_UNLOCK(player); +} + +void +_mmplayer_bus_watcher_remove(MMHandleType hplayer) +{ + mmplayer_t *player = (mmplayer_t *)hplayer; + gint64 end_time = 0; + MMPLAYER_FENTER(); + MMPLAYER_RETURN_IF_FAIL(player); + + /* disconnecting bus watch */ + if (player->bus_watcher > 0) { + __mmplayer_remove_g_source_from_context(player->context.thread_default, player->bus_watcher); + MMPLAYER_BUS_WATCHER_LOCK(player); + end_time = g_get_monotonic_time () + 2 * G_TIME_SPAN_SECOND; + while (player->bus_watcher > 0) + MMPLAYER_BUS_WATCHER_WAIT_UNTIL(player, end_time); + MMPLAYER_BUS_WATCHER_UNLOCK(player); + + g_mutex_clear(&player->bus_watcher_mutex); + g_cond_clear(&player->bus_watcher_cond); + } + + MMPLAYER_FLEAVE(); +} + +void _mmplayer_bus_msg_thread_destroy(MMHandleType hplayer) { mmplayer_t *player = (mmplayer_t *)hplayer; @@ -765,11 +801,6 @@ _mmplayer_bus_msg_thread_destroy(MMHandleType hplayer) MMPLAYER_FENTER(); MMPLAYER_RETURN_IF_FAIL(player); - /* disconnecting bus watch */ - if (player->bus_watcher) - __mmplayer_remove_g_source_from_context(player->context.thread_default, player->bus_watcher); - player->bus_watcher = 0; - /* destroy the gst bus msg thread */ if (player->bus_msg_thread) { MMPLAYER_BUS_MSG_THREAD_LOCK(player); @@ -4181,6 +4212,7 @@ __mmplayer_gst_create_pipeline(mmplayer_t *player) return MM_ERROR_NONE; INIT_ERROR: + _mmplayer_bus_watcher_remove(player); __mmplayer_gst_destroy_pipeline(player); return MM_ERROR_PLAYER_INTERNAL; } @@ -5107,6 +5139,7 @@ _mmplayer_unrealize(MMHandleType hplayer) MMPLAYER_RETURN_VAL_IF_FAIL(player, MM_ERROR_PLAYER_NOT_INITIALIZED); MMPLAYER_CMD_UNLOCK(player); + _mmplayer_bus_watcher_remove(player); /* destroy the gst bus msg thread which is created during realize. this funct have to be called before getting cmd lock. */ _mmplayer_bus_msg_thread_destroy(player); -- 2.7.4