From 75d24d1d07f29e3f5e6bb720684ad3b76366eb17 Mon Sep 17 00:00:00 2001
From: Con Kolivas <kernel@kolivas.org>
Date: Wed, 22 Apr 2015 19:34:49 +1000
Subject: [PATCH] Reference clients in the epoll list by their client id to
 avoid double lookup and possible wrong client selection

---
 src/connector.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/src/connector.c b/src/connector.c
index d3f68ac8..221b808f 100644
--- a/src/connector.c
+++ b/src/connector.c
@@ -223,12 +223,17 @@ static int accept_client(cdata_t *cdata, const int epfd, const uint64_t server)
 	LOGINFO("Connected new client %d on socket %d to %d active clients from %s:%d",
 		cdata->nfds, fd, no_clients, client->address_name, port);
 
+	ck_wlock(&cdata->lock);
+	client->id = cdata->client_id++;
+	HASH_ADD_I64(cdata->clients, id, client);
+	cdata->nfds++;
+	ck_wunlock(&cdata->lock);
+
 	client->fd = fd;
-	event.data.ptr = client;
+	event.data.u64 = client->id;
 	event.events = EPOLLIN | EPOLLRDHUP;
 	if (unlikely(epoll_ctl(epfd, EPOLL_CTL_ADD, fd, &event) < 0)) {
 		LOGERR("Failed to epoll_ctl add in accept_client");
-		recycle_client(cdata, client);
 		return 0;
 	}
 
@@ -237,12 +242,6 @@ static int accept_client(cdata_t *cdata, const int epfd, const uint64_t server)
 	 * removes it automatically from the epoll list. */
 	__inc_instance_ref(client);
 
-	ck_wlock(&cdata->lock);
-	client->id = cdata->client_id++;
-	HASH_ADD_I64(cdata->clients, id, client);
-	cdata->nfds++;
-	ck_wunlock(&cdata->lock);
-
 	return 1;
 }
 
@@ -442,8 +441,7 @@ void *receiver(void *arg)
 	serverfds = cdata->ckp->serverurls;
 	/* Add all the serverfds to the epoll */
 	for (i = 0; i < serverfds; i++) {
-		/* The small values will be easily identifiable compared to
-		 * pointers */
+		/* The small values will be less than the first client ids */
 		event.data.u64 = i;
 		event.events = EPOLLIN;
 		ret = epoll_ctl(epfd, EPOLL_CTL_ADD, cdata->serverfd[i], &event);
@@ -489,12 +487,11 @@ void *receiver(void *arg)
 			}
 			continue;
 		}
-		client = event.data.ptr;
-		/* Recheck this client still exists in the same form when it
-		 * was queued. */
-		client = ref_client_by_id(cdata, client->id);
-		if (unlikely(!client))
+		client = ref_client_by_id(cdata, event.data.u64);
+		if (unlikely(!client)) {
+			LOGWARNING("Failed to find client by id in receiver!");
 			continue;
+		}
 		if (event.events & (EPOLLERR | EPOLLHUP | EPOLLRDHUP)) {
 			/* Client disconnected */
 			LOGDEBUG("Client fd %d HUP in epoll", client->fd);
@@ -970,7 +967,9 @@ int connector(proc_instance_t *pi)
 	cklock_init(&cdata->lock);
 	cdata->pi = pi;
 	cdata->nfds = 0;
-	cdata->client_id = 1;
+	/* Set the client id to the highest serverurl count to distinguish
+	 * them from the server fds in epoll. */
+	cdata->client_id = ckp->serverurls;
 	mutex_init(&cdata->sender_lock);
 	cond_init(&cdata->sender_cond);
 	create_pthread(&cdata->pth_sender, sender, cdata);