Fix the issue of release_cb_invoked 65/252365/36 accepted/tizen/unified/20210223.065512 submit/tizen/20210221.233041
authorYoungHun Kim <yh8004.kim@samsung.com>
Wed, 27 Jan 2021 06:29:00 +0000 (15:29 +0900)
committerYoungHun Kim <yh8004.kim@samsung.com>
Fri, 19 Feb 2021 01:22:17 +0000 (10:22 +0900)
 - By considering mark_for_release() as complete release, we can
   ignore the release_cb for the resource
 - Similarly, we regard RES_STATE_FOR_RELEASE as including until
   the complete release through release_commit()
 - Fix the exceptional fd failure issue
 - Avoid to emit release callback of resource manager's other resource,
   which can be changed by release policy

Change-Id: I5f14fd4528fd57d879a0ea7d69cf8ad703806130

packaging/mm-resource-manager.spec
src/daemon/mm_resource_manager_daemon_conf.c
src/daemon/mm_resource_manager_daemon_conf.h
src/daemon/mm_resource_manager_daemon_priv.c
src/lib/mm_resource_manager_priv.c

index 906a0df..645f800 100644 (file)
@@ -1,6 +1,6 @@
 Name:       mm-resource-manager
 Summary:    A Multimedia Resource Manager API
-Version:    0.2.44
+Version:    0.2.45
 Release:    0
 Group:      Multimedia/API
 License:    Apache-2.0
index d834778..d3f7576 100644 (file)
@@ -26,6 +26,7 @@
 #define MM_RESOURCE_MANAGER_INI_MAX_VOLUME "max volume:"
 #define MM_RESOURCE_MANAGER_INI_PRIORITY "priority:"
 #define MM_RESOURCE_MANAGER_INI_MAX_INSTANCE "max instance:"
+#define MM_RESOURCE_MANAGER_INI_RELEASE_ALL "release:all"
 
 
 
@@ -77,6 +78,8 @@ gboolean mm_resource_manager_reload_conf(void)
                        MM_RESOURCE_MANAGER_INI_MAX_INSTANCE), _mm_resource_manager_get_res_str(i))->str, MM_RESOURCE_MANAGER_NO_RES);
        }
 
+       mm_resource_manager_conf.is_release_all = iniparser_getboolean(ini, MM_RESOURCE_MANAGER_INI_RELEASE_ALL, TRUE);
+
        g_string_free(param_name, TRUE);
        iniparser_freedict(ini);
 
index 7534665..d1bf63a 100644 (file)
@@ -32,6 +32,7 @@ typedef struct {
        mm_resource_manager_condition_volume_a condition_volume;
        int max_instance[MM_RESOURCE_MANAGER_RES_TYPE_MAX];
        gboolean volume_would_be_checked[MM_RESOURCE_MANAGER_RES_TYPE_MAX];
+       gboolean is_release_all;
 } mm_resource_manager_conf_s;
 
 gboolean mm_resource_manager_reload_conf(void);
