Commit 01a9118d authored by Pawel Wodkowski's avatar Pawel Wodkowski Committed by Daniel Verkamp
Browse files

jsonrpc: fix closed connection hadling 



The spdk_jsonrpc_server_conn_remove() was just swapping last connection
with that is being removed. This was fine but not for BSD queues which
rely on its own address.

Simple fix would be to add STAILQ_INIT and STAILQ_SWAP for queued
requests but we can hit the same nasty and easy to overlook bug in
future. Instead convert connection array to linked list and move around
only pointers.

Fixes #322

Change-Id: Ibb359d281f6164bcd17df37ba9d31ffdb46c2e0a
Signed-off-by: default avatarPawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-on: https://review.gerrithub.io/414257


Tested-by: default avatarSPDK Automated Test System <sys_sgsw@intel.com>
Reviewed-by: default avatarDariusz Stojaczyk <dariuszx.stojaczyk@intel.com>
Reviewed-by: default avatarDaniel Verkamp <daniel.verkamp@intel.com>
Reviewed-by: default avatarBen Walker <benjamin.walker@intel.com>
parent 967339f3

lib/jsonrpc/jsonrpc_internal.h

+7 −2
Original line number Diff line number Diff line
@@ -80,13 +80,18 @@ struct spdk_jsonrpc_server_conn { 
	STAILQ_HEAD(, spdk_jsonrpc_request) send_queue;



	struct spdk_jsonrpc_request *send_request;



	TAILQ_ENTRY(spdk_jsonrpc_server_conn) link;

};



struct spdk_jsonrpc_server {

	int sockfd;

	spdk_jsonrpc_handle_request_fn handle_request;

	struct spdk_jsonrpc_server_conn conns[SPDK_JSONRPC_MAX_CONNS];

	int num_conns;



	TAILQ_HEAD(, spdk_jsonrpc_server_conn) free_conns;

	TAILQ_HEAD(, spdk_jsonrpc_server_conn) conns;



	struct spdk_jsonrpc_server_conn conns_array[SPDK_JSONRPC_MAX_CONNS];

};



/* jsonrpc_server_tcp */

lib/jsonrpc/jsonrpc_server_tcp.c

+24 −23
Original line number Diff line number Diff line
@@ -40,13 +40,20 @@ spdk_jsonrpc_server_listen(int domain, int protocol, 
			   spdk_jsonrpc_handle_request_fn handle_request)

{

	struct spdk_jsonrpc_server *server;

	int rc, val, flag;

	int rc, val, flag, i;



	server = calloc(1, sizeof(struct spdk_jsonrpc_server));

	if (server == NULL) {

		return NULL;

	}



	TAILQ_INIT(&server->free_conns);

	TAILQ_INIT(&server->conns);



	for (i = 0; i < SPDK_JSONRPC_MAX_CONNS; i++) {

		TAILQ_INSERT_TAIL(&server->free_conns, &server->conns_array[i], link);

	}



	server->handle_request = handle_request;



	server->sockfd = socket(domain, SOCK_STREAM, protocol);
@@ -93,12 +100,12 @@ spdk_jsonrpc_server_listen(int domain, int protocol, 
void

spdk_jsonrpc_server_shutdown(struct spdk_jsonrpc_server *server)

{

	int i;

	struct spdk_jsonrpc_server_conn *conn;



	close(server->sockfd);



	for (i = 0; i < server->num_conns; i++) {

		close(server->conns[i].sockfd);

	TAILQ_FOREACH(conn, &server->conns, link) {

		close(conn->sockfd);

	}



	free(server);
@@ -119,29 +126,27 @@ static void 
spdk_jsonrpc_server_conn_remove(struct spdk_jsonrpc_server_conn *conn)

{

	struct spdk_jsonrpc_server *server = conn->server;

	int conn_idx = conn - server->conns;



	spdk_jsonrpc_server_conn_close(conn);



	pthread_spin_destroy(&conn->queue_lock);

	assert(STAILQ_EMPTY(&conn->send_queue));



	/* Swap conn with the last entry in conns */

	server->conns[conn_idx] = server->conns[server->num_conns - 1];

	server->num_conns--;

	TAILQ_REMOVE(&server->conns, conn, link);

	TAILQ_INSERT_HEAD(&server->free_conns, conn, link);

}



static int

spdk_jsonrpc_server_accept(struct spdk_jsonrpc_server *server)

{

	struct spdk_jsonrpc_server_conn *conn;

	int rc, conn_idx, flag;

	int rc, flag;



	rc = accept(server->sockfd, NULL, NULL);

	if (rc >= 0) {

		assert(server->num_conns < SPDK_JSONRPC_MAX_CONNS);

		conn_idx = server->num_conns;

		conn = &server->conns[conn_idx];

		conn = TAILQ_FIRST(&server->free_conns);

		assert(conn != NULL);



		conn->server = server;

		conn->sockfd = rc;

		conn->closed = false;
@@ -159,8 +164,8 @@ spdk_jsonrpc_server_accept(struct spdk_jsonrpc_server *server) 
			return -1;

		}



		server->num_conns++;



		TAILQ_REMOVE(&server->free_conns, conn, link);

		TAILQ_INSERT_TAIL(&server->conns, conn, link);

		return 0;

	}
@@ -332,12 +337,10 @@ more: 
int

spdk_jsonrpc_server_poll(struct spdk_jsonrpc_server *server)

{

	int rc, i;

	struct spdk_jsonrpc_server_conn *conn;



	for (i = 0; i < server->num_conns; i++) {

		conn = &server->conns[i];

	int rc;

	struct spdk_jsonrpc_server_conn *conn, *conn_tmp;



	TAILQ_FOREACH_SAFE(conn, &server->conns, link, conn_tmp) {

		if (conn->closed) {

			struct spdk_jsonrpc_request *request;
@@ -365,13 +368,11 @@ spdk_jsonrpc_server_poll(struct spdk_jsonrpc_server *server) 
	}



	/* Check listen socket */

	if (server->num_conns < SPDK_JSONRPC_MAX_CONNS) {

	if (!TAILQ_EMPTY(&server->free_conns)) {

		spdk_jsonrpc_server_accept(server);

	}



	for (i = 0; i < server->num_conns; i++) {

		conn = &server->conns[i];



	TAILQ_FOREACH(conn, &server->conns, link) {

		if (conn->closed) {

			continue;

		}