From e012108280faac4aa05160f440db277c74189370 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Wed, 22 Apr 2015 18:58:59 +1000 Subject: [PATCH 1/4] Explicitly check for EPOLLRDHUP as well --- src/connector.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/connector.c b/src/connector.c index baad4c30..d3f68ac8 100644 --- a/src/connector.c +++ b/src/connector.c @@ -225,7 +225,7 @@ static int accept_client(cdata_t *cdata, const int epfd, const uint64_t server) client->fd = fd; event.data.ptr = client; - event.events = EPOLLIN; + 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); @@ -495,7 +495,7 @@ void *receiver(void *arg) client = ref_client_by_id(cdata, client->id); if (unlikely(!client)) continue; - if ((event.events & EPOLLERR) || (event.events & EPOLLHUP)) { + if (event.events & (EPOLLERR | EPOLLHUP | EPOLLRDHUP)) { /* Client disconnected */ LOGDEBUG("Client fd %d HUP in epoll", client->fd); invalidate_client(cdata->pi->ckp, cdata, client); From b50f2ba86c0d4173b7f8d179ae5177eb8379451a Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Wed, 22 Apr 2015 19:00:57 +1000 Subject: [PATCH 2/4] Check clients match in epoll loop --- src/connector.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/connector.c b/src/connector.c index d3f68ac8..60354661 100644 --- a/src/connector.c +++ b/src/connector.c @@ -492,8 +492,7 @@ void *receiver(void *arg) 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)) + if (unlikely(client != ref_client_by_id(cdata, client->id))) continue; if (event.events & (EPOLLERR | EPOLLHUP | EPOLLRDHUP)) { /* Client disconnected */ From 88fcb55faa302c8adcd6bdae3846f3374b72b470 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Wed, 22 Apr 2015 19:10:58 +1000 Subject: [PATCH 3/4] Revert "Check clients match in epoll loop" This reverts commit b50f2ba86c0d4173b7f8d179ae5177eb8379451a. Wrong --- src/connector.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/connector.c b/src/connector.c index 60354661..d3f68ac8 100644 --- a/src/connector.c +++ b/src/connector.c @@ -492,7 +492,8 @@ void *receiver(void *arg) client = event.data.ptr; /* Recheck this client still exists in the same form when it * was queued. */ - if (unlikely(client != ref_client_by_id(cdata, client->id))) + client = ref_client_by_id(cdata, client->id); + if (unlikely(!client)) continue; if (event.events & (EPOLLERR | EPOLLHUP | EPOLLRDHUP)) { /* Client disconnected */ From 75d24d1d07f29e3f5e6bb720684ad3b76366eb17 Mon Sep 17 00:00:00 2001 From: Con Kolivas Date: Wed, 22 Apr 2015 19:34:49 +1000 Subject: [PATCH 4/4] 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);