index 139b784..7533179 100644 (file)
@@ -56,6 +56,7 @@ typedef struct {
        mm_resource_manager_app_class_e app_class;
        mm_resource_manager_res_type_e type;
        int volume;
+       gboolean is_dbus_release_emitted;
        /* if an element is NULL, there is no such a resource for the current platform. */
        mm_resource_manager_dmn_res_p resources[MM_RESOURCE_MANAGER_RES_TYPE_MAX];
 
@@ -92,11 +93,13 @@ static void __sync_increase_acquire_requests(mm_resource_manager_dmn_res_request
 static void __handle_release_requests(mm_resource_manager_dmn_p manager, mm_resource_manager_dmn_res_request_s *requests);
 static GArray* __handle_acquire_requests(mm_resource_manager_dmn_p manager, mm_resource_manager_dmn_res_request_s *requests);
 static void __handle_release_callbacks(GArray *requests);
+static int __open_release_cb_sync_fd(void);
+static void __close_release_cb_sync_fd(int fd);
 static inline void __add_cb_request(GArray *cb_requests, mm_resource_manager_dmn_p mgr,
                mm_resource_manager_res_type_e type, mm_resource_manager_res_volume volume);
 static void __destroy_all_resources(mm_resource_manager_dmn_p manager);
 static gboolean __poll(struct pollfd sync, mm_resource_manager_id id);
-static gboolean __wait_for_release_cb_sync(mm_resource_manager_id id);
+static gboolean __wait_for_release_cb_sync(mm_resource_manager_id id, int sync_fd);
 
 
 
@@ -630,9 +633,28 @@ static GArray *__handle_acquire_requests(mm_resource_manager_dmn_p manager,
        return cb_requests;
 }
 
+static int __open_release_cb_sync_fd(void)
+{
+       int fd = open(RELEASE_CB_SYNC_PATH, O_RDONLY | O_NONBLOCK);
+       MM_RM_RETVM_IF(fd == -1, fd, "[%s] open failed (%d)", RELEASE_CB_SYNC_PATH, errno);
+
+       MM_RM_DEBUG("[%d] opened %s", fd, RELEASE_CB_SYNC_PATH);
+
+       return fd;
+}
+
+static void __close_release_cb_sync_fd(int fd)
+{
+       MM_RM_RETM_IF(fd == -1, "[fd %d] invalid parameter", fd);
+       MM_RM_RETM_IF(close(fd) == -1, "[%s] close failed (%d)", RELEASE_CB_SYNC_PATH, errno);
+
+       MM_RM_DEBUG("[%d] closed %s", fd, RELEASE_CB_SYNC_PATH);
+}
+
 static void __handle_release_callbacks(GArray *requests)
 {
        int i;
+       int sync_fd;
        mm_resource_manager_id id;
        mm_resource_manager_dmn_release_cb_request_s *request;
        mm_resource_manager_res_type_e type = MM_RESOURCE_MANAGER_RES_TYPE_MAX;
@@ -663,11 +685,20 @@ static void __handle_release_callbacks(GArray *requests)
                        continue;
                }
 
+               /* Avoid to emit release callback of resource manager's other resource, which can be changed by release policy*/
+               if (conf->is_release_all && mgr->is_dbus_release_emitted) {
+                       MM_RM_WARNING("Already sending release callback to [mgr %p] RM #%"PRIu64" for %s of volume %d", mgr, id, res_name, volume);
+                       continue;
+               }
+
                MM_RM_INFO("Sending release callback to [mgr %p] RM #%"PRIu64" for %s of volume %d", mgr, id, res_name, volume);
 
+               sync_fd = __open_release_cb_sync_fd();
+               MM_RM_RETM_IF(sync_fd == -1, "[%s] open failed", RELEASE_CB_SYNC_PATH);
+
                _mmrm_dmn_dbus_release_callback(id, type, volume);
 
-               if (__wait_for_release_cb_sync(id)) {
+               if (__wait_for_release_cb_sync(id, sync_fd)) {
                        MM_RM_INFO("[SYNC] Release callback success RM #%"PRIu64, id);
                        if (conf->volume_would_be_checked[type] && volume != MM_RESOURCE_MANAGER_RES_VOLUME_FULL) {
                                conf->max_volume[type] += volume;
@@ -680,11 +711,16 @@ static void __handle_release_callbacks(GArray *requests)
                        }
 
                        mgr->resources[type]->state = MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE;
+                       if (conf->is_release_all)
+                               mgr->is_dbus_release_emitted = TRUE;
+
                        MM_RM_DEBUG("RM #%"PRIu64" (type %s mgr %p) set acquired value as state (%s)",
                                id, res_name, mgr, res_state_str[mgr->resources[type]->state]);
                } else {
                        MM_RM_ERROR("Wait for release callback sync failed RM #%"PRIu64" (type %s)", id, res_name);
                }
+
+               __close_release_cb_sync_fd(sync_fd);
        }
 }
 
@@ -785,26 +821,26 @@ static gboolean __poll(struct pollfd sync, mm_resource_manager_id id)
        return FALSE;
 }
 
