From: Yunkai Zhang <[email protected]> 1) In previous code, sheep calls client_incref() in alloc_request(), but free_request() does not call client_desref() in it, as a result it's difficult to keep ci->refcnt with correct value. Now I drop client_incref/client_decref and call ci->refcnt++/ci->refcnt-- in alloc_request/free_request directly.
2) A bug in put_request(): before calling client_decref(), we should do some clear actions like client_handler(). 3) ci->refcnt should only be increased by alloc_request(), let's initialize it with 0 in create_client(). 4) remove error message in unregister_event() when lookup_event() failed, as this function may be called several times in new helper function which named clear_client() before ci->refcnt reach zero. Signed-off-by: Yunkai Zhang <[email protected]> --- lib/event.c | 4 +--- sheep/sdnet.c | 39 +++++++++++++++++++-------------------- 2 files changed, 20 insertions(+), 23 deletions(-) diff --git a/lib/event.c b/lib/event.c index 8a69dc4..1557493 100644 --- a/lib/event.c +++ b/lib/event.c @@ -125,10 +125,8 @@ void unregister_event(int fd) struct event_info *ei; ei = lookup_event(fd); - if (!ei) { - eprintf("event info for fd %d not found\n", fd); + if (!ei) return; - } ret = epoll_ctl(efd, EPOLL_CTL_DEL, fd, NULL); if (ret) diff --git a/sheep/sdnet.c b/sheep/sdnet.c index c13cdb0..bde2526 100644 --- a/sheep/sdnet.c +++ b/sheep/sdnet.c @@ -368,8 +368,7 @@ static void requeue_request(struct request *req) queue_request(req); } -static void client_incref(struct client_info *ci); -static void client_decref(struct client_info *ci); +static void clear_client(struct client_info *ci); static struct request *alloc_local_request(void *data, int data_length) { @@ -429,7 +428,7 @@ static struct request *alloc_request(struct client_info *ci, int data_length) return NULL; req->ci = ci; - client_incref(ci); + ci->refcnt++; if (data_length) { req->data_length = data_length; req->data = valloc(data_length); @@ -453,6 +452,7 @@ static void free_request(struct request *req) sys->nr_outstanding_reqs--; sys->outstanding_data_size -= req->data_length; + req->ci->refcnt--; put_vnode_info(req->vnodes); free(req->data); free(req); @@ -473,11 +473,10 @@ void put_request(struct request *req) if (conn_tx_on(&ci->conn)) { dprintf("connection seems to be dead\n"); free_request(req); + clear_client(ci); } else { list_add(&req->request_list, &ci->done_reqs); } - - client_decref(ci); } } @@ -653,16 +652,21 @@ static void destroy_client(struct client_info *ci) free(ci); } -static void client_incref(struct client_info *ci) +static void clear_client(struct client_info *ci) { - if (ci) - ci->refcnt++; -} + if (!list_empty(&ci->conn.blocking_siblings)) + list_del_init(&ci->conn.blocking_siblings); -static void client_decref(struct client_info *ci) -{ - if (ci && --ci->refcnt == 0) - destroy_client(ci); + unregister_event(ci->conn.fd); + + dprintf("refcnt:%d, fd:%d, %s:%d\n", + ci->refcnt, ci->conn.fd, + ci->conn.ipstr, ci->conn.port); + + if (ci->refcnt) + return; + + destroy_client(ci); } static struct client_info *create_client(int fd, struct cluster_info *cluster) @@ -693,7 +697,7 @@ static struct client_info *create_client(int fd, struct cluster_info *cluster) ci->conn.fd = fd; ci->conn.events = EPOLLIN; - ci->refcnt = 1; + ci->refcnt = 0; INIT_LIST_HEAD(&ci->done_reqs); INIT_LIST_HEAD(&ci->conn.blocking_siblings); @@ -720,12 +724,7 @@ static void client_handler(int fd, int events, void *data) err: dprintf("closed connection %d, %s:%d\n", fd, ci->conn.ipstr, ci->conn.port); - - if (!list_empty(&ci->conn.blocking_siblings)) - list_del_init(&ci->conn.blocking_siblings); - - unregister_event(fd); - client_decref(ci); + clear_client(ci); } } -- 1.7.10.4 -- sheepdog mailing list [email protected] http://lists.wpkg.org/mailman/listinfo/sheepdog
