From: jusung son Date: Wed, 5 Jun 2024 00:19:22 +0000 (+0900) Subject: Release version 1.4.25 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=HEAD;hp=2908e633b87367f14f6ad52acb1e56d628bc73a3;p=platform%2Fcore%2Fappfw%2Fmessage-port.git Release version 1.4.25 Changes: - Fix dbus timing issue - Fix thread safe issue Change-Id: Ie5713c0db996536a14175ef2d1fa0b3d8a422eeb Signed-off-by: jusung son --- diff --git a/packaging/message-port.spec b/packaging/message-port.spec index 706f21c..d4f8171 100644 --- a/packaging/message-port.spec +++ b/packaging/message-port.spec @@ -1,6 +1,6 @@ Name: message-port Summary: Message Port library -Version: 1.4.24 +Version: 1.4.25 Release: 0 Group: Application Framework/Libraries License: Apache-2.0 diff --git a/src/message_port_common.c b/src/message_port_common.c index 77519bc..af01ea9 100644 --- a/src/message_port_common.c +++ b/src/message_port_common.c @@ -43,7 +43,10 @@ GDBusConnection *gdbus_conn; char *app_id; static const int MAX_MESSAGE_SIZE = 16 * 1024; -static pthread_mutex_t _message_port_mutex = PTHREAD_MUTEX_INITIALIZER; +static GRecMutex __rec_mutex; + +#define MESSAGE_PORT_CTOR __attribute__((constructor)) +#define MESSAGE_PORT_DTOR __attribute__((destructor)) int write_socket(int fd, const char *buffer, @@ -345,12 +348,27 @@ char *get_encoded_name(const char *remote_app_id, const char *port_name, bool is return md5_digest; } +MESSAGE_PORT_CTOR static void __message_port_init(void) +{ + g_rec_mutex_init(&__rec_mutex); +} + +MESSAGE_PORT_DTOR static void __message_port_fini(void) +{ + if (g_rec_mutex_trylock(&__rec_mutex)) + g_rec_mutex_unlock(&__rec_mutex); + else + _LOGE("rec_mutex is locked by another thread."); + + g_rec_mutex_clear(&__rec_mutex); +} + void message_port_lock_mutex() { - pthread_mutex_lock(&_message_port_mutex); + g_rec_mutex_lock(&__rec_mutex); } void message_port_unlock_mutex() { - pthread_mutex_unlock(&_message_port_mutex); + g_rec_mutex_unlock(&__rec_mutex); } diff --git a/src/message_port_remote.c b/src/message_port_remote.c index c18dc48..31463fc 100644 --- a/src/message_port_remote.c +++ b/src/message_port_remote.c @@ -47,7 +47,7 @@ static bool _initialized = false; static GHashTable *__local_port_info; static GHashTable *__trusted_app_list_hash; -static GHashTable *__callback_info_hash; +static GList *__callback_info_list; static GHashTable *__sender_appid_hash; typedef struct message_port_pkt { @@ -67,11 +67,6 @@ typedef struct message_port_callback_info { int g_src_id; } message_port_callback_info_s; -typedef struct callback_key_info { - int local_id; - message_port_callback_info_s *callback_info; -} callback_key_info_s; - static void __callback_info_free(gpointer data) { message_port_callback_info_s *callback_info = (message_port_callback_info_s *)data; @@ -110,28 +105,34 @@ static void __callback_info_free(gpointer data) /* LCOV_EXCL_START */ static void __callback_info_free_by_info(message_port_callback_info_s *callback_info) { - GList *callback_info_list = g_hash_table_lookup(__callback_info_hash, GUINT_TO_POINTER(callback_info->local_id)); GList *find_list; - if (callback_info_list == NULL) - return; - - find_list = g_list_find(callback_info_list, callback_info); + find_list = g_list_find(__callback_info_list, callback_info); if (find_list == NULL) return; - callback_info_list = g_list_remove_link(callback_info_list, find_list); + __callback_info_list = g_list_remove_link(__callback_info_list, find_list); __callback_info_free(callback_info); g_list_free(find_list); } -/* LCOV_EXCL_STOP */ -static void __hash_destroy_callback_info(gpointer data) +static void __callback_info_free_by_local_id(int local_id) { - GList *callback_list = (GList *)data; - - if (callback_list != NULL) - g_list_free_full(callback_list, __callback_info_free); + GList *iter; + GList *callback_info; + + for (iter = __callback_info_list; iter != NULL; ) { + if (((message_port_callback_info_s *)iter->data)->local_id == local_id) { + callback_info = iter; + iter = iter->next; + __callback_info_list = + g_list_remove_link(__callback_info_list, callback_info); + __callback_info_free(callback_info->data); + g_list_free(callback_info); + continue; + } + iter = iter->next; + } } /* LCOV_EXCL_START */ @@ -168,11 +169,6 @@ static bool __initialize(void) retvm_if(!__trusted_app_list_hash, false, "fail to create __trusted_app_list_hash"); } - if (__callback_info_hash == NULL) { - __callback_info_hash = g_hash_table_new_full(g_direct_hash, g_direct_equal, NULL, __hash_destroy_callback_info); - retvm_if(!__callback_info_hash, false, "fail to create __callback_info_hash"); - } - _initialized = true; return true; @@ -297,35 +293,26 @@ static message_port_pkt_s *__message_port_recv_raw(int fd) return pkt; } -static bool __validate_callback_info(callback_key_info_s *key_info) +static bool __validate_callback_info(message_port_callback_info_s *callback_info) { GList *cb_list; - cb_list = g_hash_table_lookup(__callback_info_hash, - GUINT_TO_POINTER(key_info->local_id)); + cb_list = g_list_find(__callback_info_list, callback_info); if (cb_list == NULL) { - _LOGI("local_info : %d is already released", key_info->local_id); - return false; - } - - cb_list = g_list_find(cb_list, key_info->callback_info); - if (cb_list == NULL) { - _LOGI("local_info : %d is already released from list", - key_info->local_id); + _LOGI("local_info : %p is already released from list", callback_info); return false; } return true; } -static bool __validate_local_info(callback_key_info_s *key_info) +static bool __validate_local_info(int local_id) { GList *cb_list; - cb_list = g_hash_table_lookup(__local_port_info, - GUINT_TO_POINTER(key_info->local_id)); + cb_list = g_hash_table_lookup(__local_port_info, GUINT_TO_POINTER(local_id)); if (cb_list == NULL) { - _LOGI("local_info : %d is already released", key_info->local_id); + _LOGI("local_info : %d is already released", local_id); return false; } @@ -337,36 +324,30 @@ static gboolean __socket_request_handler(GIOChannel *gio, gpointer data) { int fd = 0; - message_port_callback_info_s *mi = NULL; - callback_key_info_s *key_info; + message_port_callback_info_s *callback_info; message_port_pkt_s *pkt = NULL; message_port_local_port_info_s *local_port_info; bundle *kb = NULL; bool ret = true; bool existed = true; - key_info = (callback_key_info_s *)data; - if (key_info == NULL) + callback_info = (message_port_callback_info_s *)data; + if (callback_info == NULL) return FALSE; message_port_lock_mutex(); - if (__validate_callback_info(key_info) == false) { + if (__validate_callback_info(callback_info) == false) { ret = FALSE; existed = FALSE; - message_port_unlock_mutex(); goto out; } - if (__validate_local_info(key_info) == false) { + if (__validate_local_info(callback_info->local_id) == false) { ret = FALSE; - message_port_unlock_mutex(); goto out; } - message_port_unlock_mutex(); - mi = key_info->callback_info; - - local_port_info = mi->local_info; + local_port_info = callback_info->local_info; if (local_port_info == NULL || local_port_info->callback == NULL) { _LOGE("Failed to get callback info"); ret = FALSE; @@ -401,11 +382,13 @@ static gboolean __socket_request_handler(GIOChannel *gio, } if (pkt->is_bidirection) - local_port_info->callback(mi->local_id, mi->remote_app_id, - pkt->remote_port_name, pkt->is_trusted, kb, local_port_info->user_data); + local_port_info->callback(callback_info->local_id, + callback_info->remote_app_id, pkt->remote_port_name, + pkt->is_trusted, kb, local_port_info->user_data); else - local_port_info->callback(mi->local_id, mi->remote_app_id, - NULL, pkt->is_trusted, kb, local_port_info->user_data); + local_port_info->callback(callback_info->local_id, + callback_info->remote_app_id, NULL, + pkt->is_trusted, kb, local_port_info->user_data); bundle_free(kb); @@ -418,39 +401,12 @@ out: free(pkt); } - if (mi && ret == FALSE && existed == TRUE) { - message_port_lock_mutex(); - __callback_info_free_by_info(mi); - message_port_unlock_mutex(); - } - - return ret; -} - + if (ret == FALSE && existed == TRUE) + __callback_info_free_by_info(callback_info); -static void __socket_destroy_handler(gpointer data) -{ - _LOGI("__socket_destroy_handler"); - callback_key_info_s *key_info = (callback_key_info_s *)data; - free(key_info); -} - -static callback_key_info_s *__create_callback_key_info(message_port_callback_info_s *callback_info) -{ - callback_key_info_s *_key_info = (callback_key_info_s *) - calloc(1, sizeof(callback_key_info_s)); - - if (_key_info == NULL) { -/* LCOV_EXCL_START */ - _LOGE("out of memory"); - return NULL; -/* LCOV_EXCL_STOP */ - } - - _key_info->local_id = callback_info->local_id; - _key_info->callback_info = callback_info; + message_port_unlock_mutex(); - return _key_info; + return ret; } static message_port_callback_info_s *__create_callback_info(message_port_local_port_info_s *mi, char *local_appid) @@ -510,55 +466,26 @@ out: return callback_info; } -static bool __callback_info_append(message_port_callback_info_s *callback_info) +static void __callback_info_append(message_port_callback_info_s *callback_info) { - GList *callback_info_list = NULL; - message_port_callback_info_s *head_callback_info; - - callback_info_list = g_hash_table_lookup(__callback_info_hash, GUINT_TO_POINTER(callback_info->local_id)); - if (callback_info_list == NULL) { - head_callback_info = (message_port_callback_info_s *)calloc(1, sizeof(message_port_callback_info_s)); - if (head_callback_info == NULL) { -/* LCOV_EXCL_START */ - _LOGE("fail to alloc head_callback_info"); - return false; -/* LCOV_EXCL_STOP */ - } - head_callback_info->local_id = 0; - head_callback_info->remote_app_id = NULL; - head_callback_info->local_info = NULL; - head_callback_info->gio_read = NULL; - head_callback_info->g_src_id = 0; - callback_info_list = g_list_append(callback_info_list, head_callback_info); - callback_info_list = g_list_append(callback_info_list, callback_info); - g_hash_table_insert(__callback_info_hash, GUINT_TO_POINTER(callback_info->local_id), callback_info_list); - } else { - callback_info_list = g_list_append(callback_info_list, callback_info); - } - - return true; + __callback_info_list = g_list_append(__callback_info_list, callback_info); } static void __callback_info_update_user_data(int local_id, message_port_message_cb callback, void *user_data) { - GList *callback_info_list; GList *iter; message_port_callback_info_s *callback_info; - callback_info_list = g_hash_table_lookup(__callback_info_hash, GUINT_TO_POINTER(local_id)); - if (callback_info_list != NULL) { /* LCOV_EXCL_START */ - for (iter = callback_info_list; iter != NULL; iter = iter->next) { - callback_info = (message_port_callback_info_s *)iter->data; - if (callback_info->local_info != NULL) { - callback_info->local_info->callback = callback; - callback_info->local_info->user_data = user_data; - } + for (iter = __callback_info_list; iter != NULL; iter = iter->next) { + callback_info = (message_port_callback_info_s *)iter->data; + if (callback_info->local_id == local_id + && callback_info->local_info != NULL) { + callback_info->local_info->callback = callback; + callback_info->local_info->user_data = user_data; } -/* LCOV_EXCL_STOP */ - } else { - _LOGE("fail to find local_id %d ", local_id); } +/* LCOV_EXCL_STOP */ } static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invocation) @@ -577,7 +504,6 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc message_port_local_port_info_s *mi; int local_reg_id = 0; message_port_callback_info_s *callback_info = NULL; - callback_key_info_s *key_info; char buf[1024]; GDBusMessage *msg; @@ -588,6 +514,7 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc bool ret = false; char *key_appid; + message_port_lock_mutex(); g_variant_get(parameters, "(&s&sbb&s&sbu&s)", &local_appid, &local_port, &local_trusted, &bi_dir, &remote_appid, &remote_port, &remote_trusted, &len, &raw); @@ -605,19 +532,15 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc goto out; } - message_port_lock_mutex(); if (!is_local_port_registed(remote_port, remote_trusted, &local_reg_id, &mi)) { _LOGE("Invalid argument : remote_port:(%s) trusted(%d)", remote_port, remote_trusted); - message_port_unlock_mutex(); goto out; } callback_info = __create_callback_info(mi, local_appid); if (callback_info == NULL) { - message_port_unlock_mutex(); goto out; } - message_port_unlock_mutex(); if (!local_port) { _LOGE("Invalid argument : local_port"); @@ -693,37 +616,23 @@ static bool __receive_message(GVariant *parameters, GDBusMethodInvocation *invoc goto out; } - key_info = __create_callback_key_info(callback_info); - if (key_info == NULL) { - _LOGE("out of memory"); - ret = false; - goto out; - } - callback_info->g_src_id = g_io_add_watch_full( callback_info->gio_read, G_PRIORITY_DEFAULT, G_IO_IN | G_IO_HUP, - __socket_request_handler, - (gpointer)key_info, - __socket_destroy_handler); + __socket_request_handler, callback_info, + NULL); if (callback_info->g_src_id == 0) { _LOGE("fail to add watch on socket"); - free(key_info); ret = false; goto out; } - - message_port_lock_mutex(); - if (__callback_info_append(callback_info) == false) { - _LOGE("fail to append callback_info"); - ret = false; - } - message_port_unlock_mutex(); + __callback_info_append(callback_info); } } out: + message_port_unlock_mutex(); if (ret == false) __callback_info_free(callback_info); @@ -873,11 +782,34 @@ static const GDBusInterfaceVTable interface_vtable = { NULL }; -static int __register_dbus_interface(const char *port_name, bool is_trusted) +static bool __message_port_register_port(const int local_id, const char *local_port, bool is_trusted, message_port_message_cb callback, void *user_data) { + message_port_local_port_info_s *mi = (message_port_local_port_info_s *)calloc(1, sizeof(message_port_local_port_info_s)); + retvm_if(!mi, false, "Malloc failed"); + + mi->callback = callback; + mi->is_trusted = is_trusted; + mi->port_name = strdup(local_port); + if (mi->port_name == NULL) { +/* LCOV_EXCL_START */ + _LOGE("Malloc failed (%s)", local_port); + free(mi); + return false; +/* LCOV_EXCL_STOP */ + } + mi->local_id = local_id; + mi->user_data = user_data; + + g_hash_table_insert(__local_port_info, GINT_TO_POINTER(mi->local_id), mi); + return true; +} +static int __register_dbus_interface(const char *port_name, + bool is_trusted, message_port_message_cb callback, void *user_data) +{ GDBusNodeInfo *introspection_data = NULL; int registration_id = 0; + int ret = MESSAGE_PORT_ERROR_NONE; static gchar introspection_prefix[] = "" @@ -902,7 +834,6 @@ static int __register_dbus_interface(const char *port_name, bool is_trusted) char *introspection_xml = NULL; int introspection_xml_len = 0; - int owner_id = 0; GError *error = NULL; char *bus_name = NULL; @@ -927,6 +858,32 @@ static int __register_dbus_interface(const char *port_name, bool is_trusted) /* LCOV_EXCL_STOP */ } + snprintf(introspection_xml, introspection_xml_len, "%s%s%s", introspection_prefix, interface_name, introspection_postfix); + + introspection_data = g_dbus_node_info_new_for_xml(introspection_xml, NULL); + if (!introspection_data) { + ret = MESSAGE_PORT_ERROR_IO_ERROR; + _LOGE("g_dbus_node_info_new_for_xml() is failed."); + goto out; + } + + registration_id = g_dbus_connection_register_object(gdbus_conn, + MESSAGEPORT_OBJECT_PATH, introspection_data->interfaces[0], + &interface_vtable, NULL, NULL, NULL); + + _LOGD("registration_id %d", registration_id); + + if (registration_id == 0) { + ret = MESSAGE_PORT_ERROR_IO_ERROR; + _LOGE("Failed to g_dbus_connection_register_object"); + goto out; + } + + if (!__message_port_register_port(registration_id, port_name, is_trusted, + callback, user_data)) { + ret = MESSAGE_PORT_ERROR_OUT_OF_MEMORY; + goto out; + } result = g_dbus_connection_call_sync( gdbus_conn, @@ -942,39 +899,23 @@ static int __register_dbus_interface(const char *port_name, bool is_trusted) &error); if (error) { _LOGE("RequestName fail : %s", error->message); + ret = MESSAGE_PORT_ERROR_IO_ERROR; g_error_free(error); goto out; } if (result == NULL) { + ret = MESSAGE_PORT_ERROR_IO_ERROR; _LOGE("fail to get name NULL"); goto out; } g_variant_get(result, "(u)", &owner_id); if (owner_id == 0) { + ret = MESSAGE_PORT_ERROR_IO_ERROR; _LOGE("Acquiring the own name is failed"); goto out; } - _LOGD("Acquiring the own name : %d", owner_id); - - snprintf(introspection_xml, introspection_xml_len, "%s%s%s", introspection_prefix, interface_name, introspection_postfix); - - introspection_data = g_dbus_node_info_new_for_xml(introspection_xml, NULL); - if (!introspection_data) { - _LOGE("g_dbus_node_info_new_for_xml() is failed."); - goto out; - } - - registration_id = g_dbus_connection_register_object(gdbus_conn, - MESSAGEPORT_OBJECT_PATH, introspection_data->interfaces[0], - &interface_vtable, NULL, NULL, NULL); - - _LOGD("registration_id %d", registration_id); - - if (registration_id == 0) { - _LOGE("Failed to g_dbus_connection_register_object"); - goto out; - } + _LOGI("Acquiring the own name : %d", owner_id); out: if (introspection_data) @@ -986,30 +927,13 @@ out: if (result) g_variant_unref(result); - - return registration_id; -} - -static bool __message_port_register_port(const int local_id, const char *local_port, bool is_trusted, message_port_message_cb callback, void *user_data) -{ - message_port_local_port_info_s *mi = (message_port_local_port_info_s *)calloc(1, sizeof(message_port_local_port_info_s)); - retvm_if(!mi, false, "Malloc failed"); - - mi->callback = callback; - mi->is_trusted = is_trusted; - mi->port_name = strdup(local_port); - if (mi->port_name == NULL) { -/* LCOV_EXCL_START */ - _LOGE("Malloc failed (%s)", local_port); - free(mi); - return false; -/* LCOV_EXCL_STOP */ + if (ret != MESSAGE_PORT_ERROR_NONE) { + if (registration_id != 0) + g_hash_table_remove(__local_port_info, GINT_TO_POINTER(registration_id)); + registration_id = ret; } - mi->local_id = local_id; - mi->user_data = user_data; - g_hash_table_insert(__local_port_info, GINT_TO_POINTER(mi->local_id), mi); - return true; + return registration_id; } int get_local_port_info(int id, message_port_local_port_info_s **info) @@ -1042,15 +966,10 @@ int register_message_port(const char *local_port, bool is_trusted, message_port_ return local_id; } - local_id = __register_dbus_interface(local_port, is_trusted); + local_id = __register_dbus_interface(local_port, is_trusted, callback, user_data); if (local_id < 1) { _LOGE("register_dbus_interface fail !!"); - return MESSAGE_PORT_ERROR_OUT_OF_MEMORY; } - - if (!__message_port_register_port(local_id, local_port, is_trusted, callback, user_data)) - return MESSAGE_PORT_ERROR_OUT_OF_MEMORY; - return local_id; } @@ -1078,7 +997,7 @@ int unregister_local_port(int local_port_id, bool trusted_port) if (mi->is_trusted != trusted_port) return MESSAGE_PORT_ERROR_INVALID_PARAMETER; - g_hash_table_remove(__callback_info_hash, GUINT_TO_POINTER(local_port_id)); + __callback_info_free_by_local_id(local_port_id); bus_name = get_encoded_name(app_id, mi->port_name, mi->is_trusted); if (bus_name == NULL) diff --git a/test/unit_tests/test_message_port.cc b/test/unit_tests/test_message_port.cc index d347fa5..2e76c71 100644 --- a/test/unit_tests/test_message_port.cc +++ b/test/unit_tests/test_message_port.cc @@ -311,7 +311,8 @@ TEST_F(MessagePortTest, message_port_register_local_port3) { __port_id = 20; int port = message_port_register_local_port("PORT3", __message_cb, nullptr); EXPECT_EQ(port, __port_id); - __io_notify(__io_data); + if (__io_notify) + __io_notify(__io_data); } TEST_F(MessagePortTest, message_port_register_local_port_n) { @@ -391,7 +392,8 @@ TEST_F(MessagePortTest, message_port_send_message_with_local_port3) { "remote_app", "PORT", message.GetHandle(), 1); __call_sync_reply = nullptr; EXPECT_EQ(ret, MESSAGE_PORT_ERROR_NONE); - __io_notify(__io_data); + if (__io_notify) + __io_notify(__io_data); } TEST_F(MessagePortTest, message_port_register_trusted_local_port) {