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 <h.jhun@samsung.com>

1 files 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);