monitoring (4)

Author: splitice

.github/workflows/build-test.yml

@@ -27,4 +27,26 @@ jobs:
         valgrind --leak-check=full ./tests/${{ matrix.release }}/tests $PWD/src/server/${{ matrix.release }}/scache $PWD/testcases/
     - name: Spam test
       run: |
-        echo "todo"
+        echo "todo"
+    - name: Valgrind monitoring test
+      run: |
+        sudo mkdir -p /var/lib/scache/ &
+        sudo apt-get install psmisc
+        wait
+        sudo chmod 0777 /var/lib/scache/ -R
+
+        function query {
+          sleep 2
+          for i in {1..50}
+          do
+            curl 127.0.0.1:8082/conn &
+          done
+
+          sleep 110
+
+          killall curl
+        }
+
+        query &
+
+        timeout 2m valgrind --leak-check=full --show-leak-kinds=all src/server/${{ matrix.release }}/scache -B 127.0.0.1:8082

src/core/connection.cpp

@@ -282,8 +282,10 @@ static scache_connection* connection_get(int fd) {
 }
 
 bool connection_remove(int fd) {
+	scache_connection_node* node;
 	scache_connection_node* temp = NULL;
-	scache_connection_node* node = &ctable[CONNECTION_HASH_KEY(fd)];
+	assert(fd >= 0);
+	node = &ctable[CONNECTION_HASH_KEY(fd)];
 	if (node->connection.client_sock == -1) {
 		WARN("Unable to find fd: %d connection entry to remove", fd);
 		return false;
@@ -639,12 +641,14 @@ On close connection cleanup routine
 */
 void connection_cleanup_http(scache_connection_node* connection, bool toFree = false) {
 	assert(connection != NULL);
+	int fd;
 
 	//Close socket to client
 	if (connection->connection.client_sock != -1) {
 		http_cleanup(&connection->connection);
-		close(connection->connection.client_sock);
+		fd = connection->connection.client_sock;
 		connection->connection.client_sock = -1;
+		close(fd);
 	}
 
 	//Handle chained connections

src/core/db.cpp

@@ -694,10 +694,12 @@ static bool db_load_from_save(){
 close_fd:
 	fclose(fp);
 close_fd2:
-	close(db.fd_blockfile);
+	if(fp == NULL) close(fd);
+
+	fd = db.fd_blockfile;
 	db.fd_blockfile = -1;
+	close(fd);
 
-	if(fp == NULL) close(fd);
 
 	return ret;
 }

src/core/http_parse_mon.cpp

@@ -78,7 +78,8 @@ state_action http_respond_writecount_starting(scache_connection* connection) {
 static state_action http_respond_start_to_count(scache_connection* connection) {
 	DEBUG("[#%d] Ready to start to count \n", connection->client_sock);
 	monitoring_add(connection);
-	return http_respond_cleanupafterwrite(connection);
+	CONNECTION_HANDLER(connection,  http_respond_cleanupafterwrite);
+	return continue_processing;
 }
 
 state_action http_mon_read_eol_inital(scache_connection* connection, char* buffer, int n, uint32_t& temporary) {
@@ -275,6 +276,7 @@ void monitoring_add(scache_connection* conn){
 	if(mon_tail == NULL){
 		mon_tail = mon_head = conn;
 	}else{
+		// insert at tail
 		assert(mon_tail->monitoring.next == NULL);
 		mon_tail->monitoring.next = conn;
 		conn->monitoring.prev = mon_tail;
@@ -286,26 +288,41 @@ void monitoring_destroy(scache_connection* connection){
 	assert(connection->ltype == mon_listener);
 	scache_connection* t;
 
+	/*
+	head ... | connection | t | ... tail
+	*/
 	t = connection->monitoring.next;
 	if(t != NULL){
 		assert(t->monitoring.prev == connection);
 		t->monitoring.prev = connection->monitoring.prev;
 		if(t->monitoring.prev == NULL) {
-			assert(mon_tail == connection);
-			mon_tail = t;
+			assert(mon_head == connection);
+			mon_head = t;
 		}
+	}else{
+		// Only if we are destroying the tail t will be NULL
+		assert(connection == mon_tail);
+		mon_tail = t;
 	}
 
+	/*
+	head ... | t | connection | ... tail
+	*/
 	t = connection->monitoring.prev;
 	if(t != NULL){
 		assert(t->monitoring.next == connection);
 		t->monitoring.next = connection->monitoring.next;
 		if(t->monitoring.next == NULL) {
-			assert(mon_head == connection);
-			mon_head = t;
+			assert(mon_tail == connection);
+			mon_tail = t;
 		}
+	}else{
+		// Only if we are destroying the head will t be null
+		assert(mon_head == connection);
+		mon_head = t;
 	}
 }
+
 static void reverse(char s[])
 {
 	int i, j;
@@ -357,6 +374,7 @@ void monitoring_check(){
 			mon_tail = mon_head = conn;
 			conn->monitoring.next = conn->monitoring.prev = NULL;
 		}
+		conn->monitoring.next = NULL;
 
 		// If buffer wasnt cleared already, then we will need to disconnect
 		if(conn->output_buffer != NULL){