From: Sangchul Lee <sangchul1011@gmail.com>
Date: Tue, 2 Nov 2021 05:34:29 +0000 (+0900)
Subject: Change focus_lock init/lock/unlock/clear location
X-Git-Tag: submit/tizen/20211108.065142^0
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=f7da0699df1a48ecfa6863a3b0ef99c31d9464dc;p=platform%2Fcore%2Fmultimedia%2Flibmm-sound.git

Change focus_lock init/lock/unlock/clear location

These moves make a improvement for race condition between focus
state callback and destroying handle. It fixes an abort issue from
unlocking mutex that is already unlocked.

'is_destroying' variable is added and is set to return earlier in the
focus state callback.

[Version] 0.13.14
[Issue Type] Improvement

Change-Id: Ic71341fef3e7e0f1d40f55a6bd470ed6bb423651
---

diff --git a/include/mm_sound_focus_private.h b/include/mm_sound_focus_private.h
index 834ec3a..bd643db 100644
--- a/include/mm_sound_focus_private.h
+++ b/include/mm_sound_focus_private.h
@@ -50,6 +50,7 @@ typedef struct {
 	mm_sound_focus_changed_watch_cb watch_callback;
 	void* user_data;
 	bool unset_watch_callback_requested;
+	bool is_destroying;
 } focus_sound_info_t;
 
 typedef struct {
diff --git a/mm_sound_client.c b/mm_sound_client.c
index 9097b76..8974fa0 100644
--- a/mm_sound_client.c
+++ b/mm_sound_client.c
@@ -1019,6 +1019,7 @@ int mm_sound_client_register_focus(int pid, const char *stream_type,
 		goto cleanup;
 	}
 
+	g_mutex_init(&g_focus_sound_handle[index].focus_lock);
 	focus_init_callback(index, false);
 
 cleanup:
@@ -1046,12 +1047,9 @@ int mm_sound_client_unregister_focus(int id)
 		goto cleanup;
 	}
 
-	if (!g_mutex_trylock(&g_focus_sound_handle[index].focus_lock)) {
-		debug_warning("maybe focus_callback is being called, try one more time..");
-		usleep(2500000); /* 2.5 sec */
-		if (g_mutex_trylock(&g_focus_sound_handle[index].focus_lock))
-			debug_msg("finally got focus_lock");
-	}
+	g_mutex_lock(&g_focus_sound_handle[index].focus_lock);
+
+	g_focus_sound_handle[index].is_destroying = true;
 
 	ret = mm_sound_proxy_unregister_focus(index);
 	if (ret == MM_ERROR_NONE)
@@ -1059,8 +1057,6 @@ int mm_sound_client_unregister_focus(int id)
 	else
 		debug_error("Error occurred : 0x%x", ret);
 
-	g_mutex_unlock(&g_focus_sound_handle[index].focus_lock);
-
 	focus_deinit_callback(index, false);
 	g_focus_sound_handle[index].focus_fd = 0;
 	g_focus_sound_handle[index].focus_pid = 0;
@@ -1069,6 +1065,10 @@ int mm_sound_client_unregister_focus(int id)
 	g_focus_sound_handle[index].is_used = false;
 	focus_deinit_context(index, false);
 
+	g_mutex_unlock(&g_focus_sound_handle[index].focus_lock);
+	g_mutex_clear(&g_focus_sound_handle[index].focus_lock);
+
+	g_focus_sound_handle[index].is_destroying = false;
 cleanup:
 	MMSOUND_LEAVE_CRITICAL_SECTION(&g_index_mutex);
 	debug_fleave();
diff --git a/mm_sound_focus_private.c b/mm_sound_focus_private.c
index d923720..8226da2 100644
--- a/mm_sound_focus_private.c
+++ b/mm_sound_focus_private.c
@@ -92,7 +92,7 @@ static gboolean _focus_fd_check(GSource * source)
 		return FALSE;
 	}
 #ifdef __DEBUG__
-	debug_warning("CHECK : %p, 0x%x ]", source, fsource->pollfd.revents);
+	debug_warning("CHECK : %p, 0x%x ]", source, fsource->poll_fd.revents);
 #endif
 	if (fsource->poll_fd.revents & (POLLIN | POLLPRI))
 		return TRUE;
@@ -129,6 +129,12 @@ static gboolean _focus_callback_handler(gpointer user_data)
 		debug_error("fsrc is null");
 		return G_SOURCE_REMOVE;
 	}
+	if (focus_handle->is_destroying) {
+		debug_warning("focus_handle is destroying now, skip it");
+		return G_SOURCE_REMOVE;
+	}
+
+	g_mutex_lock(&focus_handle->focus_lock);
 
 	poll_fd = &focus_handle->fsrc->poll_fd;
 	debug_msg("focus_handle(%p), poll_fd(%p)", focus_handle, poll_fd);
@@ -143,12 +149,11 @@ static gboolean _focus_callback_handler(gpointer user_data)
 			char str_error[256];
 			strerror_r(errno, str_error, sizeof(str_error));
 			debug_error("GpollFD read fail, errno=%d(%s)", errno, str_error);
+			g_mutex_unlock(&focus_handle->focus_lock);
 			return G_SOURCE_CONTINUE;
 		}
 		changed_state = cb_data.state;
 
-		g_mutex_lock(&focus_handle->focus_lock);
-
 		tid = focus_handle->focus_pid;
 
 		if (changed_state != -1) {
@@ -440,8 +445,6 @@ static bool _focus_add_sound_callback(int index, focus_callback_handler_t focus_
 
 	debug_fenter();
 
-	g_mutex_init(&g_focus_sound_handle[index].focus_lock);
-
 	src = g_source_new(&event_funcs, sizeof(FocusSource));
 	if (!src) {
 		debug_error("failed to g_source_new for focus source");
@@ -496,8 +499,6 @@ static bool _focus_remove_sound_callback(int index)
 	h->focus_callback = NULL;
 	h->watch_callback = NULL;
 
-	g_mutex_clear(&h->focus_lock);
-
 	debug_msg("callbacks of focus handle[%d] are cleared", index);
 
 	return true;
diff --git a/packaging/libmm-sound.spec b/packaging/libmm-sound.spec
index 1f3ba30..985aa61 100644
--- a/packaging/libmm-sound.spec
+++ b/packaging/libmm-sound.spec
@@ -1,6 +1,6 @@
 Name:       libmm-sound
 Summary:    MMSound Package contains client lib and focus server binary
-Version:    0.13.13
+Version:    0.13.14
 Release:    0
 Group:      System/Libraries
 License:    Apache-2.0