-static gboolean __wait_for_release_cb_sync(mm_resource_manager_id id)
+static gboolean __wait_for_release_cb_sync(mm_resource_manager_id id, int sync_fd)
 {
        gboolean ret = FALSE;
        struct pollfd sync = {0};
        mm_resource_manager_id recv_id;
        ssize_t read_size;
 
-       MM_RM_DEBUG("Enter");
-       sync.fd = open(RELEASE_CB_SYNC_PATH, O_RDONLY | O_NONBLOCK);
-       MM_RM_DEBUG("[%d] opened %s", sync.fd, RELEASE_CB_SYNC_PATH);
+       MM_RM_DEBUG("Enter %d", sync_fd);
 
-       MM_RM_RETVM_IF(sync.fd == -1, FALSE, "Sync FIFO cannot be opened");
+       MM_RM_RETVM_IF(sync_fd == -1, FALSE, "[sync_fd %d] invalid parameter", sync_fd);
+
+       sync.fd = sync_fd;
 
        if (!__poll(sync, id))
-               goto out;
+               return FALSE;
 
        read_size = read(sync.fd, &recv_id, sizeof(recv_id));
        if (read_size != sizeof(recv_id)) {
                MM_RM_ERROR("Read is failed (revents=%hd, read_size=%zd)", sync.revents, read_size);
-               goto out;
+               return FALSE;
        }
 
        if (id != _mm_rm_hash64(recv_id))
@@ -812,9 +848,5 @@ static gboolean __wait_for_release_cb_sync(mm_resource_manager_id id)
 
        ret = __poll(sync, id);
 
-out:
-       close(sync.fd);
-       MM_RM_DEBUG("[%d] closed", sync.fd);
-
        return ret;
 }
index 2b255c5..0dd7109 100644 (file)
@@ -63,6 +63,8 @@ typedef struct {
 
        GMutex resources_lock;
 
+       gboolean is_release_marked[MM_RESOURCE_MANAGER_RES_TYPE_MAX];
+
        mm_resource_manager_res_volume __max_resource_volumes
                [MM_RESOURCE_MANAGER_RES_TYPE_MAX];
        mm_resource_manager_res_volume __condition_volumes
@@ -93,7 +95,7 @@ static int __get_resource_index(mm_resource_manager_s *rm,
                mm_resource_manager_res_p res);
  /* FALSE if resource is destroyed */
 static gboolean __mark_resource_for_release(GPtrArray *resources, int index, mm_resource_manager_res_p resource);
-static void __send_release_cb_sync(mm_resource_manager_id id);
+static int __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, mm_resource_manager_res_type_e type, mm_resource_manager_res_volume volume);
 static void __mm_resource_manager_status_callback(mm_resource_manager_s *handle, mm_resource_manager_status_e status);
@@ -351,8 +353,8 @@ int _mm_resource_manager_mark_for_release(mm_resource_manager_h rm, mm_resource_
                resource->type >= MM_RESOURCE_MANAGER_RES_TYPE_MAX,
                MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION, "Type [%d] is out of range", resource->type);
        MM_RM_RETVM_IF(resource->state < MM_RESOURCE_MANAGER_RES_STATE_FOR_ACQUIRE ||
-               resource->state > MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE,
-               MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION, "State [%d] is out of range", resource->state);
+               resource->state >= MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE,
+               MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION, "State [%d] is out of range or already released", resource->state);
 
        res_name = _mm_resource_manager_get_res_str(resource->type);
 
@@ -370,6 +372,7 @@ int _mm_resource_manager_mark_for_release(mm_resource_manager_h rm, mm_resource_
                return MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION;
        }
 
