Draft: Resolve "server has to remove ungracefully disconnected zombie clients"

README.md

@@ -45,6 +45,13 @@ To run the internal tests run
 
     $ ctest
 
+### Tests
+The testsuite from [googletest](https://github.com/google/googletest) is used. To run tests, call
+
+    $ cd build
+    $ make run_test
+
+
 ## Demo
 You can clone the [demo project](https://git01.iis.fhg.de/ks-ip-lib/software/libjapi-demo), with examples for all features from the repository listed below:
 

include/japi.h

@@ -53,6 +53,15 @@ typedef struct __japi_context {
 	void *userptr; /*!< Pointer to user data */
 	uint16_t num_clients; /*!< Number of connected clients */
 	uint16_t max_clients; /*!< Number of maximal allowed clients */
+	int tcp_keepalive_enable; /*!< Switch to enable keepalive mechanism in TCP server.
+								Configure using following attributes */
+	int tcp_keepalive_time; /*!< The number of seconds a connection needs to be idle
+							  before TCP begins sending out keep-alive probes. */
+	int tcp_keepalive_intvl; /*!< The number of seconds between TCP keep-alive probes.
+							  */
+	int tcp_keepalive_probes; /*!< The maximum number of TCP keep-alive probes to send
+								before giving up and killing the connection if no
+								response is ob‐ tained from the other end. */
 	pthread_mutex_t lock; /*!< Mutual access lock */
 	struct __japi_request *requests; /*!< Pointer to the JAPI request list */
 	struct __japi_pushsrv_context
@@ -173,6 +182,28 @@ int japi_set_max_allowed_clients(japi_context *ctx, uint16_t num);
  */
 int japi_include_args_in_response(japi_context *ctx, bool include_args);
 
+/*!
+ * \brief Enable or disable TCP keepalive and set the relevant parameters.
+ *
+ * Default values depend on the system. E.g. for Debian 11 `tcp_keepalive_time
+ * = 7200`, `tcp_keepalive_intvl = 75`, `tcp_keepalive_probes = 9`
+ *
+ * \param ctx					JAPI context
+ * \param tcp_keepalive_enable	Switch to enable keepalive mechanism in TCP
+ * 								server. Configure using following attributes
+ * \param tcp_keepalive_time	The number of seconds a connection needs to be
+ * 								idle before TCP begins sending out keep-alive probes.
+ * \param tcp_keepalive_intvl	The number of seconds between TCP keep-alive probes.
+ * \param tcp_keepalive_probes	The maximum number of TCP keep-alive probes to send
+ * 								before giving up and killing the connection if no
+ * 								response is obtained from the other end.
+ *
+ * \returns On success, zero is returned. On invalid parameters, -1 is returned.
+ */
+int japi_set_tcp_keepalive(japi_context *ctx, int tcp_keepalive_enable,
+						   int tcp_keepalive_time, int tcp_keepalive_intvl,
+						   int tcp_keepalive_probes);
+
 /*!
  * \brief Shutdown the JAPI server
  *

include/networking.h

@@ -77,6 +77,30 @@ int tcp4_start_server(const char* port);
  */
 int tcp6_start_server(const char* port);
 
+/*!
+ * \brief Set socket options to enable TCP keepalive functions
+ * 
+ * Change default values to ensure that lost connections are recognised and
+ * terminated.
+ * 
+ * \param socket_file_descriptor Socket to which the changes are applied.
+ * \param tcp_keepalive_enable	Switch to enable keepalive mechanism in TCP 
+ * 								server. Configure using following attributes
+ * \param tcp_keepalive_time	The number of seconds a connection needs to be
+ * 								idle before TCP begins sending out keep-alive probes.
+ * \param tcp_keepalive_intvl	The number of seconds between TCP keep-alive probes.
+ * \param tcp_keepalive_probes	The maximum number of TCP keep-alive probes to send
+ * 								before giving up and killing the connection if no
+ * 								response is obtained from the other end.
+ * 
+ * \returns On success 0. On error -1.
+ */
+int enable_tcp_keepalive(int socket_file_descriptor,
+                        int tcp_keepalive_enable,
+                        int tcp_keepalive_time,
+                        int tcp_keepalive_intvl,
+                        int tcp_keepalive_probes);
+
 #ifdef __cplusplus
 }
 #endif

src/japi.c

@@ -311,6 +311,10 @@ japi_context *japi_init(void *userptr)
 	ctx->num_clients = 0;
 	ctx->max_clients = 0;
 	ctx->include_args_in_response = false;
+	ctx->tcp_keepalive_enable = 0;
+	ctx->tcp_keepalive_time = 7200;
+	ctx->tcp_keepalive_intvl = 75;
+	ctx->tcp_keepalive_probes = 9;
 	ctx->shutdown = false;
 
 	/* Initialize mutex */
@@ -370,6 +374,37 @@ int japi_include_args_in_response(japi_context *ctx, bool include_args)
 	return 0;
 }
 
+/* change defaults from tcp_keepalive_time = 7200, tcp_keepalive_intvl = 75,
+ * tcp_keepalive_probes = 9 */
+int japi_set_tcp_keepalive(japi_context *ctx, int tcp_keepalive_enable,
+						   int tcp_keepalive_time, int tcp_keepalive_intvl,
+						   int tcp_keepalive_probes)
+{
+	/* Error handling */
+	if (ctx == NULL) {
+		fprintf(stderr, "ERROR: JAPI context is NULL.\n");
+		return -1;
+	}
+	if (tcp_keepalive_time < 0) {
+		fprintf(stderr, "ERROR: tcp_keepalive_time has to be positive or 0.\n");
+		return -1;
+	}
+	if (tcp_keepalive_intvl <= 0) {
+		fprintf(stderr, "ERROR: tcp_keepalive_intvl has to be positive.\n");
+		return -1;
+	}
+	if (tcp_keepalive_probes < 0) {
+		fprintf(stderr, "ERROR: tcp_keepalive_probes has to be positive or 0.\n");
+		return -1;
+	}
+
+	ctx->tcp_keepalive_enable = tcp_keepalive_enable;
+	ctx->tcp_keepalive_time = tcp_keepalive_time;
+	ctx->tcp_keepalive_intvl = tcp_keepalive_intvl;
+	ctx->tcp_keepalive_probes = tcp_keepalive_probes;
+	return 0;
+}
+
 /*
  * Add new client element to list
  */
@@ -497,6 +532,14 @@ int japi_start_server(japi_context *ctx, const char *port)
 		return -1;
 	}
 
