From dbfcf19f2a42a085fe1a4b21186bc00eb7abe263 Mon Sep 17 00:00:00 2001 From: Hwankyu Jhun Date: Tue, 3 May 2022 11:39:00 +0900 Subject: [PATCH] Fix vconf implementation To avoid a deadlock issue, locking mutex is removed from _vconf_notify_cb(). A new mutex is added for thread safe of gsource_tbl. When _vconf_notify_cb() is called, idlers are added for the main thread and the ui thread. Change-Id: I32da85e2dd259d373ed7a0e210eb63158a56efb9 Signed-off-by: Hwankyu Jhun --- vconf-compat/vconf.c | 277 ++++++++++++++++++++++++++++++--------------------- 1 file changed, 166 insertions(+), 111 deletions(-) diff --git a/vconf-compat/vconf.c b/vconf-compat/vconf.c index 66a9166..21b945b 100644 --- a/vconf-compat/vconf.c +++ b/vconf-compat/vconf.c @@ -66,6 +66,7 @@ static struct buxton_layer *system_layer; static struct buxton_layer *memory_layer; static GHashTable *noti_tbl; static GHashTable *gsource_tbl; +static GRecMutex vconf_gsource_mutex; struct noti { char *key; @@ -77,14 +78,15 @@ struct noti_cb { void *user_data; bool active; bool in_cb; - GMainContext *thread_ctx; + bool use_tizen_context; }; struct callback_info { keynode_t *key_node; - struct noti_cb *noticb; gint id; guint source_id; + GMainContext *context; + bool use_tizen_context; }; static struct noti_cb *_vconf_find_noti_cb(struct noti *noti, vconf_callback_fn cb); @@ -100,6 +102,20 @@ static void _vconf_mutex_unlock(void) g_rec_mutex_unlock(&vconf_mutex); } +static GMainContext *_vconf_get_tizen_glib_context(void) +{ + GMainContext *context; + const char *env; + + env = getenv("TIZEN_GLIB_CONTEXT"); + if (env) + context = (GMainContext *)strtoul(env, NULL, 10); + else + context = NULL; + + return context; +} + static guint _vconf_idle_add_full(GMainContext *context, guint priority, GSourceFunc func, gpointer data) { @@ -145,65 +161,33 @@ static keynode_t *_vconf_create_keynode(const char *key, return node; } -static struct noti_cb *_vconf_copy_noti_cb(struct noti_cb *noticb) -{ - struct noti_cb *clone; - - if (!noticb) - return NULL; - - clone = calloc(1, sizeof(struct noti_cb)); - if (!clone) - return NULL; - - clone->cb = noticb->cb; - clone->user_data = noticb->user_data; - clone->active = noticb->active; - clone->in_cb = noticb->in_cb; - - if (noticb->thread_ctx) - clone->thread_ctx = g_main_context_ref(noticb->thread_ctx); - - return clone; -} - -static void _vconf_free_noti_cb(struct noti_cb *noticb) -{ - if (!noticb) - return; - - if (noticb->thread_ctx) - g_main_context_unref(noticb->thread_ctx); - - free(noticb); -} - static void _vconf_free_callback_info(struct callback_info *cb_info) { - GMainContext *context; GSource *source; if (!cb_info) return; - if (cb_info->source_id != 0 && cb_info->noticb) { - context = cb_info->noticb->thread_ctx; - source = g_main_context_find_source_by_id(context, + if (cb_info->source_id != 0) { + source = g_main_context_find_source_by_id(cb_info->context, cb_info->source_id); if (source && !g_source_is_destroyed(source)) g_source_destroy(source); } - _vconf_free_noti_cb(cb_info->noticb); + if (cb_info->context) + g_main_context_unref(cb_info->context); + _vconf_free_keynode(cb_info->key_node); free(cb_info); } static struct callback_info *_vconf_create_callback_info(const char *key, - const struct buxton_value *val, struct noti_cb *noticb) + const struct buxton_value *val, bool use_tizen_context) { static gint id; struct callback_info *cb_info; + GMainContext *context; cb_info = calloc(1, sizeof(struct callback_info)); if (!cb_info) @@ -215,16 +199,19 @@ static struct callback_info *_vconf_create_callback_info(const char *key, return NULL; } - cb_info->noticb = _vconf_copy_noti_cb(noticb); - if (!cb_info->noticb) { - _vconf_free_callback_info(cb_info); - return NULL; + cb_info->use_tizen_context = use_tizen_context; + if (use_tizen_context) { + context = _vconf_get_tizen_glib_context(); + if (context) + cb_info->context = g_main_context_ref(context); + else + LOGE("There is no tizen glib contexts"); } g_atomic_int_inc(&id); cb_info->id = id; - cb_info->source_id = _vconf_idle_add_full(cb_info->noticb->thread_ctx, + cb_info->source_id = _vconf_idle_add_full(cb_info->context, G_PRIORITY_HIGH, _vconf_call_noti_cb, cb_info); if (cb_info->source_id == 0) { _vconf_free_callback_info(cb_info); @@ -234,9 +221,49 @@ static struct callback_info *_vconf_create_callback_info(const char *key, return cb_info; } +static int _vconf_gsource_table_insert(struct callback_info *cb_info) +{ + if (cb_info == NULL) + return -1; + + g_rec_mutex_lock(&vconf_gsource_mutex); + if (gsource_tbl == NULL) { + gsource_tbl = g_hash_table_new_full(g_direct_hash, + g_direct_equal, NULL, + (GDestroyNotify)_vconf_free_callback_info); + if (gsource_tbl == NULL) { + LOGE("Out of memory"); + g_rec_mutex_unlock(&vconf_gsource_mutex); + return -1; + } + } + + g_hash_table_insert(gsource_tbl, GINT_TO_POINTER(cb_info->id), + cb_info); + g_rec_mutex_unlock(&vconf_gsource_mutex); + + return 0; +} + +static void _vconf_gsource_table_remove(struct callback_info *cb_info) +{ + if (cb_info == NULL) + return; + + g_rec_mutex_lock(&vconf_gsource_mutex); + if (gsource_tbl == NULL) { + g_rec_mutex_unlock(&vconf_gsource_mutex); + return; + } + + g_hash_table_remove(gsource_tbl, GINT_TO_POINTER(cb_info->id)); + g_rec_mutex_unlock(&vconf_gsource_mutex); +} + VCONF_CTOR static void _vconf_init(void) { g_rec_mutex_init(&vconf_mutex); + g_rec_mutex_init(&vconf_gsource_mutex); } VCONF_DTOR static void _vconf_finish(void) @@ -250,6 +277,11 @@ VCONF_DTOR static void _vconf_finish(void) if (gsource_tbl) g_hash_table_remove_all(gsource_tbl); + if (g_rec_mutex_trylock(&vconf_gsource_mutex)) + g_rec_mutex_unlock(&vconf_gsource_mutex); + + g_rec_mutex_clear(&vconf_gsource_mutex); + if (g_rec_mutex_trylock(&vconf_mutex)) g_rec_mutex_unlock(&vconf_mutex); @@ -381,7 +413,7 @@ static void _vconf_free_noti(struct noti *noti) if (!noticb->in_cb) { noti->noti_list = g_list_delete_link(noti->noti_list, l); - _vconf_free_noti_cb(noticb); + free(noticb); } } @@ -526,35 +558,94 @@ static void _vconf_to_vconf_t(const struct buxton_value *val, keynode_t *node) } } -static gboolean _vconf_call_noti_cb(gpointer data) +static GList *_vconf_copy_noti_list(GList *noti_list, bool use_tizen_context) +{ + struct noti_cb *noticb; + GList *iter; + GList *copy_list = NULL; + + if (noti_list == NULL) + return NULL; + + _vconf_mutex_lock(); + iter = noti_list; + while (iter) { + noticb = iter->data; + iter = g_list_next(iter); + + if (noticb->use_tizen_context == use_tizen_context) { + noticb->in_cb = true; + copy_list = g_list_append(copy_list, noticb); + } + } + _vconf_mutex_unlock(); + + return copy_list; +} + +static GList *_vconf_free_copy_list(GList *noti_list, GList *copy_list) { - struct callback_info *cb_info = (struct callback_info *)data; + struct noti_cb *noticb; + GList *iter; + + if (copy_list == NULL) + return noti_list; + + _vconf_mutex_lock(); + iter = copy_list; + while (iter) { + noticb = iter->data; + iter = g_list_next(iter); + + noticb->in_cb = false; + if (!noticb->active) { + noti_list = g_list_remove(noti_list, noticb); + free(noticb); + } + } + _vconf_mutex_unlock(); + g_list_free(copy_list); + + return noti_list; +} + +static gboolean _vconf_call_noti_cb(gpointer user_data) +{ + struct callback_info *cb_info = user_data; keynode_t *node = cb_info->key_node; - struct noti_cb *noticb = cb_info->noticb; + struct noti_cb *noticb; struct noti *noti; + GList *iter; + GList *copy_list; cb_info->source_id = 0; _vconf_mutex_lock(); noti = g_hash_table_lookup(noti_tbl, node->keyname); - if (!noti) { - g_hash_table_remove(gsource_tbl, GINT_TO_POINTER(cb_info->id)); + if (noti == NULL) { _vconf_mutex_unlock(); + _vconf_gsource_table_remove(cb_info); return G_SOURCE_REMOVE; } - if (!_vconf_find_noti_cb(noti, noticb->cb)) { - g_hash_table_remove(gsource_tbl, GINT_TO_POINTER(cb_info->id)); - _vconf_mutex_unlock(); - return G_SOURCE_REMOVE; - } + copy_list = _vconf_copy_noti_list(noti->noti_list, + cb_info->use_tizen_context); _vconf_mutex_unlock(); - noticb->cb(node, noticb->user_data); + iter = copy_list; + while (iter) { + noticb = iter->data; + iter = g_list_next(iter); + + if (!noticb->active) + continue; - _vconf_mutex_lock(); - g_hash_table_remove(gsource_tbl, GINT_TO_POINTER(cb_info->id)); - _vconf_mutex_unlock(); + assert(noticb->cb); + noticb->cb(node, noticb->user_data); + } + + noti->noti_list = _vconf_free_copy_list(noti->noti_list, copy_list); + _vconf_gsource_table_remove(cb_info); return G_SOURCE_REMOVE; } @@ -563,35 +654,22 @@ static void _vconf_notify_cb(const struct buxton_layer *layer, const char *key, const struct buxton_value *val, void *user_data) { struct callback_info *cb_info; - struct noti_cb *noticb; - struct noti* noti; - GList *iter; - _vconf_mutex_lock(); - noti = g_hash_table_lookup(noti_tbl, key); - if (!noti) { - _vconf_mutex_unlock(); + cb_info = _vconf_create_callback_info(key, val, false); + if (cb_info == NULL) return; - } - if (gsource_tbl == NULL) { - gsource_tbl = g_hash_table_new_full(g_direct_hash, - g_direct_equal, NULL, - (GDestroyNotify)_vconf_free_callback_info); + if (_vconf_gsource_table_insert(cb_info) != 0) { + _vconf_free_callback_info(cb_info); + return; } - for (iter = noti->noti_list; iter; iter = g_list_next(iter)) { - noticb = iter->data; - cb_info = _vconf_create_callback_info(key, val, noticb); - if (!cb_info) { - _vconf_mutex_unlock(); - return; - } + cb_info = _vconf_create_callback_info(key, val, true); + if (cb_info == NULL) + return; - g_hash_table_insert(gsource_tbl, GINT_TO_POINTER(cb_info->id), - cb_info); - } - _vconf_mutex_unlock(); + if (_vconf_gsource_table_insert(cb_info) != 0) + _vconf_free_callback_info(cb_info); } static struct noti_cb *_vconf_find_noti_cb(struct noti *noti, vconf_callback_fn cb) @@ -613,28 +691,10 @@ static struct noti_cb *_vconf_find_noti_cb(struct noti *noti, vconf_callback_fn return NULL; } -static GMainContext *_vconf_get_tizen_glib_context(void) -{ - static GMainContext *context; - static int initialized; - const char *env; - - if (initialized) - return context; - - env = getenv("TIZEN_GLIB_CONTEXT"); - if (env) - context = (GMainContext *)strtoul(env, NULL, 10); - - initialized = 1; - return context; -} - -static int _vconf_add_noti(struct noti *noti, vconf_callback_fn cb, void *user_data, - bool use_tizen_context) +static int _vconf_add_noti(struct noti *noti, vconf_callback_fn cb, + void *user_data, bool use_tizen_context) { struct noti_cb *noticb; - GMainContext *context; if (!noti || !cb) { LOGE("Invalid parameter"); @@ -662,12 +722,7 @@ static int _vconf_add_noti(struct noti *noti, vconf_callback_fn cb, void *user_d noticb->user_data = user_data; noticb->active = true; noticb->in_cb = false; - - if (use_tizen_context) { - context = _vconf_get_tizen_glib_context(); - if (context) - noticb->thread_ctx = g_main_context_ref(context); - } + noticb->use_tizen_context = use_tizen_context; noti->noti_list = g_list_append(noti->noti_list, noticb); @@ -850,7 +905,7 @@ EXPORT int vconf_ignore_key_changed(const char *key, vconf_callback_fn cb) noticb->active = false; if (!noticb->in_cb) { noti->noti_list = g_list_remove(noti->noti_list, noticb); - _vconf_free_noti_cb(noticb); + free(noticb); } cnt = _vconf_unregister_noti(noti); -- 2.7.4