+       handle->is_release_marked[resource->type] = TRUE;
        MM_RM_INFO("(%s) is marked for release in RM #%"PRIu64, res_name, handle_id);
 
        __mm_resources_lock(handle);
@@ -389,6 +392,42 @@ int _mm_resource_manager_mark_for_release(mm_resource_manager_h rm, mm_resource_
        return MM_RESOURCE_MANAGER_ERROR_NONE;
 }
 
+int _mm_resource_manager_release_other_resources(mm_resource_manager_s *handle)
+{
+       int idx;
+       int ret;
+       int len;
+
+       MM_RM_INFO("Enter %p", handle);
+
+       __mm_resource_handles_lock();
+       MM_RESOURCE_MANAGER_CHECK(handle);
+       __mm_resources_lock(handle);
+       handle->release_cb.is_invoked = TRUE;
+       MM_RM_INFO("other resource is released RM #%"PRIu64, _mm_rm_hash64(handle->id));
+       __mm_resource_handles_unlock();
+
+       len = handle->resources->len;
+
+       for (idx = 0; idx < len; idx++) {
+               if (!__mark_resource_for_release(handle->resources, idx, (mm_resource_manager_res_p) handle->resources->pdata[idx]))
+                       MM_RM_INFO("[idx %d] resource length %d", idx, len);
+       }
+
+       MM_RM_INFO("[RELEASE] dbus_commit");
+       ret = __dbus_commit(handle);
+       if (ret == MM_RESOURCE_MANAGER_ERROR_NONE)
+               MM_RM_DEBUG("Changes in RM #%"PRIu64" have been committed successfully", _mm_rm_hash64(handle->id));
+       else
+               MM_RM_ERROR("Dbus commit request failed");
+
+       __mm_resources_unlock(handle);
+
+       MM_RM_INFO("All resources are marked for release in RM #%"PRIu64, _mm_rm_hash64(handle->id));
+
+       return ret;
+}
+
 int _mm_resource_manager_mark_all_for_release(mm_resource_manager_h rm)
 {
        mm_resource_manager_s *handle = MM_RESOURCE_MANAGER(rm);
@@ -411,8 +450,12 @@ int _mm_resource_manager_mark_all_for_release(mm_resource_manager_h rm)
        len = handle->resources->len;
 
        for (idx = 0; idx < len; idx++) {
-               if (!__mark_resource_for_release(handle->resources, idx, (mm_resource_manager_res_p) handle->resources->pdata[idx]))
-                       MM_RM_INFO("[idx %d] resource length %d", idx, len);
+               if (!__mark_resource_for_release(handle->resources, idx, (mm_resource_manager_res_p) handle->resources->pdata[idx])) {
+                       MM_RM_WARNING("[idx %d] resource is already marked or released", idx);
+                       __mm_resources_unlock(handle);
+                       return MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION;
+               }
+               handle->is_release_marked[idx] = TRUE;
        }
 
        MM_RM_INFO("All resources are marked for release in RM #%"PRIu64, _mm_rm_hash64(handle->id));
@@ -467,6 +510,7 @@ int _mm_resource_manager_commit(mm_resource_manager_h rm)
        __mm_resources_lock(handle);
        __mm_resource_handles_unlock();
 
+       MM_RM_INFO("dbus_commit RM #%"PRIu64, _mm_rm_hash64(handle->id));
        ret = __dbus_commit(handle);
        if (ret == MM_RESOURCE_MANAGER_ERROR_NONE)
                MM_RM_DEBUG("Changes in RM #%"PRIu64" have been committed successfully", _mm_rm_hash64(handle->id));
@@ -544,6 +588,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;
+       mm_resource_manager_res_type_e t;
        gboolean release_all = FALSE;
        int idx;
        int prev_len;
@@ -563,6 +608,17 @@ static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle
                return;
        }
 