+	if (ctx->tcp_keepalive_enable) {
+		if (enable_tcp_keepalive(server_socket, ctx->tcp_keepalive_enable,
+								 ctx->tcp_keepalive_time, ctx->tcp_keepalive_intvl,
+								 ctx->tcp_keepalive_probes) < 0) {
+			return -1;
+		}
+	}
+
 	int ret;
 	int nfds;
 	fd_set fdrd;
@@ -615,6 +658,9 @@ int japi_start_server(japi_context *ctx, const char *port)
 				perror("ERROR: accept() failed\n");
 				return -1;
 			}
+
+			prntdbg("Client socket to add: %d, current number of clients %d\n", client_socket, ctx->num_clients);
+
 			if (ctx->max_clients == 0 || ctx->num_clients < ctx->max_clients) {
 				japi_add_client(ctx, client_socket);
 				prntdbg("client %d added\n", client_socket);

src/networking.c

@@ -31,6 +31,7 @@
  * THE SOFTWARE.
  */
 
+#include <netinet/tcp.h>
 #include <stdio.h>
 #include <string.h>
 #include <sys/types.h>
@@ -116,3 +117,40 @@ int tcp6_start_server(const char* port)
 	return tcp_start_server_on_addr_family(port, AF_INET6);
 }
 
+int enable_tcp_keepalive(int socket_file_descriptor,
+                        int tcp_keepalive_enable,
+                        int tcp_keepalive_time,
+                        int tcp_keepalive_intvl,
+                        int tcp_keepalive_probes)
+{
+	if (setsockopt(socket_file_descriptor, SOL_SOCKET, SO_KEEPALIVE, &tcp_keepalive_enable, sizeof(tcp_keepalive_enable)) == -1)
+	{
+		perror("ERROR: setsocktop SO_KEEPALIVE");
+		fprintf(stderr, "WARNING: Keeping connection alive wont be possible. \n");
+		/* The programm can go on. */
+	};
+
+	if (!tcp_keepalive_enable)
+	{
+		return 0;
+	}
+
+	if (setsockopt(socket_file_descriptor, IPPROTO_TCP, TCP_KEEPIDLE, &tcp_keepalive_time, sizeof(tcp_keepalive_time)) == -1)
+	{
+		perror("setsockopt TCP_KEEPIDLE");
+		return -1;
+	}
+
+	if (setsockopt(socket_file_descriptor, IPPROTO_TCP, TCP_KEEPINTVL, &tcp_keepalive_intvl, sizeof(tcp_keepalive_intvl)) == -1)
+	{
+		perror("setsockopt TCP_KEEPINTVL");
+		return -1;
+	}
+
+	if (setsockopt(socket_file_descriptor, IPPROTO_TCP, TCP_KEEPCNT, &tcp_keepalive_probes, sizeof(tcp_keepalive_probes)) == -1)
+	{
+		perror("setsockopt TCP_KEEPCNT");
+		return -1;
+	}
+	return 0;
+}

