Add fix to prevent crash 84/162284/7 accepted/tizen/unified/20171211.061312 submit/tizen/20171208.102635
authoraravind.gara <aravind.gara@samsung.com>
Thu, 30 Nov 2017 06:12:30 +0000 (15:12 +0900)
committeraravind.gara <aravind.gara@samsung.com>
Fri, 8 Dec 2017 10:05:04 +0000 (19:05 +0900)
Added signalling mechanism to avoid double free possibility.
Stream destory function is called only isolator callback thread.

[Version] 0.0.13
[Issue Type] Bug fix and Enhancement

Change-Id: I3be71ff453765817f5223e0e0a99abfba75a0397
Signed-off-by: aravind.gara <aravind.gara@samsung.com>
include/internal/soundpool.h
include/internal/stream_cb_manager.h
packaging/capi-media-sound-pool.spec
src/soundpool.c
src/source.c
src/stream_cb_manager.c

index c8b16ed37a6ab731693a0f61856442ba62923a56..b1de33756040b3d89f6b7ae2dc391cb1a7a6b53b 100644 (file)
@@ -66,6 +66,8 @@ sound_pool_error_e _sound_pool_activate(sound_pool_t *pool);
 
 sound_pool_error_e _sound_pool_deactivate(sound_pool_t *pool);
 
+sound_pool_error_e _sound_pool_stop_streams(sound_pool_t *pool);
+
 sound_pool_error_e _sound_pool_get_state(sound_pool_t *pool, sound_pool_state_e *state);
 
 sound_pool_error_e _sound_pool_set_volume(sound_pool_t *pool, float volume);
index 60ef83a0bb08887c6b16b22c285f79cdf4fe74f1..af51d86caac3a6ebf618e6170f372ee6bd6c886f 100644 (file)
@@ -40,8 +40,10 @@ struct stream_cb_manager_s {
        pthread_t isolator_thread;
        pthread_mutex_t isolator_data_mutex;
        pthread_cond_t isolator_data_cond;
+       pthread_cond_t isolator_force_run_cond;
        gboolean isolator_state_changed;
        gboolean isolator_loop_run;
+       gboolean isolator_force_run;
 };
 
 typedef struct stream_cb_manager_event_data_s {
@@ -57,6 +59,10 @@ sound_pool_error_e _stream_cb_manager_destroy(stream_cb_manager_t *cbmgr);
 sound_pool_error_e _stream_cb_manager_register_event(stream_cb_manager_t *cbmgr,
                sound_stream_t *stream);
 
+sound_pool_error_e _stream_cb_manager_process_pending_events(stream_cb_manager_t *cbmgr);
+
+sound_pool_error_e _stream_cb_manager_signal_completed_events(stream_cb_manager_t *cbmgr);
+
 #ifdef __cplusplus
 }
 #endif /* __cplusplus */
index 73252a1d3bd611805f5d58e337e899dea5d36da6..2a038c4603c4f717b5aed70d519fc1f54cf1f023 100644 (file)
@@ -1,5 +1,5 @@
 Name:       capi-media-sound-pool
-Version:    0.0.12
+Version:    0.0.13
 Summary:    Tizen Sound Pool module
 Release:    0
 Group:      Multimedia/Framework
index 5034331176c7d810775cab961bfc2206ef47ae95..2eaaa5793909aa90b8909265d11e901d507138ac 100644 (file)
@@ -75,7 +75,24 @@ static void __stream_deactivate_iter(gpointer key, gpointer value,
 
        SP_DEBUG_FLEAVE();
 }
