jsonrpc: call spdk_jsonrpc_free_request only from server thread

Decrementing struct spdk_jsonrpc_server_conn::outstanding_requests
should be atomic since this variable are accesed from multiple threads.
Istead of that just route the request back to the server thread with
nothing to send.

As we are here change spdk_jsonrpc_server_send_response() to take only
struct spdk_jsonrpc_request parameter.

Change-Id: I9b856e7d530355cea43a29f58f4f9405e7e35fc2
Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
Reviewed-on: https://review.gerrithub.io/422124
Chandler-Test-Pool: SPDK Automated Test System <sys_sgsw@intel.com>
Tested-by: SPDK CI Jenkins <sys_sgci@intel.com>
Reviewed-by: Ben Walker <benjamin.walker@intel.com>
Reviewed-by: Jim Harris <james.r.harris@intel.com>

lib/jsonrpc/jsonrpc_internal.h

@@ -99,11 +99,14 @@ void spdk_jsonrpc_server_handle_request(struct spdk_jsonrpc_request *request,
 					const struct spdk_json_val *method,
 					const struct spdk_json_val *params);
 void spdk_jsonrpc_server_handle_error(struct spdk_jsonrpc_request *request, int error);
-void spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_server_conn *conn,
-				       struct spdk_jsonrpc_request *request);
+
+/* Might be called from any thread */
+void spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_request *request);
 
 /* jsonrpc_server */
 int spdk_jsonrpc_parse_request(struct spdk_jsonrpc_server_conn *conn, void *json, size_t size);
+
+/* Must be called only from server poll thread */
 void spdk_jsonrpc_free_request(struct spdk_jsonrpc_request *request);
 
 #endif

lib/jsonrpc/jsonrpc_server.c

@@ -243,13 +243,20 @@ begin_response(struct spdk_jsonrpc_request *request)
 	return w;
 }
 
+static void
+skip_response(struct spdk_jsonrpc_request *request)
+{
+	request->send_len = 0;
+	spdk_jsonrpc_server_send_response(request);
+}
+
 static void
 end_response(struct spdk_jsonrpc_request *request, struct spdk_json_write_ctx *w)
 {
 	spdk_json_write_object_end(w);
 	spdk_json_write_end(w);
 	spdk_jsonrpc_server_write_cb(request, "\n", 1);
-	spdk_jsonrpc_server_send_response(request->conn, request);
+	spdk_jsonrpc_server_send_response(request);
 }
 
 void
@@ -267,13 +274,13 @@ spdk_jsonrpc_begin_result(struct spdk_jsonrpc_request *request)
 
 	if (request->id.type == SPDK_JSON_VAL_INVALID) {
 		/* Notification - no response required */
-		spdk_jsonrpc_free_request(request);
+		skip_response(request);
 		return NULL;
 	}
 
 	w = begin_response(request);
 	if (w == NULL) {
-		spdk_jsonrpc_free_request(request);
+		skip_response(request);
 		return NULL;
 	}
 
@@ -303,7 +310,7 @@ spdk_jsonrpc_send_error_response(struct spdk_jsonrpc_request *request,
 
 	w = begin_response(request);
 	if (w == NULL) {
-		free(request);
+		skip_response(request);
 		return;
 	}
 
@@ -332,7 +339,7 @@ spdk_jsonrpc_send_error_response_fmt(struct spdk_jsonrpc_request *request,
 
 	w = begin_response(request);
 	if (w == NULL) {
-		free(request);
+		skip_response(request);
 		return;
 	}
 

lib/jsonrpc/jsonrpc_server_tcp.c

@@ -259,9 +259,10 @@ spdk_jsonrpc_server_conn_recv(struct spdk_jsonrpc_server_conn *conn)
 }
 
 void
-spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_server_conn *conn,
-				  struct spdk_jsonrpc_request *request)
+spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_request *request)
 {
+	struct spdk_jsonrpc_server_conn *conn = request->conn;
+
 	/* Queue the response to be sent */
 	pthread_spin_lock(&conn->queue_lock);
 	STAILQ_INSERT_TAIL(&conn->send_queue, request, link);
@@ -296,9 +297,6 @@ more:
 
 	if (conn->send_request == NULL) {
 		conn->send_request = spdk_jsonrpc_server_dequeue_request(conn);
-		if (conn->send_request == NULL) {
-			return 0;
-		}
 	}
 
 	request = conn->send_request;
@@ -307,20 +305,22 @@ more:
 		return 0;
 	}
 
-	rc = send(conn->sockfd, request->send_buf + request->send_offset,
-		  request->send_len, 0);
-	if (rc < 0) {
-		if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
-			return 0;
+	if (request->send_len > 0) {
+		rc = send(conn->sockfd, request->send_buf + request->send_offset,
+			  request->send_len, 0);
+		if (rc < 0) {
+			if (errno == EAGAIN || errno == EWOULDBLOCK || errno == EINTR) {
+				return 0;
+			}
+
+			SPDK_DEBUGLOG(SPDK_LOG_RPC, "send() failed: %s\n", spdk_strerror(errno));
+			return -1;
 		}
 
-		SPDK_DEBUGLOG(SPDK_LOG_RPC, "send() failed: %s\n", spdk_strerror(errno));
-		return -1;
+		request->send_offset += rc;
+		request->send_len -= rc;
 	}
 
-	request->send_offset += rc;
-	request->send_len -= rc;
-
 	if (request->send_len == 0) {
 		/*
 		 * Full response has been sent.

test/unit/lib/jsonrpc/jsonrpc_server.c/jsonrpc_server_ut.c

@@ -219,8 +219,7 @@ spdk_jsonrpc_server_handle_request(struct spdk_jsonrpc_request *request,
 }
 
 void
-spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_server_conn *conn,
-				  struct spdk_jsonrpc_request *request)
+spdk_jsonrpc_server_send_response(struct spdk_jsonrpc_request *request)
 {
 	/* TODO */
 }