test/japi_test.cc

@@ -21,15 +21,19 @@ Copyright (c) 2023 Fraunhofer IIS
  */
 
 #include <gtest/gtest.h>
+#include <netinet/in.h>
+#include <netinet/tcp.h>
 #include <stdbool.h>
+#include <sys/socket.h>
 
 extern "C" {
-#include "japi.h"
-#include "japi_intern.h"
-#include "japi_pushsrv.h"
-#include "japi_pushsrv_intern.h"
-#include "japi_utils.h"
-#include "rw_n.h"
+#include <japi.h>
+#include <japi_intern.h>
+#include <japi_pushsrv.h>
+#include <japi_pushsrv_intern.h>
+#include <japi_utils.h>
+#include <networking.h>
+#include <rw_n.h>
 }
 
 /* The handler for japi_register_request test */
@@ -551,3 +555,158 @@ TEST(JAPI_Push_Service, PushServiceRemoveEntryFromLInkedList)
 	EXPECT_STREQ(json_object_to_json_string(jobj),
 				 "{ \"services\": [ \"test04\", \"test03\" ] }");
 }
+
+TEST(JAPI, TcpKeepAliveSetup)
+{
+	japi_context *ctx;
+
+	ctx = japi_init(NULL);
+
+	/* Activate keepalive in socket. Test different parameters */
+	EXPECT_EQ(japi_set_tcp_keepalive(ctx, 1, 60, 10, 6), 0);
+	EXPECT_EQ(japi_set_tcp_keepalive(ctx, 0, 60, 10, 6), 0);
+	EXPECT_EQ(japi_set_tcp_keepalive(ctx, 1, 60, 0.999, 6),
+			  -1); // tcp_keepalive_intvl > 0, implicit type conversion
+	EXPECT_EQ(japi_set_tcp_keepalive(ctx, 1, -60, 10, 0), -1); // positive values only
+
+	/* Test if options are set correctly in context*/
+	ASSERT_EQ(japi_set_tcp_keepalive(ctx, 1, 60, 10, 6), 0);
+	EXPECT_EQ(ctx->tcp_keepalive_enable, 1);
+	EXPECT_EQ(ctx->tcp_keepalive_time, 60);
+	EXPECT_EQ(ctx->tcp_keepalive_intvl, 10);
+	EXPECT_EQ(ctx->tcp_keepalive_probes, 6);
+
+	/* Test if sockoptions are actually set */
+	int opt_val;
+	socklen_t opt_len = sizeof(opt_val);
+	int server_socket = tcp_start_server("1234");
+	ASSERT_EQ(enable_tcp_keepalive(server_socket, ctx->tcp_keepalive_enable,
+								   ctx->tcp_keepalive_time, ctx->tcp_keepalive_intvl,
+								   ctx->tcp_keepalive_probes),
+			  0);
+
+	ASSERT_EQ(getsockopt(server_socket, SOL_SOCKET, SO_KEEPALIVE, &opt_val, &opt_len),
+			  0);
+	EXPECT_EQ(opt_val, 1);
+
+	ASSERT_EQ(getsockopt(server_socket, IPPROTO_TCP, TCP_KEEPIDLE, &opt_val, &opt_len),
+			  0);
+	EXPECT_EQ(opt_val, 60);
+
+	ASSERT_EQ(getsockopt(server_socket, IPPROTO_TCP, TCP_KEEPINTVL, &opt_val, &opt_len),
+			  0);
+	EXPECT_EQ(opt_val, 10);
+
+	ASSERT_EQ(getsockopt(server_socket, IPPROTO_TCP, TCP_KEEPCNT, &opt_val, &opt_len),
+			  0);
+	EXPECT_EQ(opt_val, 6);
+
+	close(server_socket);
+}
+
+static void *serverThread(void *arg)
+{
+	japi_context *ctx = (japi_context*) arg;
+
+	japi_start_server(ctx, "1234");
+
+	pthread_exit(0);
+}
+
+TEST(JAPI, JAPI_TcpKeepAliveFunctionality)
+{
+	/* Testing TCP-Keep-Alive functionality: Set max number of clients to 1, set
+	 * keep-alive settings and setup a JAPI server. First client is going to connect,
+	 * send data disconnects and disconnects ungracefully. When the TCP keep-alive works
+	 * the ungraceful disconnected client will be closed after the configured interval.
+	 * Client 2 should then be able to connect and send data (Will not be possible when
+	 * client1 socket still is alive since max number of clients is restricted to 1).
+	 * */
+	japi_context *ctx;
+
+	ctx = japi_init(NULL);
+
+	/* Remove any ungracefull disconnected client socket after
+	 * 1s + 2 * 1s = 4s timeout. */
+	ASSERT_EQ(japi_set_tcp_keepalive(ctx, 1, 1, 1, 2), 0);
+
+	/* Allow only one client at a time. */
+	ASSERT_EQ(japi_set_max_allowed_clients(ctx, 1), 0);
+
+	pthread_t tid;
+	pthread_create(&tid, NULL, serverThread, (void*) ctx);
+
+	/* Wait for the server to start. */
+	/* TODO: Use mutex */
+	sleep(2);
+
+	// Connect the client socket to the server
+	struct sockaddr_in serverAddr = {0};
+	serverAddr.sin_family = AF_INET;
+	serverAddr.sin_port = htons(1234);
+	serverAddr.sin_addr.s_addr = htonl(INADDR_ANY);
+
+	int clientSock1, clientSock2;
+	const char *message =  "{'japi_request': 'japi_cmd_list'}\n";
+	ssize_t num_bytes_sent, numBytes_recv;
+	char buf[1024];
+
+	/***** CLIENT 1 *****/
+	// Create a client socket and send request
+	clientSock1 = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_NE(clientSock1, -1) << "Failed to create client socket1";
+	ASSERT_EQ(connect(clientSock1, (struct sockaddr *)&serverAddr,
+	sizeof(serverAddr)), 0) << "Failed to connect client socket1";
+	num_bytes_sent = send(clientSock1, message, strlen(message), 0);
+	ASSERT_EQ(num_bytes_sent, strlen(message)) << "Message was not fully sent";
+
+	// Receive reponse of server
+	memset(buf, 0, sizeof(buf));
+	numBytes_recv = recv(clientSock1, buf, sizeof(buf), 0);
+	ASSERT_NE(numBytes_recv, -1) << "Receive message failed";
+	ASSERT_NE(numBytes_recv, 0) << "Received message is empty";
+
+	/* Close the first client socket ungracefully.
+	 * TODO: Find a way to do this and replace close(). */
+	close(clientSock1);
+
+	/********************/
+
+	/* Wait for the keep-Alive mechanism to kill client 1 socket. */
+	sleep(5);
+
+	/***** CLIENT 2 *****/
+
+	/* Connecting a second client to the server with succesfull send and receive requires the frist client to
+	 * to be remove properly. If the TCP-KEEP alive mechanism was not working properly, the first client will
+	 * be still alive after the waiting time and following receive of client 2 will signal an empty message
+	 * */
+	// Create a client socket and send request
+	clientSock2 = socket(AF_INET, SOCK_STREAM, 0);
+	ASSERT_NE(clientSock2, -1) << "Failed to create client socket2";
+	ASSERT_EQ(connect(clientSock2, (struct sockaddr *)&serverAddr,
+	sizeof(serverAddr)), 0) << "Failed to connect client socket2";
+	num_bytes_sent = send(clientSock2, message, strlen(message), 0);
+	ASSERT_EQ(num_bytes_sent, strlen(message)) << "Message was not fully sent";
+
+	// Receive reponse of server
+	memset(buf, 0, sizeof(buf));
+	numBytes_recv = recv(clientSock2, buf, sizeof(buf), 0);
+	ASSERT_NE(numBytes_recv, -1) << "Receive message failed";
+	ASSERT_NE(numBytes_recv, 0) << "Received message is empty";
+
+	close(clientSock2);
+
+	/********************/
+
+	/* Wait for debug messages just in case server is faster with closing. */
+	sleep(1);
+
+	/* Signal shutdown to server. */
+	ctx->shutdown = true;
+
+	/* Wait for server to end. */
+	pthread_join(tid, NULL);
+	japi_destroy(ctx);
+	return;
+}