From 6326f6d0d33019f5af7b31fb3a8026a7a20f30c9 Mon Sep 17 00:00:00 2001
From: "Rob Swindell (on Windows 11)" <rob@synchro.net>
Date: Tue, 13 Feb 2024 23:35:55 -0800
Subject: [PATCH] Set javascript callback "terminated" flag to true when
 recycling

(or terminating) the server.

This will allow background JS threads to terminate when recycling, so the
server doesn't just hang indefinitelyi when recycling.

Add more logging in cleanup() when waiting for children threads to terminate.

Also, eliminate the global 'terminate' variable, answering the question:
  Can this be changed to a if(ws_set!=NULL) check instead?

Yes, yes it can.
---
 src/sbbs3/websrvr.c | 21 ++++++++-------------
 1 file changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/sbbs3/websrvr.c b/src/sbbs3/websrvr.c
index 1310f670fd..8792229412 100644
--- a/src/sbbs3/websrvr.c
+++ b/src/sbbs3/websrvr.c
@@ -108,7 +108,7 @@ static protected_uint32_t active_clients;
 static protected_uint32_t thread_count;
 static volatile ulong	client_highwater=0;
 static volatile bool	terminate_server=false;
-static volatile bool	terminated=false;
+static volatile bool	terminate_js=false;
 static volatile bool	terminate_http_logging_thread=false;
 static struct xpms_set	*ws_set=NULL;
 static char		root_dir[MAX_PATH+1];
@@ -6899,11 +6899,13 @@ static void cleanup(int code)
 	if(protected_uint32_value(thread_count) > 1) {
 		lprintf(LOG_INFO,"0000 Waiting for %d child threads to terminate", protected_uint32_value(thread_count)-1);
 		while(protected_uint32_value(thread_count) > 1) {
-			mswait(100);
+			mswait(1000);
 			listSemPost(&log_list);
+			lprintf(LOG_INFO,"0000 Waiting for %d child threads to terminate", protected_uint32_value(thread_count)-1);
 		}
 		lprintf(LOG_INFO,"0000 Done waiting");
 	}
+	terminate_js = false;
 	free_cfg(&scfg);
 
 	listFree(&log_list);
@@ -6920,10 +6922,9 @@ static void cleanup(int code)
 	semfile_list_free(&recycle_semfiles);
 	semfile_list_free(&shutdown_semfiles);
 
-	if(!terminated) {	/* Can this be changed to a if(ws_set!=NULL) check instead? */
+	if(ws_set!=NULL) {
 		xpms_destroy(ws_set, close_socket_cb, NULL);
 		ws_set=NULL;
-		terminated=true;
 	}
 
 	update_clients();	/* active_clients is destroyed below */
@@ -7257,7 +7258,6 @@ void web_server(void* arg)
 			cleanup(1);
 			return;
 		}
-		terminated=false;
 		lprintf(LOG_DEBUG,"Web Server socket set created");
 
 		/*
@@ -7302,7 +7302,7 @@ void web_server(void* arg)
 		lprintf(LOG_INFO,"Web Server thread started");
 		mqtt_client_max(&mqtt, startup->max_clients);
 
-		while(!terminated && !terminate_server) {
+		while(!terminate_server) {
 			YIELD();
 			/* check for re-cycle/shutdown/pause semaphores */
 			if(!(startup->options&BBS_OPT_NO_RECYCLE)) {
@@ -7371,12 +7371,6 @@ void web_server(void* arg)
 			if(client_socket == INVALID_SOCKET)
 				continue;
 
-			if(terminated) {	/* terminated */
-				pthread_mutex_unlock(&session->struct_filled);
-				session=NULL;
-				break;
-			}
-
 			if(startup->socket_open!=NULL)
 				startup->socket_open(startup->cbdata,true);
 
@@ -7429,7 +7423,7 @@ void web_server(void* arg)
 			session->addr_len=client_addr_len;
    			session->socket=client_socket;
 			session->js_callback.auto_terminate=true;
-			session->js_callback.terminated=&terminate_server;
+			session->js_callback.terminated=&terminate_js;
 			session->js_callback.limit=startup->js.time_limit;
 			session->js_callback.gc_interval=startup->js.gc_interval;
 			session->js_callback.yield_interval=startup->js.yield_interval;
@@ -7446,6 +7440,7 @@ void web_server(void* arg)
 			session=NULL;
 		}
 
+		terminate_js = true;
 		/* Wait for active clients to terminate */
 		if(protected_uint32_value(active_clients)) {
 			lprintf(LOG_INFO, "Waiting for %d active clients to disconnect..."
-- 
GitLab