From fb7ac5632e2dd6a7a4057c6290d14cc0be0f8ea6 Mon Sep 17 00:00:00 2001 From: Jiwoong Im Date: Thu, 25 Aug 2016 17:40:28 +0900 Subject: [PATCH] extend lock scope of clients_lock & vconf_lock in library Change-Id: Ibdecd0e326e8b4f965f10dc4e08dfa7acd58f27e Signed-off-by: Jiwoong Im --- lib/buxton2.c | 259 ++++++++++++++++++++++++++++++++++++++++----------- vconf-compat/vconf.c | 23 ++--- 2 files changed, 213 insertions(+), 69 deletions(-) diff --git a/lib/buxton2.c b/lib/buxton2.c index 5310d2b..8dba4a1 100644 --- a/lib/buxton2.c +++ b/lib/buxton2.c @@ -699,8 +699,6 @@ static void proc_msg_cb(void *user_data, assert(client); - pthread_mutex_unlock(&clients_lock); - switch (type) { case MSG_NOTI: proc_msg_noti(client, data, len); @@ -723,8 +721,6 @@ static void proc_msg_cb(void *user_data, bxt_err("proc msg: unknown message type %d", type); break; } - - pthread_mutex_lock(&clients_lock); } static int proc_msg(struct buxton_client *client) @@ -734,16 +730,13 @@ static int proc_msg(struct buxton_client *client) assert(client); - pthread_mutex_lock(&clients_lock); f = g_list_find(clients, client); if (!f) { bxt_dbg("recv msg: cli %p removed\n", client); - pthread_mutex_unlock(&clients_lock); return 0; } r = proto_recv_async(client->fd, proc_msg_cb, client); - pthread_mutex_unlock(&clients_lock); if (r == -1) { bxt_err("recv msg: fd %d errno %d", client->fd, errno); return -1; @@ -911,10 +904,14 @@ EXPORT int buxton_set_value(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = set_value(client, layer, key, val, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -939,19 +936,26 @@ EXPORT int buxton_set_value_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = set_value(client, layer, key, val, set_value_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -999,10 +1003,14 @@ EXPORT int buxton_get_value(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = get_value(client, layer, key, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1035,21 +1043,28 @@ EXPORT int buxton_get_value_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = get_value(client, layer, key, get_value_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } *val = resp.val; + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1096,10 +1111,14 @@ EXPORT int buxton_list_keys(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = list_keys(client, layer, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1151,16 +1170,22 @@ EXPORT int buxton_list_keys_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = list_keys(client, layer, list_keys_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } @@ -1169,6 +1194,7 @@ EXPORT int buxton_list_keys_sync(struct buxton_client *client, if (len) *len = resp.nmlen; + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1275,21 +1301,28 @@ EXPORT int buxton_register_notification(struct buxton_client *client, return -1; } + pthread_mutex_lock(&clients_lock); r = find_noti(client, layer, key, ¬i); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (noti && noti->reg == TRUE) { r = add_noticb(noti, notify, notify_data); + pthread_mutex_unlock(&clients_lock); return call_resp(r == -1 ? errno : 0, layer, key, callback, user_data); } req = register_noti(client, layer, key, notify, notify_data, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1318,29 +1351,40 @@ EXPORT int buxton_register_notification_sync(struct buxton_client *client, return -1; } + pthread_mutex_lock(&clients_lock); r = find_noti(client, layer, key, ¬i); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } - if (noti && noti->reg == TRUE) + if (noti && noti->reg == TRUE) { + pthread_mutex_unlock(&clients_lock); return add_noticb(noti, notify, notify_data); + } memset(&resp, 0, sizeof(resp)); req = register_noti(client, layer, key, notify, notify_data, reg_noti_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1440,26 +1484,37 @@ EXPORT int buxton_unregister_notification(struct buxton_client *client, return -1; } + pthread_mutex_lock(&clients_lock); r = find_noti(client, layer, key, ¬i); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (!noti) { errno = ENOENT; + pthread_mutex_unlock(&clients_lock); return -1; } r = del_noticb(noti, notify, &cnt); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return call_resp(errno, layer, key, callback, user_data); + } - if (cnt || noti->reg == FALSE) + if (cnt || noti->reg == FALSE) { + pthread_mutex_unlock(&clients_lock); return call_resp(0, layer, key, callback, user_data); + } req = unregister_noti(client, layer, key, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1489,37 +1544,51 @@ EXPORT int buxton_unregister_notification_sync(struct buxton_client *client, return -1; } + pthread_mutex_lock(&clients_lock); r = find_noti(client, layer, key, ¬i); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (!noti) { errno = ENOENT; + pthread_mutex_unlock(&clients_lock); return -1; } r = del_noticb(noti, notify, &cnt); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } - if (cnt || noti->reg == FALSE) + if (cnt || noti->reg == FALSE) { + pthread_mutex_unlock(&clients_lock); return 0; + } memset(&resp, 0, sizeof(resp)); req = unregister_noti(client, layer, key, unreg_noti_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1575,11 +1644,15 @@ EXPORT int buxton_create_value(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = create_value(client, layer, key, read_privilege, write_privilege, val, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1605,20 +1678,27 @@ EXPORT int buxton_create_value_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = create_value(client, layer, key, read_privilege, write_privilege, val, create_value_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1666,10 +1746,14 @@ EXPORT int buxton_unset_value(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = unset_value(client, layer, key, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1693,19 +1777,26 @@ EXPORT int buxton_unset_value_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = unset_value(client, layer, key, unset_value_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1767,11 +1858,15 @@ EXPORT int buxton_set_privilege(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = set_priv(client, layer, key, type, privilege, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1797,14 +1892,19 @@ EXPORT int buxton_set_privilege_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = set_priv(client, layer, key, type, privilege, set_priv_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; @@ -1865,10 +1965,14 @@ EXPORT int buxton_get_privilege(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = get_priv(client, layer, key, type, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1902,16 +2006,22 @@ EXPORT int buxton_get_privilege_sync(struct buxton_client *client, memset(&resp, 0, sizeof(resp)); + pthread_mutex_lock(&clients_lock); req = get_priv(client, layer, key, type, get_priv_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } @@ -1919,6 +2029,7 @@ EXPORT int buxton_get_privilege_sync(struct buxton_client *client, resp.val->value.s = NULL; buxton_value_free(resp.val); + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1982,10 +2093,14 @@ EXPORT int buxton_enable_security(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = security_control(client, TRUE, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -1995,19 +2110,26 @@ EXPORT int buxton_enable_security_sync(struct buxton_client *client) struct bxt_req *req; struct response resp; + pthread_mutex_lock(&clients_lock); req = security_control(client, TRUE, security_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -2016,10 +2138,14 @@ EXPORT int buxton_disable_security(struct buxton_client *client, { struct bxt_req *req; + pthread_mutex_lock(&clients_lock); req = security_control(client, FALSE, callback, user_data); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -2029,19 +2155,26 @@ EXPORT int buxton_disable_security_sync(struct buxton_client *client) struct bxt_req *req; struct response resp; + pthread_mutex_lock(&clients_lock); req = security_control(client, FALSE, security_sync_cb, &resp); - if (!req) + if (!req) { + pthread_mutex_unlock(&clients_lock); return -1; + } r = wait_msg(client, req->msgid); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&clients_lock); return -1; + } if (resp.res) { errno = resp.res; + pthread_mutex_unlock(&clients_lock); return -1; } + pthread_mutex_unlock(&clients_lock); return 0; } @@ -2099,12 +2232,27 @@ static gboolean close_conn(gpointer data) static void free_client(struct buxton_client *cli) { + GList *f; + if (!cli) return; - close_conn(cli); + f = g_list_find(clients, cli); + if (f) { + if (cli->fd != -1) { + close(cli->fd); + cli->fd = -1; + } + + if (cli->fd_id) { + g_source_remove(cli->fd_id); + cli->fd_id = 0; + } + + if (cli->st_callback) + cli->st_callback(BUXTON_STATUS_DISCONNECTED, cli->st_data); + } - pthread_mutex_lock(&clients_lock); clients = g_list_remove(clients, cli); pthread_mutex_lock(&cli->lock); @@ -2116,7 +2264,6 @@ static void free_client(struct buxton_client *cli) pthread_mutex_unlock(&cli->lock); free(cli); - pthread_mutex_unlock(&clients_lock); } int connect_server(const char *addr) @@ -2179,15 +2326,16 @@ static gboolean recv_cb(gint fd, GIOCondition cond, gpointer data) pthread_mutex_unlock(&clients_lock); return G_SOURCE_REMOVE; } - pthread_mutex_unlock(&clients_lock); r = proc_msg(cli); if (r == -1) { cli->fd_id = 0; g_idle_add(close_conn, cli); + pthread_mutex_unlock(&clients_lock); return G_SOURCE_REMOVE; } + pthread_mutex_unlock(&clients_lock); return G_SOURCE_CONTINUE; } @@ -2200,14 +2348,17 @@ EXPORT int buxton_close(struct buxton_client *client) return -1; } + pthread_mutex_lock(&clients_lock); l = g_list_find(clients, client); if (!l) { errno = ENOENT; + pthread_mutex_unlock(&clients_lock); return -1; } free_client(client); + pthread_mutex_unlock(&clients_lock); return 0; } @@ -2285,9 +2436,11 @@ static void buxton_client_exit(void) GList *l; GList *n; + pthread_mutex_lock(&clients_lock); for (l = clients, n = g_list_next(l); l; l = n, n = g_list_next(n)) free_client(l->data); clients = NULL; + pthread_mutex_unlock(&clients_lock); } diff --git a/vconf-compat/vconf.c b/vconf-compat/vconf.c index 0171f8b..daa0ea2 100644 --- a/vconf-compat/vconf.c +++ b/vconf-compat/vconf.c @@ -180,11 +180,8 @@ static void free_noti(struct noti *noti) static void _close_for_noti(void) { - pthread_mutex_lock(&vconf_lock); - _refcnt--; if (_refcnt) { - pthread_mutex_unlock(&vconf_lock); return; } @@ -199,26 +196,20 @@ static void _close_for_noti(void) buxton_close(noti_client); noti_client = NULL; - - pthread_mutex_unlock(&vconf_lock); } static int _open_for_noti(void) { int r; - pthread_mutex_lock(&vconf_lock); - _refcnt++; if (_refcnt > 1) { - pthread_mutex_unlock(&vconf_lock); return 0; } r = buxton_open(¬i_client, NULL, NULL); if (r == -1) { LOGE("Can't connect to buxton: %d", errno); - pthread_mutex_unlock(&vconf_lock); return -1; } @@ -228,7 +219,6 @@ static int _open_for_noti(void) system_layer = buxton_create_layer("system"); memory_layer = buxton_create_layer("memory"); - pthread_mutex_unlock(&vconf_lock); return 0; } @@ -490,17 +480,19 @@ EXPORT int vconf_notify_key_changed(const char *key, vconf_callback_fn cb, return -1; } + pthread_mutex_lock(&vconf_lock); r = _open_for_noti(); - if (r == -1) + if (r == -1) { + pthread_mutex_unlock(&vconf_lock); return -1; + } - pthread_mutex_lock(&vconf_lock); noti = g_hash_table_lookup(noti_tbl, key); if (!noti) { noti = create_noti(key, cb, user_data); if (!noti) { - pthread_mutex_unlock(&vconf_lock); _close_for_noti(); + pthread_mutex_unlock(&vconf_lock); return -1; } r = buxton_register_notification_sync(noti_client, get_layer(key), key, @@ -510,16 +502,15 @@ EXPORT int vconf_notify_key_changed(const char *key, vconf_callback_fn cb, key, errno); g_hash_table_remove(noti_tbl, key); } - pthread_mutex_unlock(&vconf_lock); /* increase reference count */ if (r == 0) _open_for_noti(); } else { r = add_noti(noti, cb, user_data); - pthread_mutex_unlock(&vconf_lock); } _close_for_noti(); + pthread_mutex_unlock(&vconf_lock); if (r == 0) last_result = true; @@ -593,11 +584,11 @@ EXPORT int vconf_ignore_key_changed(const char *key, vconf_callback_fn cb) noti = g_hash_table_lookup(noti_tbl, key); if (noti) g_hash_table_remove(noti_tbl, key); - pthread_mutex_unlock(&vconf_lock); /* decrease reference count */ _close_for_noti(); + pthread_mutex_unlock(&vconf_lock); return 0; } -- 2.7.4