From 17c006a7c33f8af0032fa02be90641a110de8993 Mon Sep 17 00:00:00 2001 From: yidong0635 Date: Tue, 18 Jun 2019 14:36:05 -0400 Subject: [PATCH] lib/jsonrpc: Fix memory leaks about connection request. There're outstanding requests in spdk_jsonrpc_parse_request which caused by connection close. There are methods to call spdk_jsonrpc_server_conn_close, including spdk_jsonrpc_server_conn_remove and spdk_jsonrpc_server_shutdown, Some rpc methods call these functions to terminate connections ,that leads to memory leaks. Try to free outstanding requests after deciding to terminate a connection. And do this follwing with close(conn->sockfd). Fix issue #784, and can resolve other similar memory leaks about this. Signed-off-by: yidong0635 Change-Id: Icd287bd0c5670ee8ec32750b999f82b0fa89cf84 Reviewed-on: https://review.gerrithub.io/c/spdk/spdk/+/458438 Tested-by: SPDK CI Jenkins Reviewed-by: Changpeng Liu Reviewed-by: Darek Stojaczyk --- lib/jsonrpc/jsonrpc_server_tcp.c | 64 +++++++++++++++----------------- 1 file changed, 29 insertions(+), 35 deletions(-) diff --git a/lib/jsonrpc/jsonrpc_server_tcp.c b/lib/jsonrpc/jsonrpc_server_tcp.c index 70b7a56b9..f3d870755 100644 --- a/lib/jsonrpc/jsonrpc_server_tcp.c +++ b/lib/jsonrpc/jsonrpc_server_tcp.c @@ -98,12 +98,39 @@ spdk_jsonrpc_server_listen(int domain, int protocol, return server; } +static struct spdk_jsonrpc_request * +spdk_jsonrpc_server_dequeue_request(struct spdk_jsonrpc_server_conn *conn) +{ + struct spdk_jsonrpc_request *request = NULL; + + pthread_spin_lock(&conn->queue_lock); + request = STAILQ_FIRST(&conn->send_queue); + if (request) { + STAILQ_REMOVE_HEAD(&conn->send_queue, link); + } + pthread_spin_unlock(&conn->queue_lock); + return request; +} + +static void +spdk_jsonrpc_server_free_conn_request(struct spdk_jsonrpc_server_conn *conn) +{ + struct spdk_jsonrpc_request *request; + + spdk_jsonrpc_free_request(conn->send_request); + conn->send_request = NULL ; + while ((request = spdk_jsonrpc_server_dequeue_request(conn)) != NULL) { + spdk_jsonrpc_free_request(request); + } +} + static void spdk_jsonrpc_server_conn_close(struct spdk_jsonrpc_server_conn *conn) { conn->closed = true; if (conn->sockfd >= 0) { + spdk_jsonrpc_server_free_conn_request(conn); close(conn->sockfd); conn->sockfd = -1; @@ -315,20 +342,6 @@ spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_request *request) pthread_spin_unlock(&conn->queue_lock); } -static struct spdk_jsonrpc_request * -spdk_jsonrpc_server_dequeue_request(struct spdk_jsonrpc_server_conn *conn) -{ - struct spdk_jsonrpc_request *request = NULL; - - pthread_spin_lock(&conn->queue_lock); - request = STAILQ_FIRST(&conn->send_queue); - if (request) { - STAILQ_REMOVE_HEAD(&conn->send_queue, link); - } - pthread_spin_unlock(&conn->queue_lock); - return request; -} - static int spdk_jsonrpc_server_conn_send(struct spdk_jsonrpc_server_conn *conn) @@ -392,27 +405,8 @@ spdk_jsonrpc_server_poll(struct spdk_jsonrpc_server *server) spdk_jsonrpc_server_conn_close(conn); } - if (conn->sockfd == -1) { - struct spdk_jsonrpc_request *request; - - /* - * The client closed the connection, but there may still be requests - * outstanding; we have no way to cancel outstanding requests, so wait until - * each outstanding request sends a response (which will be discarded, since - * the connection is closed). - */ - - spdk_jsonrpc_free_request(conn->send_request); - conn->send_request = NULL; - - while ((request = spdk_jsonrpc_server_dequeue_request(conn)) != NULL) { - spdk_jsonrpc_free_request(request); - } - - if (conn->outstanding_requests == 0) { - SPDK_DEBUGLOG(SPDK_LOG_RPC, "all outstanding requests completed\n"); - spdk_jsonrpc_server_conn_remove(conn); - } + if (conn->sockfd == -1 && conn->outstanding_requests == 0) { + spdk_jsonrpc_server_conn_remove(conn); } }