From 21abe76b01a6649b6c9d57d631de6020452030f7 Mon Sep 17 00:00:00 2001 From: Sung-jae Park Date: Thu, 20 Mar 2014 17:35:55 +0900 Subject: Validate context before destroy it. dlist_remove_data always find the data before delete the item. This patch will find the item first like dlist_remove_data. but if there is no item in the list, do not release it. just skip the destroy function call. Change-Id: Ia15fa5c7dc3f283a19e7b5109c3c5796623b55d2 --- packaging/libcom-core.spec | 2 +- src/com-core.c | 58 +++++++++++++++++++++++++++++++++++----------- src/com-core_packet.c | 58 +++++++++++++++++++++++++++++++++++++--------- 3 files changed, 92 insertions(+), 26 deletions(-) diff --git a/packaging/libcom-core.spec b/packaging/libcom-core.spec index 754b0e7..8e27d57 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.5.8 +Version: 0.5.9 Release: 1 Group: Base/IPC License: Apache-2.0 diff --git a/src/com-core.c b/src/com-core.c index 4cbb44b..dfbb309 100644 --- a/src/com-core.c +++ b/src/com-core.c @@ -38,9 +38,15 @@ static struct { struct dlist *conn_cb_list; struct dlist *disconn_cb_list; + enum processing_event_callback { + PROCESSING_NONE = 0x0, + PROCESSING_DISCONNECTION = 0x01, + PROCESSING_CONNECTION = 0x02, + } processing_event_callback; } s_info = { .conn_cb_list = NULL, .disconn_cb_list = NULL, + .processing_event_callback = PROCESSING_NONE, }; struct cbdata { @@ -49,6 +55,7 @@ struct cbdata { }; struct evtdata { + int deleted; int (*evt_cb)(int fd, void *data); void *data; }; @@ -59,14 +66,19 @@ HAPI void invoke_con_cb_list(int handle) struct dlist *n; struct evtdata *cbdata; + s_info.processing_event_callback |= PROCESSING_CONNECTION; dlist_foreach_safe(s_info.conn_cb_list, l, n, cbdata) { - if (cbdata->evt_cb(handle, cbdata->data) < 0) { - if (dlist_find_data(s_info.conn_cb_list, cbdata)) { - s_info.conn_cb_list = dlist_remove(s_info.conn_cb_list, l); - free(cbdata); - } + /*! + * \NOTE + * cbdata->deleted must has to be checked before call the function and + * return from the function call. + */ + if (cbdata->deleted || cbdata->evt_cb(handle, cbdata->data) < 0 || cbdata->deleted) { + s_info.conn_cb_list = dlist_remove(s_info.conn_cb_list, l); + free(cbdata); } } + s_info.processing_event_callback &= ~PROCESSING_CONNECTION; } HAPI void invoke_disconn_cb_list(int handle) @@ -75,14 +87,19 @@ HAPI void invoke_disconn_cb_list(int handle) struct dlist *n; struct evtdata *cbdata; + s_info.processing_event_callback |= PROCESSING_DISCONNECTION; dlist_foreach_safe(s_info.disconn_cb_list, l, n, cbdata) { - if (cbdata->evt_cb(handle, cbdata->data) < 0) { - if (dlist_find_data(s_info.disconn_cb_list, cbdata)) { - s_info.disconn_cb_list = dlist_remove(s_info.disconn_cb_list, l); - free(cbdata); - } + /*! + * \NOTE + * cbdata->deleted must has to be checked before call the function and + * return from the function call. + */ + if (cbdata->deleted || cbdata->evt_cb(handle, cbdata->data) < 0 || cbdata->deleted) { + s_info.disconn_cb_list = dlist_remove(s_info.disconn_cb_list, l); + free(cbdata); } } + s_info.processing_event_callback &= ~PROCESSING_DISCONNECTION; } static gboolean client_cb(GIOChannel *src, GIOCondition cond, gpointer data) @@ -302,6 +319,7 @@ EAPI int com_core_client_create(const char *addr, int is_sync, int (*service_cb) } g_io_channel_unref(gio); + invoke_con_cb_list(client_fd); return client_fd; } @@ -317,6 +335,7 @@ EAPI int com_core_add_event_callback(enum com_core_event_type type, int (*evt_cb cbdata->evt_cb = evt_cb; cbdata->data = data; + cbdata->deleted = 0; if (type == CONNECTOR_CONNECTED) { s_info.conn_cb_list = dlist_append(s_info.conn_cb_list, cbdata); @@ -465,8 +484,14 @@ EAPI void *com_core_del_event_callback(enum com_core_event_type type, int (*cb)( if (cbdata->evt_cb == cb && cbdata->data == data) { void *data; data = cbdata->data; - dlist_remove_data(s_info.conn_cb_list, cbdata); - free(cbdata); + + if ((s_info.processing_event_callback & PROCESSING_CONNECTION) == PROCESSING_CONNECTION) { + cbdata->deleted = 1; + } else { + dlist_remove_data(s_info.conn_cb_list, cbdata); + free(cbdata); + } + return data; } } @@ -475,8 +500,13 @@ EAPI void *com_core_del_event_callback(enum com_core_event_type type, int (*cb)( if (cbdata->evt_cb == cb && cbdata->data == data) { void *data; data = cbdata->data; - dlist_remove_data(s_info.disconn_cb_list, cbdata); - free(cbdata); + + if ((s_info.processing_event_callback & PROCESSING_DISCONNECTION) == PROCESSING_DISCONNECTION) { + cbdata->deleted = 1; + } else { + dlist_remove_data(s_info.disconn_cb_list, cbdata); + free(cbdata); + } return data; } } diff --git a/src/com-core_packet.c b/src/com-core_packet.c index 16844bb..75744fc 100644 --- a/src/com-core_packet.c +++ b/src/com-core_packet.c @@ -77,7 +77,7 @@ struct request_ctx { int (*recv_cb)(pid_t pid, int handle, const struct packet *packet, void *data); void *data; - int in_recv; + int inuse; }; struct recv_ctx { @@ -92,6 +92,8 @@ struct recv_ctx { pid_t pid; struct packet *packet; double timeout; + + int inuse; }; static inline struct request_ctx *find_request_ctx(int handle, double seq) @@ -110,8 +112,20 @@ static inline struct request_ctx *find_request_ctx(int handle, double seq) static inline void destroy_request_ctx(struct request_ctx *ctx) { + struct dlist *l; + + if (ctx->inuse) { + return; + } + + l = dlist_find_data(s_info.request_list, ctx); + if (!l) { + return; + } + + s_info.request_list = dlist_remove(s_info.request_list, l); + packet_unref(ctx->packet); - dlist_remove_data(s_info.request_list, ctx); free(ctx); } @@ -130,7 +144,7 @@ static inline struct request_ctx *create_request_ctx(int handle) ctx->packet = NULL; ctx->recv_cb = NULL; ctx->data = NULL; - ctx->in_recv = 0; + ctx->inuse = 0; s_info.request_list = dlist_append(s_info.request_list, ctx); return ctx; @@ -152,7 +166,19 @@ static inline struct recv_ctx *find_recv_ctx(int handle) static inline void destroy_recv_ctx(struct recv_ctx *ctx) { - dlist_remove_data(s_info.recv_list, ctx); + struct dlist *l; + + if (ctx->inuse) { + return; + } + + l = dlist_find_data(s_info.recv_list, ctx); + if (!l) { + return; + } + + s_info.recv_list = dlist_remove(s_info.recv_list, l); + packet_destroy(ctx->packet); free(ctx); } @@ -173,12 +199,13 @@ static inline struct recv_ctx *create_recv_ctx(int handle, double timeout) ctx->handle = handle; ctx->pid = (pid_t)-1; ctx->timeout = timeout; + ctx->inuse = 0; s_info.recv_list = dlist_append(s_info.recv_list, ctx); return ctx; } -static inline int packet_ready(int handle, const struct recv_ctx *receive, struct method *table) +static inline int packet_ready(int handle, struct recv_ctx *receive, struct method *table) { struct request_ctx *request; double sequence; @@ -198,9 +225,11 @@ static inline int packet_ready(int handle, const struct recv_ctx *receive, struc } if (request->recv_cb) { - request->in_recv = 1; + request->inuse = 1; + receive->inuse = 1; request->recv_cb(receive->pid, handle, receive->packet, request->data); - request->in_recv = 0; + receive->inuse = 0; + request->inuse = 0; } destroy_request_ctx(request); @@ -211,7 +240,9 @@ static inline int packet_ready(int handle, const struct recv_ctx *receive, struc continue; } + receive->inuse = 1; result = table[i].handler(receive->pid, handle, receive->packet); + receive->inuse = 0; if (result) { ret = s_info.vtable.send(handle, (void *)packet_data(result), packet_size(result), DEFAULT_TIMEOUT); if (ret < 0) { @@ -221,6 +252,7 @@ static inline int packet_ready(int handle, const struct recv_ctx *receive, struc } packet_destroy(result); } + break; } @@ -231,10 +263,14 @@ static inline int packet_ready(int handle, const struct recv_ctx *receive, struc continue; } + receive->inuse = 1; result = table[i].handler(receive->pid, handle, receive->packet); + receive->inuse = 0; if (result) { packet_destroy(result); } + + break; } break; default: @@ -253,8 +289,8 @@ static int client_disconnected_cb(int handle, void *data) struct request_ctx *request; struct dlist *l; struct dlist *n; + int inuse_found = 0; pid_t pid = (pid_t)-1; - int referred = 0; receive = find_recv_ctx(handle); if (receive) { @@ -268,8 +304,8 @@ static int client_disconnected_cb(int handle, void *data) continue; } - if (request->in_recv) { - referred = 1; + if (request->inuse) { + inuse_found = 1; continue; } @@ -280,7 +316,7 @@ static int client_disconnected_cb(int handle, void *data) destroy_request_ctx(request); } - if (receive && !referred) { + if (receive && !inuse_found) { destroy_recv_ctx(receive); } -- cgit v1.2.3 From 91ba80b04e61e8e689b9757cd337c4c5c8fc3c6a Mon Sep 17 00:00:00 2001 From: Sung-jae Park Date: Thu, 20 Mar 2014 22:00:32 +0900 Subject: Optimize the perf / validate FD. Reduce the malloc for handling the receive context. Validate socket FD. Change-Id: Ia492e7d0a23dd233f6227676aa2beee94d106a6c --- src/com-core.c | 20 ++++++++++++++++++-- src/com-core_packet.c | 19 ++++++++++++++++++- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/com-core.c b/src/com-core.c index dfbb309..d254014 100644 --- a/src/com-core.c +++ b/src/com-core.c @@ -23,6 +23,7 @@ #include #include #include +#include #include @@ -102,6 +103,20 @@ HAPI void invoke_disconn_cb_list(int handle) s_info.processing_event_callback &= ~PROCESSING_DISCONNECTION; } +static int validate_handle(int fd) +{ + int error; + socklen_t len; + + len = sizeof(error); + if (getsockopt(fd, SOL_SOCKET, SO_ERROR, &error, &len) < 0) { + ErrPrint("getsockopt: %s\n", strerror(errno)); + return 0; + } + + return !(error == EBADF); +} + static gboolean client_cb(GIOChannel *src, GIOCondition cond, gpointer data) { int client_fd; @@ -132,7 +147,8 @@ static gboolean client_cb(GIOChannel *src, GIOCondition cond, gpointer data) return FALSE; } - return TRUE; + /* Check whether the socket FD is closed or not */ + return validate_handle(client_fd) ? TRUE : FALSE; } static gboolean accept_cb(GIOChannel *src, GIOCondition cond, gpointer cbdata) @@ -200,7 +216,7 @@ static gboolean accept_cb(GIOChannel *src, GIOCondition cond, gpointer cbdata) g_io_channel_unref(gio); invoke_con_cb_list(client_fd); - return TRUE; + return validate_handle(socket_fd) ? TRUE : FALSE; } EAPI int com_core_server_create(const char *addr, int is_sync, int (*service_cb)(int fd, void *data), void *data) diff --git a/src/com-core_packet.c b/src/com-core_packet.c index 75744fc..6c0146d 100644 --- a/src/com-core_packet.c +++ b/src/com-core_packet.c @@ -164,6 +164,20 @@ static inline struct recv_ctx *find_recv_ctx(int handle) return NULL; } +static inline void recreate_recv_ctx(struct recv_ctx *ctx) +{ + if (ctx->packet) { + packet_destroy(ctx->packet); + ctx->packet = NULL; + } + ctx->state = RECV_STATE_INIT; + ctx->offset = 0; + ctx->pid = (pid_t)-1; + // ctx->inuse + // ctx->handle + // ctx->timeout +} + static inline void destroy_recv_ctx(struct recv_ctx *ctx) { struct dlist *l; @@ -448,7 +462,10 @@ static int service_cb(int handle, void *data) if (receive->state == RECV_STATE_READY) { ret = packet_ready(handle, receive, data); if (ret == 0) { - destroy_recv_ctx(receive); + /*! + * If ret is negative value, the receive context will be destroyed from disconnected callback + */ + recreate_recv_ctx(receive); } /*! * if ret is negative value, disconnected_cb will be called after this function -- cgit v1.2.3