+       for (t = MM_RESOURCE_MANAGER_RES_TYPE_VIDEO_DECODER; t < MM_RESOURCE_MANAGER_RES_TYPE_MAX; t++) {
+               if (handle->is_release_marked[t] && t == type) {
+                       MM_RM_WARNING("mark_for_release() of %s is executed, which means that we can sync with rm dmn right now RM #%"PRIu64,
+                               res_name, handle_id);
+                       if (__send_release_cb_sync(handle->id) != MM_RESOURCE_MANAGER_ERROR_NONE)
+                               MM_RM_ERROR("__send_release_cb_sync is failed");
+                       __mm_resource_handles_unlock();
+                       return;
+               }
+       }
+
        __mm_resources_lock(handle);
        __mm_resource_handles_unlock();
        prev_len = handle->resources->len;
@@ -570,7 +626,7 @@ static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle
        MM_RM_DEBUG("resource %p length %d", handle->resources, prev_len);
 
        for (idx = 0; idx < prev_len; idx++) {
-               resource = (mm_resource_manager_res_s*)handle->resources->pdata[idx];
+               resource = (mm_resource_manager_res_s *)handle->resources->pdata[idx];
                if (resource->type == type && resource->volume == volume) {
                        /* FIXME : Set true in advance release callback invoking to prevent deadlock with resource marking */
                        handle->release_cb.is_invoked = TRUE;
@@ -579,7 +635,15 @@ static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle
                        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);
+                       if (__send_release_cb_sync(handle->id) != MM_RESOURCE_MANAGER_ERROR_NONE) {
+                               MM_RM_ERROR("__send_release_cb_sync is failed");
+                               handle->release_cb.is_invoked = FALSE;
+                               break;
+                       }
+
+                       /* If there is only one resource, the release callback value must be reset considering return value in case the handle is reused. */
+                       if (!release_all)
+                               handle->release_cb.is_invoked = FALSE;
 
                        g_ptr_array_remove_index_fast(handle->resources, idx);
 
@@ -588,18 +652,18 @@ static void __mm_resource_manager_release_callback(mm_resource_manager_s *handle
                        break;
                }
        }
-
-       handle->release_cb.is_invoked = FALSE;
-       MM_RM_WARNING("Reset release_cb release_cb.is_invoked RM #%"PRIu64, handle_id);
-
        __mm_resources_unlock(handle);
 
+       MM_RM_DEBUG("[%d] RELEASE ALL", release_all);
+
        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_release_other_resources(handle) == MM_RESOURCE_MANAGER_ERROR_NONE) {
                        MM_RM_DEBUG("All resources are released after release cb");
-               else
+                       handle->release_cb.is_invoked = FALSE;
+                       MM_RM_WARNING("Reset release_cb release_cb.is_invoked RM #%"PRIu64, handle_id);
+               } else {
                        MM_RM_WARNING("Resources cannot be released after release cb");
+               }
        }
 }
 
@@ -628,6 +692,7 @@ static void __mm_resource_handles_unlock(void)
 static void __mm_resources_lock(mm_resource_manager_s *h)
 {
        MM_RM_RETM_IF(!h, "handle is NULL");
+       LOGD(">> resource lock");
        g_mutex_lock(&h->resources_lock);
 }
 
@@ -635,6 +700,7 @@ static void __mm_resources_unlock(mm_resource_manager_s *h)
 {
        MM_RM_RETM_IF(!h, "handle is NULL");
        g_mutex_unlock(&h->resources_lock);
+       LOGD("<< resource unlock");
 }
 
 static int __check_resource(mm_resource_manager_s *rm, mm_resource_manager_res_type_e type, mm_resource_manager_res_volume volume)
@@ -720,8 +786,8 @@ static gboolean __mark_resource_for_release(GPtrArray *resources, int index, mm_
                MM_RM_DEBUG("Resource %p is successfully marked", resource);
                break;
        case MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE:
-               MM_RM_DEBUG("Resource %p is already marked", resource);
-               break;
+               MM_RM_DEBUG("Resource %p is already marked or released", resource);
+               return FALSE;
        }
 
        return TRUE;
