Ignore client's duplicate release request after invoking release callback 67/233167/19
authorYoungHun Kim <yh8004.kim@samsung.com>
Tue, 12 May 2020 23:43:10 +0000 (08:43 +0900)
committerYoungHun Kim <yh8004.kim@samsung.com>
Fri, 22 May 2020 01:52:08 +0000 (10:52 +0900)
Change-Id: I94e6f588bba579b50a02a3ff4f460e6498928a09

src/lib/mm_resource_manager_priv.c

index 8b16e01..be0dfe7 100644 (file)
@@ -44,6 +44,7 @@ typedef struct {
 } mm_resource_manager_cb_s;
 
 typedef struct {
+       mm_resource_manager_id id;
        mm_resource_manager_res_type_e type;
        mm_resource_manager_res_volume volume;
        mm_resource_manager_res_state_e state;
@@ -87,7 +88,7 @@ static void __destroy_resource(void *res);
 static int __get_resource_index(mm_resource_manager_s *rm,
                mm_resource_manager_res_p res);
 static gboolean __mark_resource_for_release(GPtrArray *resources, int index,
-               mm_resource_manager_res_p resource);  /* FALSE if resource is destroyed */
+               mm_resource_manager_res_p resource); /* FALSE if resource is destroyed */
 static void __send_release_cb_sync(mm_resource_manager_id id);
 static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle,
                mm_resource_manager_id id,
