Add the condition race at all the instance_queue 09/167909/3 accepted/tizen/unified/20180124.061536 submit/tizen/20180123.080531
authorYoungHun Kim <yh8004.kim@samsung.com>
Tue, 23 Jan 2018 03:15:22 +0000 (12:15 +0900)
committerYoungHun Kim <yh8004.kim@samsung.com>
Tue, 23 Jan 2018 06:32:40 +0000 (15:32 +0900)
Change-Id: I96983d314ac0bb726ffe1f168a6fec77b80a51ee

server/include/muse_server_connection.h
server/src/muse_server_connection.c
server/src/muse_server_private.c
server/src/muse_server_system.c

index 4364af3..eda80f7 100644 (file)
@@ -55,6 +55,8 @@ void ms_connection_deinit(ms_connection_t *connection);
 int ms_connection_register(muse_module_h m);
 int ms_connection_unregister(muse_module_h m);
 ms_event_e ms_connection_event_trigger(int *value);
+void ms_connection_lock(ms_connection_t *connection);
+void ms_connection_unlock(ms_connection_t *connection);
 
 #ifdef __cplusplus
 }
index 55d3a42..af74022 100644 (file)
@@ -43,6 +43,11 @@ static void _ms_connection_module_instance_info(muse_module_h m, ms_connection_t
 
        for (idx = 0; idx < len; idx++) {
                connecting_m = (muse_module_h)g_queue_peek_nth(queue, idx);
+               if (!connecting_m) {
+                       LOGW("[%d] Make sure if the queue length is changed (%d = %d), which means that it was destroyed somewhere",
+                               idx, len, g_queue_get_length(queue));
+                       continue;
+               }
                snprintf(pid, MUSE_PARAM_MAX, "%d ", connecting_m->pid);
                strncat(pids, pid, MUSE_MSG_LEN_MAX - strlen(pids) - 1);
        }
@@ -65,7 +70,7 @@ int ms_connection_register(muse_module_h m)
        g_return_val_if_fail(connection->instance_queue, MM_ERROR_INVALID_ARGUMENT);
        g_return_val_if_fail(m, MM_ERROR_INVALID_ARGUMENT);
 
-       g_mutex_lock(&connection->lock);
+       ms_connection_lock(connection);
 
        queue = connection->instance_queue;
 
@@ -85,7 +90,7 @@ int ms_connection_register(muse_module_h m)
 
        _ms_connection_module_instance_info(m, connection, queue, API_CREATE);
 
-       g_mutex_unlock(&connection->lock);
+       ms_connection_unlock(connection);
 
        LOGD("Leave");
 
@@ -105,7 +110,7 @@ int ms_connection_unregister(muse_module_h m)
        g_return_val_if_fail(connection->instance_queue, MM_ERROR_INVALID_ARGUMENT);
        g_return_val_if_fail(m, MM_ERROR_INVALID_ARGUMENT);
 
-       g_mutex_lock(&connection->lock);
+       ms_connection_lock(connection);
 
        queue = connection->instance_queue;
 
@@ -122,7 +127,7 @@ int ms_connection_unregister(muse_module_h m)
 
        _ms_connection_module_instance_info(m, connection, queue, API_DESTROY);
 
-       g_mutex_unlock(&connection->lock);
+       ms_connection_unlock(connection);
 
        LOGD("Leave");
 
@@ -164,6 +169,18 @@ ms_event_e ms_connection_event_trigger(int *value)
        return event_value;
 }
 
