From 2c2e2201c3691934d69ff4fbf7094295359d10d8 Mon Sep 17 00:00:00 2001 From: Sung-jae Park Date: Thu, 13 Jun 2013 09:27:46 +0900 Subject: [PATCH] Update the terminating sequence. [model] Tizen [binary_type] AP [customer] Tizen Developer [issue#] N/A [problem] Hangs while terminating thread base com-core. [cause] When the socket descriptor is used in a thread via "select", main thread should not close it. [solution] Use the event-pipe to escape from select. [team] HomeTF [request] [horizontal_expansion] Change-Id: Ied7a8eccb4fc7ccb1ea180c1c79bed73d800087e --- include/debug.h | 16 +++++++++-- include/util.h | 14 ++++++++++ packaging/libcom-core.spec | 2 +- src/com-core.c | 2 ++ src/com-core_packet-router.c | 65 ++++++++------------------------------------ src/com-core_thread.c | 57 ++++++++++++++++++++++++++++---------- src/secure_socket.c | 12 ++++---- 7 files changed, 90 insertions(+), 78 deletions(-) diff --git a/include/debug.h b/include/debug.h index 1a06d11..d629ed5 100644 --- a/include/debug.h +++ b/include/debug.h @@ -15,9 +15,21 @@ * */ +#if !defined(SECURE_LOGD) +#define SECURE_LOGD LOGD +#endif + +#if !defined(SECURE_LOGE) +#define SECURE_LOGE LOGE +#endif + +#if !defined(SECURE_LOGW) +#define SECURE_LOGW LOGW +#endif + #if !defined(FLOG) -#define DbgPrint(format, arg...) LOGD("[%s/%s:%d] " format, util_basename(__FILE__), __func__, __LINE__, ##arg) -#define ErrPrint(format, arg...) LOGE("[%s/%s:%d] " format, util_basename(__FILE__), __func__, __LINE__, ##arg) +#define DbgPrint(format, arg...) SECURE_LOGD("[%s/%s:%d] " format, util_basename(__FILE__), __func__, __LINE__, ##arg) +#define ErrPrint(format, arg...) SECURE_LOGE("[%s/%s:%d] " format, util_basename(__FILE__), __func__, __LINE__, ##arg) #else extern FILE *__file_log_fp; #define DbgPrint(format, arg...) do { fprintf(__file_log_fp, "[LOG] [%s/%s:%d] " format, util_basename(__FILE__), __func__, __LINE__, ##arg); fflush(__file_log_fp); } while (0) diff --git a/include/util.h b/include/util.h index a0ef886..e3e45c1 100644 --- a/include/util.h +++ b/include/util.h @@ -33,4 +33,18 @@ do { \ ErrPrint("Failed to unlock: %s\n", strerror(ret)); \ } while (0) +#define CLOSE_PIPE(p) do { \ + int status; \ + status = close(p[PIPE_READ]); \ + if (status < 0) \ + ErrPrint("close: %s\n", strerror(errno)); \ + status = close(p[PIPE_WRITE]); \ + if (status < 0) \ + ErrPrint("close: %s\n", strerror(errno)); \ +} while (0) + +#define PIPE_READ 0 +#define PIPE_WRITE 1 +#define PIPE_MAX 2 + /* End of a file */ diff --git a/packaging/libcom-core.spec b/packaging/libcom-core.spec index a12a6e6..9507b1d 100644 --- a/packaging/libcom-core.spec +++ b/packaging/libcom-core.spec @@ -1,6 +1,6 @@ Name: libcom-core Summary: Library for the light-weight IPC -Version: 0.3.15 +Version: 0.4.1 Release: 1 Group: HomeTF/Framework License: Apache License diff --git a/src/com-core.c b/src/com-core.c index fea5ce1..7aa172e 100644 --- a/src/com-core.c +++ b/src/com-core.c @@ -483,6 +483,7 @@ EAPI void *com_core_del_event_callback(enum com_core_event_type type, int (*cb)( EAPI int com_core_server_destroy(int handle) { DbgPrint("Close server handle[%d]\n", handle); + invoke_disconn_cb_list(handle); secure_socket_destroy_handle(handle); return 0; } @@ -490,6 +491,7 @@ EAPI int com_core_server_destroy(int handle) EAPI int com_core_client_destroy(int handle) { DbgPrint("Close client handle[%d]\n", handle); + invoke_disconn_cb_list(handle); secure_socket_destroy_handle(handle); return 0; } diff --git a/src/com-core_packet-router.c b/src/com-core_packet-router.c index 362355c..8002eca 100644 --- a/src/com-core_packet-router.c +++ b/src/com-core_packet-router.c @@ -37,9 +37,6 @@ #include "util.h" #include "com-core_packet-router.h" -#define PIPE_READ 0 -#define PIPE_WRITE 1 - struct packet_item { pid_t pid; struct packet *packet; @@ -100,8 +97,8 @@ struct router { pthread_mutex_t send_packet_list_lock; struct dlist *send_packet_list; - int recv_pipe[2]; - int send_pipe[2]; + int recv_pipe[PIPE_MAX]; + int send_pipe[PIPE_MAX]; pthread_t send_thid; @@ -673,11 +670,7 @@ static struct router *create_router(const char *sock, int handle, struct method ErrPrint("pipe2: %s\n", strerror(errno)); free(router->sock); - if (close(router->recv_pipe[0]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->recv_pipe[1]) < 0) - ErrPrint("close: %s\n", strerror(errno)); + CLOSE_PIPE(router->recv_pipe); ret = pthread_mutex_destroy(&router->send_packet_list_lock); if (ret != 0) @@ -701,17 +694,8 @@ static struct router *create_router(const char *sock, int handle, struct method gio = g_io_channel_unix_new(router->recv_pipe[PIPE_READ]); if (!gio) { - if (close(router->recv_pipe[PIPE_READ]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->recv_pipe[PIPE_WRITE]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_READ]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_WRITE]) < 0) - ErrPrint("close: %s\n", strerror(errno)); + CLOSE_PIPE(router->recv_pipe); + CLOSE_PIPE(router->send_pipe); free(router->sock); @@ -742,17 +726,8 @@ static struct router *create_router(const char *sock, int handle, struct method } g_io_channel_unref(gio); - if (close(router->recv_pipe[PIPE_READ]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->recv_pipe[PIPE_WRITE]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_READ]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_WRITE]) < 0) - ErrPrint("close: %s\n", strerror(errno)); + CLOSE_PIPE(router->recv_pipe); + CLOSE_PIPE(router->send_pipe); free(router->sock); @@ -783,17 +758,8 @@ static struct router *create_router(const char *sock, int handle, struct method g_source_remove(router->id); - if (close(router->recv_pipe[PIPE_READ]) < 0) - ErrPrint("Close: %s\n", strerror(errno)); - - if (close(router->recv_pipe[PIPE_WRITE]) < 0) - ErrPrint("Close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_READ]) < 0) - ErrPrint("Close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_WRITE]) < 0) - ErrPrint("Close: %s\n", strerror(errno)); + CLOSE_PIPE(router->recv_pipe); + CLOSE_PIPE(router->send_pipe); free(router->sock); @@ -839,17 +805,8 @@ static inline __attribute__((always_inline)) int destroy_router(struct router *r if (router->id > 0) g_source_remove(router->id); - if (close(router->recv_pipe[PIPE_READ]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->recv_pipe[PIPE_WRITE]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_READ]) < 0) - ErrPrint("close: %s\n", strerror(errno)); - - if (close(router->send_pipe[PIPE_WRITE]) < 0) - ErrPrint("close: %s\n", strerror(errno)); + CLOSE_PIPE(router->recv_pipe); + CLOSE_PIPE(router->send_pipe); free(router->sock); diff --git a/src/com-core_thread.c b/src/com-core_thread.c index 0b3172b..4f0b12b 100644 --- a/src/com-core_thread.c +++ b/src/com-core_thread.c @@ -37,8 +37,6 @@ #include "util.h" int errno; -#define PIPE_READ 0 -#define PIPE_WRITE 1 #define EVENT_READY 'a' #define EVENT_TERM 'e' @@ -78,7 +76,8 @@ struct tcb { pthread_t thid; int handle; struct dlist *chunk_list; - int evt_pipe[2]; + int evt_pipe[PIPE_MAX]; + int ctrl_pipe[PIPE_MAX]; pthread_mutex_t chunk_lock; guint id; /*!< g_io_watch */ @@ -149,6 +148,9 @@ static inline void terminate_thread(struct tcb *tcb) void *res = NULL; struct chunk *chunk; + if (write(tcb->ctrl_pipe[PIPE_WRITE], &tcb, sizeof(tcb)) != sizeof(tcb)) + ErrPrint("Unable to write CTRL pipe\n"); + secure_socket_destroy_handle(tcb->handle); status = pthread_join(tcb->thid, &res); @@ -300,6 +302,7 @@ static void *client_cb(void *data) fd_set set; int readsize; char event_ch; + int fd; DbgPrint("Thread is created for %d (server: %d)\n", tcb->handle, tcb->server_handle); /*! @@ -309,8 +312,11 @@ static void *client_cb(void *data) while (1) { FD_ZERO(&set); FD_SET(tcb->handle, &set); + FD_SET(tcb->ctrl_pipe[PIPE_READ], &set); + + fd = tcb->handle > tcb->ctrl_pipe[PIPE_READ] ? tcb->handle : tcb->ctrl_pipe[PIPE_READ]; - ret = select(tcb->handle + 1, &set, NULL, NULL, NULL); + ret = select(fd + 1, &set, NULL, NULL, NULL); if (ret < 0) { if (errno == EINTR) { DbgPrint("Select receives INTR\n"); @@ -325,6 +331,12 @@ static void *client_cb(void *data) continue; } + if (FD_ISSET(tcb->ctrl_pipe[PIPE_READ], &set)) { + DbgPrint("Thread is canceled\n"); + ret = -ECANCELED; + break; + } + if (!FD_ISSET(tcb->handle, &set)) { ErrPrint("Unexpected handle is toggled\n"); ret = -EINVAL; @@ -397,11 +409,8 @@ static inline void tcb_destroy(struct tcb *tcb) if (tcb->id > 0) g_source_remove(tcb->id); - if (tcb->evt_pipe[PIPE_WRITE] > 0) - close(tcb->evt_pipe[PIPE_WRITE]); - - if (tcb->evt_pipe[PIPE_READ] > 0) - close(tcb->evt_pipe[PIPE_READ]); + CLOSE_PIPE(tcb->evt_pipe); + CLOSE_PIPE(tcb->ctrl_pipe); status = pthread_mutex_destroy(&tcb->chunk_lock); if (status != 0) @@ -490,7 +499,20 @@ static inline struct tcb *tcb_create(int client_fd, int is_sync, int (*service_c return NULL; } - DbgPrint("[%d] New TCB created: %d, %d\n", client_fd, tcb->evt_pipe[0], tcb->evt_pipe[1]); + if (pipe2(tcb->ctrl_pipe, O_NONBLOCK | O_CLOEXEC) < 0) { + ErrPrint("Error: %s\n", strerror(errno)); + + CLOSE_PIPE(tcb->evt_pipe); + + status = pthread_mutex_destroy(&tcb->chunk_lock); + if (status != 0) + ErrPrint("Error: %s\n", strerror(status)); + + free(tcb); + return NULL; + } + + DbgPrint("[%d] New TCB created: %d, %d\n", client_fd, tcb->evt_pipe[PIPE_READ], tcb->evt_pipe[PIPE_WRITE]); return tcb; } @@ -899,18 +921,22 @@ EAPI int com_core_thread_server_destroy(int handle) if (tcb->server_handle != handle) continue; + invoke_disconn_cb_list(handle); terminate_thread(tcb); tcb_destroy(tcb); + return 0; } dlist_foreach_safe(s_info.server_list, l, n, server) { - if (server->handle == handle) { - server_destroy(server); - break; - } + if (server->handle != handle) + continue; + + invoke_disconn_cb_list(handle); + server_destroy(server); + return 0; } - return 0; + return -ENOENT; } /*! @@ -925,6 +951,7 @@ EAPI int com_core_thread_client_destroy(int handle) if (!tcb) return -ENOENT; + invoke_disconn_cb_list(handle); terminate_thread(tcb); tcb_destroy(tcb); return 0; diff --git a/src/secure_socket.c b/src/secure_socket.c index a69a7b3..8c6c93e 100644 --- a/src/secure_socket.c +++ b/src/secure_socket.c @@ -81,7 +81,7 @@ EAPI int secure_socket_create_client(const char *peer) ErrPrint("Failed to connect to server [%s] %s\n", peer, strerror(errno)); if (close(handle) < 0) - ErrPrint("close a handle: %s\n", strerror(errno)); + ErrPrint("close: %s\n", strerror(errno)); return -ENOTCONN; } @@ -89,7 +89,7 @@ EAPI int secure_socket_create_client(const char *peer) if (setsockopt(handle, SOL_SOCKET, SO_PASSCRED, &on, sizeof(on)) < 0) { ErrPrint("Failed to change sock opt : %s\n", strerror(errno)); if (close(handle) < 0) - ErrPrint("close a handle: %s\n", strerror(errno)); + ErrPrint("close: %s\n", strerror(errno)); return -EFAULT; } @@ -112,7 +112,7 @@ EAPI int secure_socket_create_server(const char *peer) ErrPrint("Failed to bind a socket %s\n", strerror(errno)); if (close(handle) < 0) - ErrPrint("Close a handle : %s\n", strerror(errno)); + ErrPrint("close: %s\n", strerror(errno)); return state; } @@ -123,7 +123,7 @@ EAPI int secure_socket_create_server(const char *peer) ErrPrint("Failed to listen a socket %s\n", strerror(errno)); if (close(handle) < 0) - ErrPrint("Close a handle : %s\n", strerror(errno)); + ErrPrint("close: %s\n", strerror(errno)); return state; } @@ -154,7 +154,7 @@ EAPI int secure_socket_get_connection_handle(int server_handle) ret = -errno; ErrPrint("Failed to change sock opt : %s\n", strerror(errno)); if (close(handle) < 0) - ErrPrint("Close a handle: %s\n", strerror(errno)); + ErrPrint("close: %s\n", strerror(errno)); return ret; } @@ -248,7 +248,7 @@ EAPI int secure_socket_destroy_handle(int handle) { DbgPrint("Close socket handle %d\n", handle); if (close(handle) < 0) { - ErrPrint("Failed to close a handle: %s\n", strerror(errno)); + ErrPrint("close: %s\n", strerror(errno)); return -1; } return 0; -- 2.7.4