@@ -330,25 +331,52 @@ int _mm_resource_manager_resize_marked(mm_resource_manager_h rm,
 int _mm_resource_manager_mark_for_release(mm_resource_manager_h rm,
                mm_resource_manager_res_h resource_h)
 {
-       mm_resource_manager_s *handle = MM_RESOURCE_MANAGER(rm);
-       mm_resource_manager_res_p resource = (mm_resource_manager_res_p) resource_h;
-       int i;
+               mm_resource_manager_s *handle;
+       mm_resource_manager_res_p resource;
+       mm_resource_manager_res_p res_p;
+       mm_resource_manager_id handle_id;
+       mm_resource_manager_id resource_id;
+       int idx = 0;
 
        __mm_resource_handles_lock();
+
+       handle = MM_RESOURCE_MANAGER(rm);
        MM_RESOURCE_MANAGER_CHECK(handle);
+       handle_id = handle->id;
+       MM_RM_HASH64(handle_id);
+
+       resource = (mm_resource_manager_res_p)resource_h;
+       resource_id = resource->id;
+       MM_RM_HASH64(resource_id);
+
+       MM_RM_INFO("Resource %p (%s) is marked for release in RM #%"PRIu64,
+               resource_h, _mm_resource_manager_get_res_str(resource->type), handle_id);
+
+       MM_RM_RETVM_IF(handle_id != resource_id, MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER,
+                       "handle RM #%"PRIu64" is not resource RM #%"PRIu64, handle_id, resource_id);
+
+       for (idx = 0; idx < handle->resources->len; idx++) {
+               res_p = (mm_resource_manager_res_p)handle->resources->pdata[idx];
+               if (res_p->state == MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE) {
+                       MM_RM_DEBUG("Resource %p (%s) release is executing in RM #%"PRIu64,
+                               res_p, _mm_resource_manager_get_res_str(res_p->type), _mm_rm_hash64(res_p->id));
+                       __mm_resource_handles_unlock();
+                       return MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION;
+               }
+       }
+
        __mm_resources_lock(handle);
        __mm_resource_handles_unlock();
 
-       i = __get_resource_index(handle, resource);
-       MM_RM_UNLOCK_RETVM_IF(i == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
-                       handle->resources_lock,
-                       MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER,
+       idx = __get_resource_index(handle, resource);
+       MM_RM_UNLOCK_RETVM_IF(idx == MM_RESOURCE_MANAGER_RES_NOT_FOUND,
+                       handle->resources_lock, MM_RESOURCE_MANAGER_ERROR_INVALID_PARAMETER,
                        "Invalid resource handle");
 
-       __mark_resource_for_release(handle->resources, i, resource);
+       MM_RM_UNLOCK_RETVM_IF(!__mark_resource_for_release(handle->resources, idx, resource),
+                       handle->resources_lock, MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION,
+                       "Not in a release state");
 
-       MM_RM_INFO("Resource %p is marked for release in resource manager #%"PRIu64,
-                       resource_h, _mm_rm_hash64(handle->id));
        __mm_resources_unlock(handle);
 
        return MM_RESOURCE_MANAGER_ERROR_NONE;
@@ -369,8 +397,7 @@ int _mm_resource_manager_mark_all_for_release(mm_resource_manager_h rm)
                        i--;
        }
 
-       MM_RM_INFO("All resources are marked for release in resource manager #%"PRIu64,
-                       _mm_rm_hash64(handle->id));
+       MM_RM_INFO("All resources are marked for release in resource manager #%"PRIu64, _mm_rm_hash64(handle->id));
        __mm_resources_unlock(handle);
 
        return MM_RESOURCE_MANAGER_ERROR_NONE;
@@ -500,7 +527,7 @@ static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle
        mm_resource_manager_res_s *resource;
        mm_resource_manager_id handle_id;
        gboolean release_all = FALSE;
-       int i;
+       int idx;
 
        MM_RM_INFO("Release callback is emitted for %s of volume %d in RM #%"PRIu64,
                        _mm_resource_manager_get_res_str(type), volume, id);
@@ -511,16 +538,24 @@ static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle
                __mm_resources_lock(handle);
                __mm_resource_handles_unlock();
 
-               for (i = 0; i < handle->resources->len; i++) {
-                       resource = (mm_resource_manager_res_s*)handle->resources->pdata[i];
+               for (idx = 0; idx < handle->resources->len; idx++) {
+                       resource = (mm_resource_manager_res_s*)handle->resources->pdata[idx];
                        if (resource->type == type && resource->volume == volume) {
-
-                               release_all = ((mm_resource_manager_release_cb)
-                                               handle->release_cb.cb)(handle, resource, handle->release_cb.user_data);
+                               if (resource->state == MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE) {
+                                       MM_RM_DEBUG("Resource %p (%s) is already marked to release RM #%"PRIu64,
+                                               resource, _mm_resource_manager_get_res_str(resource->type), _mm_rm_hash64(resource->id));
+                               }
+
+                               resource->state = MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE;
+                               MM_RM_INFO("[res %p type %s volume %d] release_cb",
+                                       resource, _mm_resource_manager_get_res_str(type), volume);
+                               release_all = ((mm_resource_manager_release_cb)handle->release_cb.cb)
+                                               (handle, resource, handle->release_cb.user_data);
+                               MM_RM_INFO("[%d] release_cb is completed", release_all);
 
                                __send_release_cb_sync(handle->id);
 
-                               g_ptr_array_remove_index_fast(handle->resources, i);
+                               g_ptr_array_remove_index_fast(handle->resources, idx);
                                break;
                        }
                }
@@ -615,8 +650,13 @@ static int __create_resource(mm_resource_manager_s *rm,
                return ret;
 
        *res = g_new0(mm_resource_manager_res_s, 1);
+       (*res)->id = handle->id;
        (*res)->type = type;
-       (*res)->volume = volume;
+       if (handle->__max_resource_volumes[type] == MM_RESOURCE_MANAGER_RES_VOLUME_FULL &&
+               volume != MM_RESOURCE_MANAGER_RES_VOLUME_FULL)
+               (*res)->volume = MM_RESOURCE_MANAGER_RES_VOLUME_FULL;
+       else
+               (*res)->volume = volume;
 
        return MM_RESOURCE_MANAGER_ERROR_NONE;
 }
@@ -657,7 +697,7 @@ static gboolean __mark_resource_for_release(GPtrArray *resources, int index,
                break;
        case MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE:
                MM_RM_DEBUG("Resource %p is already marked", resource);
-               break;
+               return FALSE;
        }
 
        return TRUE;
@@ -679,7 +719,7 @@ static void __send_release_cb_sync(mm_resource_manager_id id)
        int sync_fd;
 
        sync_fd = open(RELEASE_CB_SYNC_PATH, O_WRONLY);
-       MM_RM_RETM_IF(sync_fd == -1, "Sync FIFO cannot be opened");
+       MM_RM_RETM_IF(sync_fd == -1, "Sync FIFO cannot be opened [errno %d]", errno);
 
        if (write(sync_fd, &id, sizeof(id)) == sizeof(id))
                MM_RM_DEBUG("Sync message is sent successfully");
@@ -725,7 +765,7 @@ static int __dbus_init(mm_resource_manager_s *handle)
        MM_RM_RET_IF_GERR(error, "Dbus proxy cannot be created");
 
        if (g_signal_connect(handle->dbus_proxy, "release_callback", (GCallback)__dbus_release_callback, NULL) < 1 ||
-                       g_signal_connect(handle->dbus_proxy, "status_callback", (GCallback)__dbus_status_callback, NULL) < 1) {
+               g_signal_connect(handle->dbus_proxy, "status_callback", (GCallback)__dbus_status_callback, NULL) < 1) {
 
                g_object_unref(handle->dbus_proxy);
                handle->dbus_proxy = NULL;