fixup! Resolve the handles's mutex deadlock

Change-Id: I2101b3baadb8c6fa4a5c27c05e12da459d4e541d

diff --git a/packaging/mm-resource-manager.spec b/packaging/mm-resource-manager.spec
index 42dcff65883f58df82623ab5e333901b22cb4b5d..482305c874da8565c83d71d5de4d7093110f313c 100644 (file)
--- a/packaging/mm-resource-manager.spec
+++ b/packaging/mm-resource-manager.spec
@@ -1,6 +1,6 @@
 Name:       mm-resource-manager
 Summary:    A Multimedia Resource Manager API
-Version:    0.2.14
+Version:    0.2.15
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0

diff --git a/src/common/mm_resource_manager_utils.h b/src/common/mm_resource_manager_utils.h
index b7d5d07ab90e77b18165216e9938338e06d60226..05507672b26b64f0e617fa16146730c779d2b16a 100644 (file)
--- a/src/common/mm_resource_manager_utils.h
+++ b/src/common/mm_resource_manager_utils.h
@@ -85,16 +85,6 @@
                }                                                           \
        } while (0)
 
-#define MM_RM_UNLOCK2_RETVM_IF(expr, mtx, mtx2, val, fmt, arg...)          \
-       do {                                                            \
-               if (expr) {                                                 \
-                       LOGE(FONT_COLOR_RED""fmt""FONT_COLOR_RESET, ##arg);     \
-                       g_mutex_unlock(&mtx);                                   \
-                       g_mutex_unlock(&mtx2);                                   \
-                       return(val);                                            \
-               }                                                           \
-       } while (0)
-
 #define MM_RM_RETM_IF_E(expr, fmt, arg...)                          \
        do {                                                            \
                int err = expr;                                             \

diff --git a/src/lib/mm_resource_manager_priv.c b/src/lib/mm_resource_manager_priv.c
index 5390dbb6aaa3035b6e91ed171cdeeb9e4a028a73..524b57df0d3cc3f081763302a9238c5e94b7dcf8 100644 (file)
--- a/src/lib/mm_resource_manager_priv.c
+++ b/src/lib/mm_resource_manager_priv.c
@@ -251,10 +251,11 @@ int _mm_resource_manager_mark_for_acquire(
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        ret = __create_resource(handle, type, volume, &resource);
-       MM_RM_UNLOCK2_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE,
-                       handle->resources_lock, handles_lock, ret, "Resource cannot be created");
+       MM_RM_UNLOCK_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE,
+                       handle->resources_lock, ret, "Resource cannot be created");
        g_ptr_array_add(handle->resources, resource);
 
        *resource_h = resource;
@@ -263,7 +264,6 @@ int _mm_resource_manager_mark_for_acquire(
                        "resource manager #%"PRIu64, *resource_h, type, volume,
                        _mm_rm_hash64(handle->id));
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        return ret;
 }
@@ -282,10 +282,11 @@ int _mm_resource_manager_resize_marked(mm_resource_manager_h rm,
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        i = __get_resource_index(handle, resource);
-       MM_RM_UNLOCK2_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
-                       handle->resources_lock, handles_lock,
+       MM_RM_UNLOCK_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
+                       handle->resources_lock,
                        MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER,
                        "Invalid resource handle");
        if (new_volume == resource->volume) {
@@ -302,8 +303,8 @@ int _mm_resource_manager_resize_marked(mm_resource_manager_h rm,
        case MM_RESOURCE_MANAGER_RES_STATE_FOR_ACQUIRE:
                if (add_volume > 0) {
                        ret = __check_resource(handle, resource->type, add_volume);
-                       MM_RM_UNLOCK2_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE,
-                               handle->resources_lock, handles_lock, ret, "Resource check failed");
+                       MM_RM_UNLOCK_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE,
+                               handle->resources_lock, ret, "Resource check failed");
                }
                resource->volume = new_volume;
                break;
@@ -311,8 +312,8 @@ int _mm_resource_manager_resize_marked(mm_resource_manager_h rm,
        case MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE:
        case MM_RESOURCE_MANAGER_RES_STATE_ACQUIRED:
                ret = __create_resource(handle, resource->type, add_volume, &tmp_resource);
-               MM_RM_UNLOCK2_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE,
-                               handle->resources_lock, handles_lock, ret, "Resource cannot be created");
+               MM_RM_UNLOCK_RETVM_IF(ret != MM_RESOURCE_MANAGER_ERROR_NONE,
+                               handle->resources_lock, ret, "Resource cannot be created");
 
                tmp_resource->volume = resource->volume;
                tmp_resource->state = MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE;
@@ -328,7 +329,6 @@ int _mm_resource_manager_resize_marked(mm_resource_manager_h rm,
        MM_RM_INFO("Resource %p is resized for acquire in resource manager #%"PRIu64,
                        resource_h, _mm_rm_hash64(handle->id));
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        return ret;
 }
@@ -343,10 +343,11 @@ int _mm_resource_manager_mark_for_release(mm_resource_manager_h rm,
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        i = __get_resource_index(handle, resource);
-       MM_RM_UNLOCK2_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
-                       handle->resources_lock, handles_lock,
+       MM_RM_UNLOCK_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
+                       handle->resources_lock,
                        MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER,
                        "Invalid resource handle");
 
@@ -355,7 +356,6 @@ int _mm_resource_manager_mark_for_release(mm_resource_manager_h rm,
        MM_RM_INFO("Resource %p is marked for release in resource manager #%"PRIu64,
                        resource_h, _mm_rm_hash64(handle->id));
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        return MM_RESOURCE_MANAGER_ERROR_NONE;
 }
