Validate socket FD & optimize recv context.tizen_3.0.m14.2_ivi_releasesubmit/tizen_ivi_panda/20140403.011837submit/tizen/20140430.015826accepted/tizen/mobile/20140702.100351accepted/tizen/ivi/panda/20140403.015123accepted/tizen/ivi/20140430.194310accepted/tizen/common/20140506.092610tizen_ivi_pandatizen_3.0.m14.2_iviaccepted/tizen_ivi_panda

From the recv ctx callback, the socket can be closed.
But the glib cannot detect the closed FD correctly.
(It doesn't toggles G_IO_HUP, G_IO_ERR, G_IO_NVAL bits even though I closed the FD)
So I checked the FD from the callback explicitly, before return from it.
However this patch will be available only for the Single-Thread loop. (com-core)

And the receive context is optimized.
Every single loop after done to prepare A receive context,
it will be destroyed. But this patch will reuse it instead of destroying it.
So there is no need to reallocate same memory again.
It will save few milli (or micro) seconds.

for the future-work.
I'll optimize the "packet" creating code.
It can be reused too after finish the processing of receive context.
Then we can save more few milli(micro) seconds (for reallocating time).

Each execution takes small time.
But this scenario occupies almost time for communicating with other processes.
it means, even though it can save few milli(micro) secs for each time.
The total saved time will be meaningful.

Change-Id: Ibb3adc715090b6d78a5cfeb48d28463f35b9a0f4

3 files changed, 128 insertions, 29 deletions

diff --git a/packaging/libcom-core.spec b/packaging/libcom-core.spec
index 29ec9c9..df338f3 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.7
+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..d254014 100644
--- a/src/com-core.c
+++ b/src/com-core.c
@@ -23,6 +23,7 @@
 #include <stdlib.h>
 #include <sys/time.h>
 #include <sys/types.h>
+#include <sys/socket.h>
 
 #include <glib.h>
 
@@ -38,9 +39,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 +56,7 @@ struct cbdata {
 };
 
 struct evtdata {
+	int deleted;
 	int (*evt_cb)(int fd, void *data);
 	void *data;
 };
@@ -59,14 +67,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 +88,33 @@ 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 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)
@@ -115,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)
@@ -183,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)
@@ -302,6 +335,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 +351,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 +500,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 +516,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..6c0146d 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;
@@ -150,9 +164,35 @@ 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)
 {
-	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 +213,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 +239,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 +254,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 +266,7 @@ static inline int packet_ready(int handle, const struct recv_ctx *receive, struc
 				}
 				packet_destroy(result);
 			}
+
 			break;
 		}
 
@@ -231,10 +277,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 +303,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 +318,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 +330,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);
 	}
 
@@ -412,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