From: YoungHun Kim Date: Wed, 27 Jan 2021 06:29:00 +0000 (+0900) Subject: Fix the issue of release_cb_invoked X-Git-Tag: submit/tizen/20210221.233041^0 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5dc1dee2bc1ec08855b426363b56a5c29523f9a0;p=platform%2Fcore%2Fmultimedia%2Fmm-resource-manager.git Fix the issue of release_cb_invoked - 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 --- diff --git a/packaging/mm-resource-manager.spec b/packaging/mm-resource-manager.spec index 906a0df..645f800 100644 --- 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.44 +Version: 0.2.45 Release: 0 Group: Multimedia/API License: Apache-2.0 diff --git a/src/daemon/mm_resource_manager_daemon_conf.c b/src/daemon/mm_resource_manager_daemon_conf.c index d834778..d3f7576 100644 --- a/src/daemon/mm_resource_manager_daemon_conf.c +++ b/src/daemon/mm_resource_manager_daemon_conf.c @@ -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); diff --git a/src/daemon/mm_resource_manager_daemon_conf.h b/src/daemon/mm_resource_manager_daemon_conf.h index 7534665..d1bf63a 100644 --- a/src/daemon/mm_resource_manager_daemon_conf.h +++ b/src/daemon/mm_resource_manager_daemon_conf.h @@ -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); diff --git a/src/daemon/mm_resource_manager_daemon_priv.c b/src/daemon/mm_resource_manager_daemon_priv.c index 139b784..7533179 100644 --- a/src/daemon/mm_resource_manager_daemon_priv.c +++ b/src/daemon/mm_resource_manager_daemon_priv.c @@ -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; } diff --git a/src/lib/mm_resource_manager_priv.c b/src/lib/mm_resource_manager_priv.c index 2b255c5..0dd7109 100644 --- a/src/lib/mm_resource_manager_priv.c +++ b/src/lib/mm_resource_manager_priv.c @@ -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;