[sheepdog] [PATCH V2 2/2] sheep: free all requests when connection is dead

Yunkai Zhang yunkai.me at gmail.com
Fri Jul 6 10:55:23 CEST 2012


From: Yunkai Zhang <qiushu.zyk at taobao.com>

It's important to free all requests when connection is dead(especially when
EPOLLERR occur, some requests might be not cleared correctly), or it will lead
to memory leak and sys->nr_outstanding_reqs would be always larger than 0 as a
result sheep could not be shutdown.

Conneciont's request is created by alloc_request() and will be kept in four
places:
  1) ci->rx_req       -- after alloc_request()
  2) inflight         -- after reset ci->rx_req and queue_request()
  3) ci->dones_reqs   -- after put_request() added to ci->dones_reqs list
  4) ci->tx_req       -- after init_tx_hdr() and assigned to ci->tx_req
When request is inflight, ci->refcnt will always be larger than 0, but it will
be added to ci->dones_reqs after work_fn finished.

As long as we can promise that one request only be kept in one of these four
places, than we can free it correctly.

This patch do this clear work in clear_client().

BTW: add and update some log info

Signed-off-by: Yunkai Zhang <qiushu.zyk at taobao.com>
---
 sheep/sdnet.c |   27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/sheep/sdnet.c b/sheep/sdnet.c
index c9abc25..c49e971 100644
--- a/sheep/sdnet.c
+++ b/sheep/sdnet.c
@@ -540,6 +540,7 @@ static void client_rx_handler(struct client_info *ci)
 
 	if (is_conn_dead(conn) && ci->rx_req) {
 		free_request(ci->rx_req);
+		ci->rx_req = NULL;
 		return;
 	}
 
@@ -557,7 +558,8 @@ static void client_rx_handler(struct client_info *ci)
 	else
 		req->rp.data_length = hdr->data_length;
 
-	dprintf("connection from: %s:%d\n", ci->conn.ipstr, ci->conn.port);
+	dprintf("connection from: %d, %s:%d\n", ci->conn.fd,
+		ci->conn.ipstr, ci->conn.port);
 	queue_request(req);
 }
 
@@ -637,10 +639,13 @@ again:
 
 	if (is_conn_dead(&ci->conn)) {
 		free_request(ci->tx_req);
+		ci->tx_req = NULL;
 		return;
 	}
 
 	if (ci->conn.c_tx_state == C_IO_END) {
+		dprintf("connection from: %d, %s:%d\n", ci->conn.fd,
+			ci->conn.ipstr, ci->conn.port);
 		free_request(ci->tx_req);
 		ci->tx_req = NULL;
 		goto again;
@@ -656,6 +661,23 @@ static void destroy_client(struct client_info *ci)
 
 static void clear_client(struct client_info *ci)
 {
+	struct request *req;
+
+	if (ci->rx_req) {
+		free_request(ci->rx_req);
+		ci->rx_req = NULL;
+	}
+
+	if (ci->tx_req) {
+		free_request(ci->tx_req);
+		ci->tx_req = NULL;
+	}
+
+	list_for_each_entry(req, &ci->done_reqs, request_list) {
+		list_del(&req->request_list);
+		free_request(req);
+	}
+
 	if (!list_empty(&ci->conn.blocking_siblings))
 		list_del_init(&ci->conn.blocking_siblings);
 
@@ -736,8 +758,7 @@ static void client_handler(int fd, int events, void *data)
 
 	if (is_conn_dead(&ci->conn)) {
 err:
-		dprintf("closed connection %d, %s:%d\n", fd,
-			ci->conn.ipstr, ci->conn.port);
+		dprintf("connection seems to be dead\n");
 		clear_client(ci);
 	}
 }
-- 
1.7.10.4




More information about the sheepdog mailing list