@@ -738,23 +804,30 @@ static gboolean __check_rm_handle(mm_resource_manager_s *handle)
        return FALSE;
 }
 
-static void __send_release_cb_sync(mm_resource_manager_id id)
+static int __send_release_cb_sync(mm_resource_manager_id id)
 {
+       int ret = MM_RESOURCE_MANAGER_ERROR_NONE;
        int sync_fd;
 
        MM_RM_DEBUG("Enter");
        sync_fd = open(RELEASE_CB_SYNC_PATH, O_WRONLY | O_NONBLOCK);
        MM_RM_DEBUG("[%d] opened %s", sync_fd, RELEASE_CB_SYNC_PATH);
 
-       MM_RM_RETM_IF(sync_fd == -1, "Sync FIFO cannot be opened [errno %d]", errno);
+       MM_RM_RETVM_IF(sync_fd == -1, MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION,
+               "Sync FIFO cannot be opened [errno %d]", errno);
 
-       if (write(sync_fd, &id, sizeof(id)) == sizeof(id))
+       if (write(sync_fd, &id, sizeof(id)) == sizeof(id)) {
                MM_RM_INFO("[SYNC] message is sent successfully RM #%"PRIu64, _mm_rm_hash64(id));
-       else
+       } else {
                MM_RM_ERROR("[SYNC] message cannot be sent RM #%"PRIu64, _mm_rm_hash64(id));
+               ret = MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION;
+       }
 
-       close(sync_fd);
+       MM_RM_RETVM_IF(close(sync_fd) == -1, MM_RESOURCE_MANAGER_ERROR_INVALID_OPERATION,
+               "[%d] close failed [errno %d]", sync_fd, errno);
        MM_RM_DEBUG("[%d] closed", sync_fd);
+
+       return ret;
 }
 
 static void __init_lib()
@@ -966,6 +1039,7 @@ static int __dbus_commit(mm_resource_manager_s *handle)
                                break;
                        case MM_RESOURCE_MANAGER_RES_STATE_FOR_RELEASE:
                                g_ptr_array_remove_index_fast(handle->resources, i--);
+                               handle->is_release_marked[resource->type] = FALSE;
                                break;
                        default:
                                break;
@@ -998,11 +1072,19 @@ static void __dbus_release_callback(MMResourceManager *object, guint64 arg_id, g
        __mm_resource_handles_lock();
 
        for (i = 0; i < handles->len; i++) {
-               handle = (mm_resource_manager_s*)handles->pdata[i];
+               handle = (mm_resource_manager_s *)handles->pdata[i];
                handle_id = handle->id;
                MM_RM_HASH64(handle_id);
 
                if (handle->dbus_proxy == object && handle_id == arg_id) {
+                       if (handle->release_cb.is_invoked) {
+                               MM_RM_WARNING("other resource release cb is already executed RM #%"PRIu64, handle_id);
+                               if (__send_release_cb_sync(handle->id) != MM_RESOURCE_MANAGER_ERROR_NONE)
+                                       MM_RM_ERROR("__send_release_cb_sync is failed");
+                               __mm_resource_handles_unlock();
+                               return;
+                       }
+                       MM_RM_INFO("[release_callback] RM #%"PRIu64, handle_id);
                        __mm_resource_manager_release_callback(handle, arg_id, arg_resource_type, arg_volume);
                        return;
                }
@@ -1020,7 +1102,7 @@ static void __dbus_status_callback(MMResourceManager *object, gint arg_status)
        __mm_resource_handles_lock();
 
        for (i = 0; i < handles->len; i++) {
-               handle = (mm_resource_manager_s*)handles->pdata[i];
+               handle = (mm_resource_manager_s *)handles->pdata[i];
                if (handle->dbus_proxy == object) {
                        __mm_resource_manager_status_callback(handle, arg_status);
                        return;