extend lock scope of clients_lock & vconf_lock in library

Change-Id: Ibdecd0e326e8b4f965f10dc4e08dfa7acd58f27e
Signed-off-by: Jiwoong Im <jiwoong.im@samsung.com>

diff --git a/lib/buxton2.c b/lib/buxton2.c
index 5310d2b..8dba4a1 100644 (file)
--- 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, &noti);
-       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, &noti);
-       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, &noti);
-       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, &noti);
-       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 (file)
--- 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(&noti_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;
 }