+void ms_connection_lock(ms_connection_t *connection)
+{
+       g_return_if_fail(connection);
+       g_mutex_lock(&connection->lock);
+}
+
+void ms_connection_unlock(ms_connection_t *connection)
+{
+       g_return_if_fail(connection);
+       g_mutex_unlock(&connection->lock);
+}
+
 void ms_connection_deinit(ms_connection_t *connection)
 {
        g_return_if_fail(connection);
index 9df8d11..8312bdf 100644 (file)
@@ -252,10 +252,12 @@ static gboolean _ms_connection_handler(GIOChannel *source, GIOCondition conditio
        intptr_t module_addr = 0;
        ms_workqueue_job_t *job = NULL;
        GQueue *instance_queue = NULL;
+       ms_connection_t *connection = ms_get_instance()->connection;
 
        LOGI("Enter");
 
        g_return_val_if_fail(muse_server, FALSE);
+       g_return_val_if_fail(connection, FALSE);
 
        _ms_lock_state();
 
@@ -301,7 +303,9 @@ static gboolean _ms_connection_handler(GIOChannel *source, GIOCondition conditio
        } else {
                _ms_get_module_addr(client_sockfd, &module_addr);
 
-               instance_queue = ms_get_instance()->connection->instance_queue;
+               ms_connection_lock(connection);
+
+               instance_queue = connection->instance_queue;
                len = g_queue_get_length(instance_queue);
 
                m = (muse_module_h)module_addr;
@@ -309,7 +313,8 @@ static gboolean _ms_connection_handler(GIOChannel *source, GIOCondition conditio
                for (idx = 0; idx < len; idx++) {
                        peeked_m = (muse_module_h)g_queue_peek_nth(instance_queue, idx);
                        if (!peeked_m) {
-                               LOGW("%d's th module of queue is NULL", idx);
+                               LOGW("[%d] Make sure if the queue length is changed (%d = %d), which means that it was destroyed somewhere",
+                                       idx, len, g_queue_get_length(instance_queue));
                                continue;
                        }
 
@@ -319,6 +324,7 @@ static gboolean _ms_connection_handler(GIOChannel *source, GIOCondition conditio
                        if (!m) {
                                if (candidate_m) {
                                        LOGE("muse-server can't support the error case which threre are several modules now");
+                                       ms_connection_unlock(connection);
                                        goto out;
                                }
 
@@ -334,20 +340,26 @@ static gboolean _ms_connection_handler(GIOChannel *source, GIOCondition conditio
                                continue;
 
                        if (muse_core_fd_is_valid(m->ch[MUSE_CHANNEL_DATA].sock_fd)) {
-                               SECURE_LOGE("[%d] %s pid %d %p you had better check if instance destroy completed properly", client_sockfd, muse_server->conf->host[m->idx], pid, m);
+                               SECURE_LOGE("[%d] %s pid %d %p you had better check if instance destroy completed properly",
+                                       client_sockfd, muse_server->conf->host[m->idx], pid, m);
+                               ms_connection_unlock(connection);
                                goto out;
                        }
 
                        m->ch[MUSE_CHANNEL_DATA].sock_fd = client_sockfd;
-                       SECURE_LOGI("%s (pid %d) module : %p module addr from client : %p", muse_server->conf->host[m->idx], pid, m, (void *)module_addr);
+                       SECURE_LOGI("%s (pid %d) module : %p module addr from client : %p",
+                               muse_server->conf->host[m->idx], pid, m, (void *)module_addr);
                        break;
                }
 
                if (candidate_m) {
                        m = candidate_m;
                        m->ch[MUSE_CHANNEL_DATA].sock_fd = client_sockfd;
-                       SECURE_LOGW("[%d] %s pid %d %p restore module address at the only one null data channel", client_sockfd, muse_server->conf->host[m->idx], pid, m);
+                       SECURE_LOGW("[%d] %s pid %d %p restore module address at the only one null data channel",
+                               client_sockfd, muse_server->conf->host[m->idx], pid, m);
                }
+
+               ms_connection_unlock(connection);
        }
 
        job = calloc(1, sizeof(ms_workqueue_job_t));
@@ -550,22 +562,32 @@ void ms_check_memory(int pid)
 {
        int used_pss_mb = ms_system_get_memory_info(pid);
        char err_msg[MUSE_MSG_LEN_MAX] = {'\0',};
+       ms_connection_t *connection = ms_get_instance()->connection;
+       ms_config_t *conf = ms_get_instance()->conf;
+
+       g_return_if_fail(connection);
+       g_return_if_fail(conf);
 
        _ms_lock_state();
 
-       if (g_queue_is_empty(ms_get_instance()->connection->instance_queue) && muse_server->state != MUSE_SERVER_STATE_IDLE) {
-               if (used_pss_mb > ms_get_instance()->conf->memory_threshold) {
+       ms_connection_lock(connection);
+
+       if (g_queue_is_empty(connection->instance_queue) && muse_server->state != MUSE_SERVER_STATE_IDLE) {
+               if (used_pss_mb > conf->memory_threshold) {
                        muse_server->state = MUSE_SERVER_STATE_IDLE;
                        ms_log_process_info(pid);
 
-                       snprintf(err_msg, sizeof(err_msg), "[Memory Leak] %d > %d (KB)", used_pss_mb, ms_get_instance()->conf->memory_threshold);
+                       snprintf(err_msg, sizeof(err_msg), "[Memory Leak] %d > %d (KB)", used_pss_mb, conf->memory_threshold);
 
                        LOGE("%s", err_msg);
+                       ms_connection_unlock(connection);
                        _ms_unlock_state();
                        ms_respawn(SIGTERM);
                }
        }
 
+       ms_connection_unlock(connection);
+
        _ms_unlock_state();
 }
 
index c2d2fab..b2cb7ea 100644 (file)
@@ -33,7 +33,7 @@ enum muse_poweroff_type {
 
 static void _ms_external_storage_state_changed_cb(int storage_id, storage_dev_e dev, storage_state_e state, const char *fstype,
                        const char *fsuuid, const char *mount_path, bool primary, int flags, void *user_data);
-static void _ms_poweroff_state_changed_cb(GDBusConnection *connection, const gchar *sender_name, const gchar *object_path,
+static void _ms_poweroff_state_changed_cb(GDBusConnection *con, const gchar *sender_name, const gchar *object_path,
                        const gchar *interface_name, const gchar *signal_name, GVariant *parameters, gpointer user_data);
 static void _ms_system_unsubscribe_poweroff_state_change(void);
 static gboolean _ms_system_free_key(gpointer key, gpointer value, gpointer user_data);
@@ -45,17 +45,19 @@ static void _ms_system_unsubscribe_poweroff_state_change(void)
        g_dbus_connection_signal_unsubscribe(system->connection, system->muse_poweroff_id);
 }
 
-static void _ms_poweroff_state_changed_cb(GDBusConnection *connection, const gchar *sender_name, const gchar *object_path,
+static void _ms_poweroff_state_changed_cb(GDBusConnection *con, const gchar *sender_name, const gchar *object_path,
                        const gchar *interface_name, const gchar *signal_name, GVariant *parameters, gpointer user_data)
 {
        int val_int = 0;
        GQueue *queue;
        ms_cmd_dispatcher_info_t dispatch;
        ms_system_t *system = ms_get_instance()->system;
+       ms_connection_t *connection = ms_get_instance()->connection;
 
        LOGE("received power off signal");
 
        g_return_if_fail(system);
+       g_return_if_fail(connection);
        g_return_if_fail(object_path);
        g_return_if_fail(signal_name);
        g_return_if_fail(parameters);
@@ -76,12 +78,16 @@ static void _ms_poweroff_state_changed_cb(GDBusConnection *connection, const gch
 
        g_mutex_lock(&system->lock);
 
-       queue = ms_get_instance()->connection->instance_queue;
+       ms_connection_lock(connection);
+
+       queue = connection->instance_queue;
 
        dispatch.cmd = MUSE_MODULE_COMMAND_SHUTDOWN;
 
        g_queue_foreach(queue, ms_cmd_dispatch_foreach_func, (gpointer)&dispatch);
 
+       ms_connection_unlock(connection);
+
        g_mutex_unlock(&system->lock);
 
        exit(EXIT_FAILURE);
@@ -93,8 +99,10 @@ static void _ms_external_storage_state_changed_cb(int storage_id, storage_dev_e
        GQueue *queue;
        ms_cmd_dispatcher_info_t dispatch;
        ms_system_t *system = (ms_system_t *)user_data;
+       ms_connection_t *connection = ms_get_instance()->connection;
 
        g_return_if_fail(system);
+       g_return_if_fail(connection);
 
        switch (state) {
        case STORAGE_STATE_UNMOUNTABLE:
@@ -115,7 +123,9 @@ static void _ms_external_storage_state_changed_cb(int storage_id, storage_dev_e
 
        g_mutex_lock(&system->lock);
 
-       queue = ms_get_instance()->connection->instance_queue;
+       ms_connection_lock(connection);
+
+       queue = connection->instance_queue;
 
        dispatch.cmd = MUSE_MODULE_COMMAND_EXTERNAL_STORAGE_STATE_CHANGED;
 
@@ -125,6 +135,8 @@ static void _ms_external_storage_state_changed_cb(int storage_id, storage_dev_e
 
        g_queue_foreach(queue, ms_cmd_dispatch_foreach_func, (gpointer)&dispatch);
 
+       ms_connection_unlock(connection);
+
        g_mutex_unlock(&system->lock);
 }
 
@@ -258,7 +270,7 @@ int ms_system_get_memory_info(int pid)
 
        if (MM_ERROR_NONE == ret && info) {
                used_pss_mb = info->pss;
-               LOGD("[%d] Proportional set size %d (KB)", pid, used_pss_mb);
+               LOGI("[%d] Proportional set size %d (KB)", pid, used_pss_mb);
        } else {
                LOGE("Fail to get process (%d) memory %s", pid, get_error_message(ret));
        }