@@ -368,6 +368,7 @@ int _mm_resource_manager_mark_all_for_release(mm_resource_manager_h rm)
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        for (i = 0; i < handle->resources->len; i++) {
                if (!__mark_resource_for_release(handle->resources, i,
@@ -378,7 +379,6 @@ int _mm_resource_manager_mark_all_for_release(mm_resource_manager_h rm)
        MM_RM_INFO("All resources are marked for release in resource manager #%"PRIu64,
                        _mm_rm_hash64(handle->id));
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        return MM_RESOURCE_MANAGER_ERROR_NONE;
 }
@@ -394,10 +394,11 @@ int _mm_resource_manager_get_resource_info(mm_resource_manager_h rm,
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        i = __get_resource_index(handle, resource);
-       MM_RM_UNLOCK2_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
-                       handle->resources_lock, handles_lock,
+       MM_RM_UNLOCK_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
+                       handle->resources_lock,
                        MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER,
                        "Invalid resource handle");
 
@@ -408,7 +409,6 @@ int _mm_resource_manager_get_resource_info(mm_resource_manager_h rm,
        MM_RM_INFO("Info structure of resource %p in resource manager #%"PRIu64" is filled",
                        resource_h, _mm_rm_hash64(handle->id));
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        return MM_RESOURCE_MANAGER_ERROR_NONE;
 }
@@ -421,6 +421,7 @@ int _mm_resource_manager_commit(mm_resource_manager_h rm)
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        ret = __dbus_commit(handle);
        if (ret == MM_RESOURCE_MANAGER_ERROR_NONE)
@@ -429,7 +430,6 @@ int _mm_resource_manager_commit(mm_resource_manager_h rm)
        else
                MM_RM_ERROR("Dbus commit request failed");
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        return ret;
 }
@@ -442,11 +442,11 @@ int _mm_resource_manager_set_status_cb(mm_resource_manager_h rm,
        g_mutex_lock(&handles_lock);
        MM_RESOURCE_MANAGER_CHECK(handle);
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
 
        handle->status_cb.cb = cb;
        handle->status_cb.user_data = user_data;
        g_mutex_unlock(&handle->resources_lock);
-       g_mutex_unlock(&handles_lock);
 
        MM_RM_INFO("Status callback %p in resource manager #%"PRIu64" is set", cb,
                        _mm_rm_hash64(handle->id));
@@ -502,6 +502,7 @@ void __mm_resource_manager_release_callback(mm_resource_manager_s *handle,
        MM_RM_HASH64(handle_id);
        if (handle_id == id) {
                g_mutex_lock(&handle->resources_lock);
+               g_mutex_unlock(&handles_lock);
                for (j = 0; j < handle->resources->len; j++) {
                        resource = (mm_resource_manager_res_s*)handle->resources->pdata[j];
                        if (resource->type == type && resource->volume == volume) {
@@ -519,15 +520,15 @@ void __mm_resource_manager_release_callback(mm_resource_manager_s *handle,
                g_mutex_unlock(&handle->resources_lock);
 
                if (release_all) {
-                       if (_mm_resource_manager_mark_all_for_release(handle) ==
-                                       MM_RESOURCE_MANAGER_ERROR_NONE &&
-                                       _mm_resource_manager_commit(handle) ==
-                               MM_RESOURCE_MANAGER_ERROR_NONE) {
+                       if (_mm_resource_manager_mark_all_for_release(handle) == MM_RESOURCE_MANAGER_ERROR_NONE
+                               && _mm_resource_manager_commit(handle) == MM_RESOURCE_MANAGER_ERROR_NONE) {
                                MM_RM_DEBUG("All resources are released after release cb");
                        } else {
                                MM_RM_ERROR("Resources cannot be released after release cb");
                        }
                }
+       } else {
+               g_mutex_unlock(&handles_lock);
        }
 }
 
@@ -535,6 +536,7 @@ void __mm_resource_manager_status_callback(mm_resource_manager_s *handle,
                mm_resource_manager_status_e status)
 {
        g_mutex_lock(&handle->resources_lock);
+       g_mutex_unlock(&handles_lock);
        if (handle->status_cb.cb) {
                ((mm_resource_manager_status_cb)handle->status_cb.cb)(handle, status,
                                handle->status_cb.user_data);
@@ -907,6 +909,7 @@ static void __dbus_release_callback(MMResourceManager *object, guint64 arg_id,
                gint arg_resource_type, gint arg_volume)
 {
        mm_resource_manager_s *handle;
+       gboolean unlock = TRUE;
        int i;
 
        g_mutex_lock(&handles_lock);
@@ -915,15 +918,19 @@ static void __dbus_release_callback(MMResourceManager *object, guint64 arg_id,
                if (handle->dbus_proxy == object) {
                        __mm_resource_manager_release_callback(handle, arg_id,
                                        arg_resource_type, arg_volume);
+                       unlock = FALSE;
                        break;
                }
        }
-       g_mutex_unlock(&handles_lock);
+
+       if (unlock)
+               g_mutex_unlock(&handles_lock);
 }
 
 static void __dbus_status_callback(MMResourceManager *object, gint arg_status)
 {
        mm_resource_manager_s *handle;
+       gboolean unlock = TRUE;
        int i;
 
        g_mutex_lock(&handles_lock);
@@ -931,10 +938,13 @@ static void __dbus_status_callback(MMResourceManager *object, gint arg_status)
                handle = (mm_resource_manager_s*)handles->pdata[i];
                if (handle->dbus_proxy == object) {
                        __mm_resource_manager_status_callback(handle, arg_status);
+                       unlock = FALSE;
                        break;
                }
        }
-       g_mutex_unlock(&handles_lock);
+
+       if (unlock)
+               g_mutex_unlock(&handles_lock);
 }
 
 static gpointer __dispatcher_thread(gpointer user_data)