+static void __stream_stop_iter(gpointer key, gpointer value,
+               gpointer user_data)
+{
+       SP_DEBUG_FENTER();
+       SP_RETM_IF(!user_data, "Empty user data.");
+       SP_RETM_IF(!value, "Empty sound stream data.");
+       guint *len = (guint*)user_data;
+       sound_stream_t *stream = (sound_stream_t *)value;
 
+       if ((stream->state == SOUND_POOL_STREAM_STATE_PLAYING)
+               || (stream->state == SOUND_POOL_STREAM_STATE_SUSPENDED)
+               || (stream->state == SOUND_POOL_STREAM_STATE_PAUSED)) {
+               alSourceStop(stream->al_source);
+               (*len)++;
+       }
+
+       SP_DEBUG_FLEAVE();
+}
 sound_pool_error_e _sound_pool_create(sound_pool_t **pool)
 {
        SP_DEBUG_FENTER();
@@ -144,14 +161,7 @@ sound_pool_error_e _sound_pool_destroy(sound_pool_t *pool)
 {
        SP_DEBUG_FENTER();
        SP_INST_CHECK(pool, SOUND_POOL_ERROR_INVALID_PARAMETER);
-       _sound_pool_deactivate(pool);
-
-       if (pool->cbmgr) {
-               sound_pool_error_e ret = _stream_cb_manager_destroy(pool->cbmgr);
-               SP_RETVM_IF(ret != SOUND_POOL_ERROR_NONE, ret,
-                               "Error occurred when " "trying to destroy sound pool callback manager.");
-               pool->cbmgr = NULL;
-       }
+       _sound_pool_stop_streams(pool);
 
        if (pool->sources) {
                GHashTableIter iter;
@@ -168,6 +178,13 @@ sound_pool_error_e _sound_pool_destroy(sound_pool_t *pool)
                pool->sources = NULL;
        }
 
+       if (pool->cbmgr) {
+               sound_pool_error_e ret = _stream_cb_manager_destroy(pool->cbmgr);
+               SP_RETVM_IF(ret != SOUND_POOL_ERROR_NONE, ret,
+                               "Error occurred when " "trying to destroy sound pool callback manager.");
+               pool->cbmgr = NULL;
+       }
+
        if (pool->streams) {
                g_hash_table_unref(pool->streams);
                pool->streams = NULL;
@@ -235,6 +252,10 @@ sound_pool_error_e _sound_pool_deactivate(sound_pool_t *pool)
                        "Already deactivated sound pool can't be deactivated again.");
 
        sound_pool_state_e old_state = pool->state;
+
+       /* Wait for completing the isoloator callback thread events */
+       _stream_cb_manager_process_pending_events(pool->cbmgr);
+
        pool->state = SOUND_POOL_STATE_INACTIVE;
 
        if (g_hash_table_size(pool->streams) > 0) {
@@ -255,6 +276,26 @@ sound_pool_error_e _sound_pool_deactivate(sound_pool_t *pool)
        return SOUND_POOL_ERROR_NONE;
 }
 
+sound_pool_error_e _sound_pool_stop_streams(sound_pool_t *pool)
+{
+       SP_DEBUG_FENTER();
+       SP_INST_CHECK(pool, SOUND_POOL_ERROR_INVALID_PARAMETER);
+
+       pool->state = SOUND_POOL_STATE_INACTIVE;
+
+       if (g_hash_table_size(pool->streams) > 0) {
+               SP_RETVM_IF(!alcMakeContextCurrent(pool->al_context),
+                               SOUND_POOL_ERROR_INVALID_OPERATION, "Can't set current context.");
+               guint len = 0;
+               g_hash_table_foreach(pool->streams, __stream_stop_iter, &len);
+               SP_INFO("Stopping [%d] number of streams.", len);
+       }
+
+       SP_INFO("Sound pool has been stopped");
+
+       SP_DEBUG_FLEAVE();
+       return SOUND_POOL_ERROR_NONE;
+}
 sound_pool_error_e _sound_pool_get_state(sound_pool_t *pool,
                sound_pool_state_e *state)
 {
index c35d8af8d33f9f464ee4c1f2953d49fa533c65be..1618eb7a1aee0e3fd7e15436b7565ae36197560d 100644 (file)
@@ -22,6 +22,7 @@
 
 #include "internal/source.h"
 #include "internal/stream.h"
+#include "internal/stream_cb_manager.h"
 #include <unistd.h>
 #ifdef ENABLE_ALURE
 #include <AL/alure.h>
@@ -81,7 +82,11 @@ static sound_pool_error_e __sound_pool_remove_source(sound_pool_t *pool, sound_s
                                if (stream->state == SOUND_POOL_STREAM_STATE_STOPPED || stream->state == SOUND_POOL_STREAM_STATE_FINISHED) {
                                        SP_DEBUG("Callback isolator thread destroying the stream");
                                } else {
-                                       _sound_stream_destroy((sound_stream_t*)value);
+                                       if (stream->state == SOUND_POOL_STREAM_STATE_SUSPENDED
+                                               || stream->state == SOUND_POOL_STREAM_STATE_PLAYING
+                                               || stream->state == SOUND_POOL_STREAM_STATE_PAUSED)
+                                       SP_DEBUG("Callback isolator thread destroys the stream");
+                                       _sound_stream_stop(stream);
                                }
                        }
                        guint size_after = g_hash_table_size(pool->streams);
@@ -183,6 +188,10 @@ sound_pool_error_e _sound_source_destroy(sound_source_t *src)
        SP_RETVM_IF(!src, SOUND_POOL_ERROR_INVALID_PARAMETER,
                        "Can't destroy NULL sound source");
 
+       SP_INST_CHECK(src->parent_pool, SOUND_POOL_ERROR_INVALID_PARAMETER);
+       SP_INST_CHECK(src->parent_pool->cbmgr, SOUND_POOL_ERROR_INVALID_PARAMETER);
+       stream_cb_manager_t *cbmgr = (stream_cb_manager_t *)(src->parent_pool->cbmgr);
+
        /* If parent pool exists, then source has to be removed from the pool */
        if (src->parent_pool && __sound_pool_remove_source(src->parent_pool, src)
                        != SOUND_POOL_ERROR_NONE)
@@ -191,6 +200,9 @@ sound_pool_error_e _sound_source_destroy(sound_source_t *src)
        SP_DEBUG("Deleting OpenAL buffer with id [%u]", src->al_buffer);
        alDeleteBuffers(1, &src->al_buffer);
 
+       /* Wait for completing the isolator callback thread events */
+       _stream_cb_manager_process_pending_events(cbmgr);
+
        SP_SAFE_GFREE(src->tag_name);
        SP_SAFE_GFREE(src);
 
index 9dfc8a52d87c45d19778caba9fa61934ce850e82..53c53caf96ef9de3cb0a9d06904abf769d79e8d1 100644 (file)
@@ -49,6 +49,43 @@ static void __thread_cancel_cleanup(void *user_data)
        SP_DEBUG_FLEAVE();
 }
 
+sound_pool_error_e _stream_cb_manager_process_pending_events(stream_cb_manager_t *cbmgr)
+{
+       SP_DEBUG_FENTER();
+       SP_RETVM_IF(!cbmgr, SOUND_POOL_ERROR_INVALID_PARAMETER, "Can't handle callback "
+                       "manager, it is NULL.");
+
+       pthread_mutex_lock(&cbmgr->isolator_data_mutex);
+       if (cbmgr->isolator_state_changed) {
+               cbmgr->isolator_force_run = TRUE;
+               SP_INFO("waiting for isolator callback thread completion");
+               pthread_cond_wait(&cbmgr->isolator_force_run_cond, &cbmgr->isolator_data_mutex);
+       }
+       pthread_mutex_unlock(&cbmgr->isolator_data_mutex);
+
+       SP_DEBUG_FLEAVE();
+       return SOUND_POOL_ERROR_NONE;
+}
+
+sound_pool_error_e _stream_cb_manager_signal_completed_events(stream_cb_manager_t *cbmgr)
+{
+       SP_DEBUG_FENTER();
+       SP_RETVM_IF(!cbmgr, SOUND_POOL_ERROR_INVALID_PARAMETER, "Can't handle callback "
+                       "manager, it is NULL.");
+
+       pthread_mutex_lock(&cbmgr->isolator_data_mutex);
+       cbmgr->isolator_state_changed = FALSE;
+       if (cbmgr->isolator_force_run) {
+               SP_INFO("Signal indicating isolator callback completed the task");
+               pthread_cond_signal(&cbmgr->isolator_force_run_cond);
+       }
+       cbmgr->isolator_force_run = FALSE;
+       pthread_mutex_unlock(&cbmgr->isolator_data_mutex);
+
+       SP_DEBUG_FLEAVE();
+       return SOUND_POOL_ERROR_NONE;
+}
+
 static gpointer __sound_pool_callback_isolator(gpointer user_data)
 {
        SP_DEBUG_FENTER();
@@ -91,10 +128,8 @@ static gpointer __sound_pool_callback_isolator(gpointer user_data)
                                _sound_stream_destroy(event_data->stream);
                        SP_SAFE_GFREE(event_data);
                }
-
-               pthread_mutex_lock(&cbmgr->isolator_data_mutex);
-               cbmgr->isolator_state_changed = FALSE;
-               pthread_mutex_unlock(&cbmgr->isolator_data_mutex);
+               /* Signal indicating isolator callback thread completed the events */
+               _stream_cb_manager_signal_completed_events(cbmgr);
 
                pthread_mutex_lock(&cbmgr->isolator_data_mutex);
                runing = cbmgr->isolator_loop_run;
@@ -135,6 +170,7 @@ sound_pool_error_e _stream_cb_manager_create(sound_pool_t *pool,
 
        _cbmgr->isolator_state_changed = FALSE;
        _cbmgr->isolator_loop_run = TRUE;
+       _cbmgr->isolator_force_run = FALSE;
 
        err = pthread_cond_init(&_cbmgr->isolator_data_cond, NULL);
 
@@ -143,7 +179,13 @@ sound_pool_error_e _stream_cb_manager_create(sound_pool_t *pool,
                ret = SOUND_POOL_ERROR_OUT_OF_MEMORY;
                GOTO_FAIL("", creturn);
        }
+       err = pthread_cond_init(&_cbmgr->isolator_force_run_cond, NULL);
 
+       if (0 != err) {
+               SP_ERROR("Error while initialize condition for isolation thread with error[%d].", err);
+               ret = SOUND_POOL_ERROR_OUT_OF_MEMORY;
+               GOTO_FAIL("", creturn);
+       }
        err = pthread_create(&_cbmgr->isolator_thread, NULL, &__sound_pool_callback_isolator, (void*)_cbmgr);
 
        if (0 != err) {
@@ -184,6 +226,8 @@ sound_pool_error_e _stream_cb_manager_destroy(stream_cb_manager_t *cbmgr)
 
        pthread_t thread = cbmgr->isolator_thread;
 
+       /* Wait for completing the isolator callback thread events */
+       _stream_cb_manager_process_pending_events(cbmgr);
        err = pthread_kill(thread, 0);
        if (0 == err) {
                err = pthread_cancel(thread);