From 6350abd09e328f7a2c79710ce5cf20d6aa39cd31 Mon Sep 17 00:00:00 2001
From: rswindell <>
Date: Wed, 8 Jan 2014 02:41:22 +0000
Subject: [PATCH] More _beginthread() race condition fixes to prevent recycle
 and terminate related crashes (mainly due to null pointer dereferences of
 scfg_t members freed in cleanup()). Use of new protected_int_value()  for
 extra paranoia (but can't use it on destroyed protected-int's).

---
 src/sbbs3/ftpsrvr.c  | 39 +++++++++++++------------
 src/sbbs3/mailsrvr.c | 52 +++++++++++++++++----------------
 src/sbbs3/main.cpp   | 28 +++++++++---------
 src/sbbs3/services.c |  6 ++--
 src/sbbs3/websrvr.c  | 68 +++++++++++++++++++++-----------------------
 5 files changed, 95 insertions(+), 98 deletions(-)

diff --git a/src/sbbs3/ftpsrvr.c b/src/sbbs3/ftpsrvr.c
index 55dbbfc6cd..8dd9151fe3 100644
--- a/src/sbbs3/ftpsrvr.c
+++ b/src/sbbs3/ftpsrvr.c
@@ -191,7 +191,7 @@ static void status(char* str)
 static void update_clients(void)
 {
 	if(startup!=NULL && startup->clients!=NULL)
-		startup->clients(startup->cbdata,active_clients.value);
+		startup->clients(startup->cbdata,protected_uint32_value(active_clients));
 }
 
 static void client_on(SOCKET sock, client_t* client, BOOL update)
@@ -206,11 +206,10 @@ static void client_off(SOCKET sock)
 		startup->client_on(startup->cbdata,FALSE,sock,NULL,FALSE);
 }
 
-static int32_t thread_up(BOOL setuid)
+static void thread_up(BOOL setuid)
 {
 	if(startup!=NULL && startup->thread_up!=NULL)
 		startup->thread_up(startup->cbdata,TRUE, setuid);
-	return thread_count.value;
 }
 
 static int32_t thread_down(void)
@@ -2716,7 +2715,7 @@ static void ctrl_thread(void* arg)
 				if(node.status==NODE_INUSE)
 					sockprintf(sock," Node %3d: %s",i+1, username(&scfg,node.useron,str));
 			}
-			sockprintf(sock,"211 End (%d active FTP clients)", active_clients.value);
+			sockprintf(sock,"211 End (%d active FTP clients)", protected_uint32_value(active_clients));
 			continue;
 		}
 		if(!stricmp(cmd, "SITE VER")) {
@@ -4498,6 +4497,13 @@ static void cleanup(int code, int line)
 	lprintf(LOG_DEBUG,"0000 cleanup called from line %d",line);
 #endif
 
+	if(protected_uint32_value(thread_count) > 1) {
+		lprintf(LOG_DEBUG,"#### FTP Server waiting for %d child threads to terminate", protected_uint32_value(thread_count)-1);
+		while(protected_uint32_value(thread_count) > 1) {
+			mswait(100);
+		}
+	}
+
 	free_cfg(&scfg);
 	free_text(text);
 
@@ -4507,20 +4513,13 @@ static void cleanup(int code, int line)
 	if(server_socket!=INVALID_SOCKET)
 		ftp_close_socket(&server_socket,__LINE__);
 
-	if(thread_count.value > 1) {
-		lprintf(LOG_DEBUG,"#### Waiting for %d child threads to terminate", thread_count.value-1);
-		while(thread_count.value > 1) {
-			mswait(100);
-		}
-	}
+	update_clients();	/* active_clients is destroyed below */
 
-	if(active_clients.value)
-		lprintf(LOG_WARNING,"#### !FTP Server terminating with %ld active clients", active_clients.value);
+	if(protected_uint32_value(active_clients))
+		lprintf(LOG_WARNING,"#### !FTP Server terminating with %ld active clients", protected_uint32_value(active_clients));
 	else
 		protected_uint32_destroy(active_clients);
 
-	update_clients();
-
 #ifdef _WINSOCKAPI_
 	if(WSAInitialized && WSACleanup()!=0) 
 		lprintf(LOG_ERR,"0000 !WSACleanup ERROR %d",ERROR_VALUE);
@@ -4801,7 +4800,7 @@ void DLLCALL ftp_server(void* arg)
 
 		while(server_socket!=INVALID_SOCKET && !terminate_server) {
 
-			if(thread_count.value <= 1) {
+			if(protected_uint32_value(thread_count) <= 1) {
 				if(!(startup->options&FTP_OPT_NO_RECYCLE)) {
 					if((p=semfile_list_check(&initialized,recycle_semfiles))!=NULL) {
 						lprintf(LOG_INFO,"0000 Recycle semaphore file (%s) detected",p);
@@ -4873,7 +4872,7 @@ void DLLCALL ftp_server(void* arg)
 				continue;
 			}
 			
-			if(active_clients.value>=startup->max_clients) {
+			if(protected_uint32_value(active_clients)>=startup->max_clients) {
 				lprintf(LOG_WARNING,"%04d !MAXIMUM CLIENTS (%d) reached, access denied"
 					,client_socket, startup->max_clients);
 				sockprintf(client_socket,"421 Maximum active clients reached, please try again later.");
@@ -4903,14 +4902,14 @@ void DLLCALL ftp_server(void* arg)
 		lprintf(LOG_DEBUG,"0000 server_socket: %d",server_socket);
 		lprintf(LOG_DEBUG,"0000 terminate_server: %d",terminate_server);
 #endif
-		if(active_clients.value) {
+		if(protected_uint32_value(active_clients)) {
 			lprintf(LOG_DEBUG,"%04d Waiting for %d active clients to disconnect..."
-				,server_socket, active_clients.value);
+				,server_socket, protected_uint32_value(active_clients));
 			start=time(NULL);
-			while(active_clients.value) {
+			while(protected_uint32_value(active_clients)) {
 				if(time(NULL)-start>startup->max_inactivity) {
 					lprintf(LOG_WARNING,"%04d !TIMEOUT waiting for %d active clients"
-						,server_socket, active_clients.value);
+						,server_socket, protected_uint32_value(active_clients));
 					break;
 				}
 				mswait(100);
diff --git a/src/sbbs3/mailsrvr.c b/src/sbbs3/mailsrvr.c
index 5e174815ff..558b13ee9e 100644
--- a/src/sbbs3/mailsrvr.c
+++ b/src/sbbs3/mailsrvr.c
@@ -8,7 +8,7 @@
  * @format.tab-size 4		(Plain Text/Source Code File Header)			*
  * @format.use-tabs true	(see http://www.synchro.net/ptsc_hdr.html)		*
  *																			*
- * Copyright 2013 Rob Swindell - http://www.synchro.net/copyright.html		*
+ * Copyright 2014 Rob Swindell - http://www.synchro.net/copyright.html		*
  *																			*
  * This program is free software; you can redistribute it and/or			*
  * modify it under the terms of the GNU General Public License				*
@@ -200,7 +200,7 @@ static BOOL winsock_startup(void)
 static void update_clients(void)
 {
 	if(startup!=NULL && startup->clients!=NULL)
-		startup->clients(startup->cbdata,active_clients.value+active_sendmail);
+		startup->clients(startup->cbdata,protected_uint32_value(active_clients)+active_sendmail);
 }
 
 static void client_on(SOCKET sock, client_t* client, BOOL update)
@@ -215,12 +215,10 @@ static void client_off(SOCKET sock)
 		startup->client_on(startup->cbdata,FALSE,sock,NULL,FALSE);
 }
 
-static int32_t thread_up(BOOL setuid)
+static void thread_up(BOOL setuid)
 {
-	int32_t	count =	protected_uint32_adjust(&thread_count,1);
 	if(startup!=NULL && startup->thread_up!=NULL)
 		startup->thread_up(startup->cbdata,TRUE,setuid);
-	return count;
 }
 
 static int32_t thread_down(void)
@@ -4287,7 +4285,6 @@ static void sendmail_thread(void* arg)
 	SetThreadName("SendMail");
 	thread_up(TRUE /* setuid */);
 
-	sendmail_running=TRUE;
 	terminate_sendmail=FALSE;
 
 	lprintf(LOG_INFO,"0000 SendMail thread started");
@@ -4814,6 +4811,13 @@ static void cleanup(int code)
 {
 	int					i;
 
+	if(protected_uint32_value(thread_count)) {
+		lprintf(LOG_DEBUG,"#### Waiting for %d child threads to terminate", protected_uint32_value(thread_count)-1);
+		while(protected_uint32_value(thread_count) > 1) {
+			mswait(100);
+		}
+	}
+
 	free_cfg(&scfg);
 
 	semfile_list_free(&recycle_semfiles);
@@ -4844,20 +4848,13 @@ static void cleanup(int code)
 		pop3_socket=INVALID_SOCKET;
 	}
 
-	if(thread_count.value > 1) {
-		lprintf(LOG_DEBUG,"#### Waiting for %d child threads to terminate", thread_count.value-1);
-		while(thread_count.value > 1) {
-			mswait(100);
-		}
-	}
+	update_clients();	/* active_clients is destroyed below */
 
-	if(active_clients.value)
-		lprintf(LOG_WARNING,"#### !Mail Server terminating with %ld active clients", active_clients.value);
+	if(protected_uint32_value(active_clients))
+		lprintf(LOG_WARNING,"#### !Mail Server terminating with %ld active clients", protected_uint32_value(active_clients));
 	else
 		protected_uint32_destroy(active_clients);
 
-	update_clients();
-
 #ifdef _WINSOCKAPI_	
 	if(WSAInitialized && WSACleanup()!=0) 
 		lprintf(LOG_ERR,"0000 !WSACleanup ERROR %d",ERROR_VALUE);
@@ -4997,6 +4994,7 @@ void DLLCALL mail_server(void* arg)
 
 	do {
 
+		protected_uint32_adjust(&thread_count,1);
 		thread_up(FALSE /* setuid */);
 
 		status("Initializing");
@@ -5273,8 +5271,11 @@ void DLLCALL mail_server(void* arg)
 
 		sem_init(&sendmail_wakeup_sem,0,0);
 
-		if(!(startup->options&MAIL_OPT_NO_SENDMAIL))
+		if(!(startup->options&MAIL_OPT_NO_SENDMAIL)) {
+			sendmail_running=TRUE;
+			protected_uint32_adjust(&thread_count,1);
 			_beginthread(sendmail_thread, 0, NULL);
+		}
 
 		status(STATUS_WFC);
 
@@ -5297,7 +5298,7 @@ void DLLCALL mail_server(void* arg)
 
 		while(server_socket!=INVALID_SOCKET && !terminate_server) {
 
-			if(active_clients.value==0) {
+			if(protected_uint32_value(active_clients)==0 && protected_uint32_value(thread_count) <= 1) {
 				if(!(startup->options&MAIL_OPT_NO_RECYCLE)) {
 					if((p=semfile_list_check(&initialized,recycle_semfiles))!=NULL) {
 						lprintf(LOG_INFO,"%04d Recycle semaphore file (%s) detected"
@@ -5393,7 +5394,7 @@ void DLLCALL mail_server(void* arg)
 					continue;
 				}
 
-				if(active_clients.value>=startup->max_clients) {
+				if(protected_uint32_value(active_clients)>=startup->max_clients) {
 					lprintf(LOG_WARNING,"%04d SMTP !MAXIMUM CLIENTS (%u) reached, access denied (%u total)"
 						,client_socket, startup->max_clients, ++stats.connections_refused);
 					sockprintf(client_socket,"421 Maximum active clients reached, please try again later.");
@@ -5420,6 +5421,7 @@ void DLLCALL mail_server(void* arg)
 
 				smtp->socket=client_socket;
 				smtp->client_addr=client_addr;
+				protected_uint32_adjust(&thread_count,1);
 				_beginthread(smtp_thread, 0, smtp);
 				stats.connections_served++;
 			}
@@ -5457,7 +5459,7 @@ void DLLCALL mail_server(void* arg)
 					continue;
 				}
 
-				if(active_clients.value>=startup->max_clients) {
+				if(protected_uint32_value(active_clients)>=startup->max_clients) {
 					lprintf(LOG_WARNING,"%04d POP3 !MAXIMUM CLIENTS (%u) reached, access denied (%u total)"
 						,client_socket, startup->max_clients, ++stats.connections_refused);
 					sockprintf(client_socket,"-ERR Maximum active clients reached, please try again later.");
@@ -5488,20 +5490,20 @@ void DLLCALL mail_server(void* arg)
 
 				pop3->socket=client_socket;
 				pop3->client_addr=client_addr;
-
+				protected_uint32_adjust(&thread_count,1);
 				_beginthread(pop3_thread, 0, pop3);
 				stats.connections_served++;
 			}
 		}
 
-		if(active_clients.value) {
+		if(protected_uint32_value(active_clients)) {
 			lprintf(LOG_DEBUG,"%04d Waiting for %d active clients to disconnect..."
-				,server_socket, active_clients.value);
+				,server_socket, protected_uint32_value(active_clients));
 			start=time(NULL);
-			while(active_clients.value) {
+			while(protected_uint32_value(active_clients)) {
 				if(startup->max_inactivity && time(NULL)-start>startup->max_inactivity) {
 					lprintf(LOG_WARNING,"%04d !TIMEOUT (%u seconds) waiting for %d active clients"
-						,server_socket, startup->max_inactivity, active_clients.value);
+						,server_socket, startup->max_inactivity, protected_uint32_value(active_clients));
 					break;
 				}
 				mswait(100);
diff --git a/src/sbbs3/main.cpp b/src/sbbs3/main.cpp
index 6dfe8adf9d..76a7534b1d 100644
--- a/src/sbbs3/main.cpp
+++ b/src/sbbs3/main.cpp
@@ -8,7 +8,7 @@
  * @format.tab-size 4		(Plain Text/Source Code File Header)			*
  * @format.use-tabs true	(see http://www.synchro.net/ptsc_hdr.html)		*
  *																			*
- * Copyright 2013 Rob Swindell - http://www.synchro.net/copyright.html		*
+ * Copyright 2014 Rob Swindell - http://www.synchro.net/copyright.html		*
  *																			*
  * This program is free software; you can redistribute it and/or			*
  * modify it under the terms of the GNU General Public License				*
@@ -120,7 +120,7 @@ static const char* status(const char* str)
 static void update_clients()
 {
 	if(startup!=NULL && startup->clients!=NULL)
-		startup->clients(startup->cbdata,node_threads_running.value);
+		startup->clients(startup->cbdata,protected_uint32_value(node_threads_running));
 }
 
 void client_on(SOCKET sock, client_t* client, BOOL update)
@@ -1528,7 +1528,6 @@ void input_thread(void *arg)
 	lprintf(LOG_DEBUG,"Node %d input thread started",sbbs->cfg.node_num);
 #endif
 
-    sbbs->input_thread_running = true;
 	sbbs->console|=CON_R_INPUT;
 
 	while(sbbs->online && sbbs->client_socket!=INVALID_SOCKET
@@ -1796,8 +1795,6 @@ void passthru_output_thread(void* arg)
 	SetThreadName("Passthrough Output");
 	thread_up(FALSE /* setuid */);
 
-    sbbs->passthru_output_thread_running = true;
-
 	while(sbbs->client_socket!=INVALID_SOCKET && sbbs->passthru_socket!=INVALID_SOCKET && !terminate_server) {
 		if(!sbbs->input_thread_mutex_locked) {
 			SLEEP(1);
@@ -1912,8 +1909,6 @@ void passthru_input_thread(void* arg)
 	SetThreadName("Passthrough Input");
 	thread_up(FALSE /* setuid */);
 
-	sbbs->passthru_input_thread_running = true;
-
 	while(sbbs->passthru_socket!=INVALID_SOCKET && !terminate_server) {
 		tv.tv_sec=1;
 		tv.tv_usec=0;
@@ -2013,7 +2008,6 @@ void output_thread(void* arg)
 	lprintf(LOG_DEBUG,"%s output thread started",node);
 #endif
 
-    sbbs->output_thread_running = true;
 	sbbs->console|=CON_R_ECHO;
 
 #ifdef TCP_MAXSEG
@@ -2234,8 +2228,6 @@ void event_thread(void* arg)
 
 	eprintf(LOG_INFO,"BBS Events thread started");
 
-	sbbs->event_thread_running = true;
-
 	sbbs_srand();	/* Seed random number generator */
 
 	SetThreadName("BBS Events");
@@ -4818,6 +4810,7 @@ NO_SSH:
 		cleanup(1);
 		return;
 	}
+    sbbs->output_thread_running = true;
 	_beginthread(output_thread, 0, sbbs);
 
 	if(!(startup->options&BBS_OPT_NO_EVENTS)) {
@@ -4828,6 +4821,7 @@ NO_SSH:
 			cleanup(1);
 			return;
 		}
+		events->event_thread_running = true;
 		_beginthread(event_thread, 0, events);
 	}
 
@@ -4940,7 +4934,7 @@ NO_SSH:
 
 	while(!terminate_server) {
 
-		if(node_threads_running.value==0) {	/* check for re-run flags and recycle/shutdown sem files */
+		if(protected_uint32_value(node_threads_running)==0) {	/* check for re-run flags and recycle/shutdown sem files */
 			if(!(startup->options&BBS_OPT_NO_RECYCLE)) {
 
 				bool rerun=false;
@@ -5471,7 +5465,9 @@ NO_SSH:
 				goto NO_PASSTHRU;
 			}
 			close_socket(tmp_sock);
+			new_node->passthru_output_thread_running = true;
 			_beginthread(passthru_output_thread, 0, new_node);
+			new_node->passthru_input_thread_running = true;
 			_beginthread(passthru_input_thread, 0, new_node);
 
 NO_PASSTHRU:
@@ -5489,7 +5485,9 @@ NO_PASSTHRU:
 #endif
 
 	    protected_uint32_adjust(&node_threads_running, 1);
+	    new_node->input_thread_running = true;
 		new_node->input_thread=(HANDLE)_beginthread(input_thread,0, new_node);
+	    new_node->output_thread_running = true;
 		_beginthread(output_thread, 0, new_node);
 		_beginthread(node_thread, 0, new_node);
 		served++;
@@ -5524,13 +5522,13 @@ NO_PASSTHRU:
     sem_post(&sbbs->outbuf.sem);
 
     // Wait for all node threads to terminate
-	if(node_threads_running.value) {
-		lprintf(LOG_INFO,"Waiting for %d node threads to terminate...", node_threads_running.value);
+	if(protected_uint32_value(node_threads_running)) {
+		lprintf(LOG_INFO,"Waiting for %d node threads to terminate...", protected_uint32_value(node_threads_running));
 		start=time(NULL);
-		while(node_threads_running.value) {
+		while(protected_uint32_value(node_threads_running)) {
 			if(time(NULL)-start>TIMEOUT_THREAD_WAIT) {
 				lprintf(LOG_ERR,"!TIMEOUT waiting for %d node thread(s) to "
-            		"terminate", node_threads_running.value);
+            		"terminate", protected_uint32_value(node_threads_running));
 				break;
 			}
 			mswait(100);
diff --git a/src/sbbs3/services.c b/src/sbbs3/services.c
index b5d556ce8c..149e4d60ca 100644
--- a/src/sbbs3/services.c
+++ b/src/sbbs3/services.c
@@ -1664,8 +1664,8 @@ static service_t* read_services_ini(const char* services_ini, service_t* service
 
 static void cleanup(int code)
 {
-	while(threads_pending_start.value) {
-		lprintf(LOG_NOTICE,"#### Services cleanup waiting on %d threads pending start",threads_pending_start.value);
+	while(protected_uint32_value(threads_pending_start)) {
+		lprintf(LOG_NOTICE,"#### Services cleanup waiting on %d threads pending start",protected_uint32_value(threads_pending_start));
 		SLEEP(1000);
 	}
 	protected_uint32_destroy(threads_pending_start);
@@ -1983,7 +1983,7 @@ void DLLCALL services_thread(void* arg)
 		/* Main Server Loop */
 		while(!terminated) {
 
-			if(active_clients()==0 && threads_pending_start.value==0) {
+			if(active_clients()==0 && protected_uint32_value(threads_pending_start)==0) {
 				if(!(startup->options&BBS_OPT_NO_RECYCLE)) {
 					if((p=semfile_list_check(&initialized,recycle_semfiles))!=NULL) {
 						lprintf(LOG_INFO,"0000 Recycle semaphore file (%s) detected",p);
diff --git a/src/sbbs3/websrvr.c b/src/sbbs3/websrvr.c
index fc21d3c863..b229a8e092 100644
--- a/src/sbbs3/websrvr.c
+++ b/src/sbbs3/websrvr.c
@@ -103,10 +103,10 @@ enum {
 static scfg_t	scfg;
 static volatile BOOL	http_logging_thread_running=FALSE;
 static protected_uint32_t active_clients;
+static protected_uint32_t thread_count;
 static volatile ulong	sockets=0;
 static volatile BOOL	terminate_server=FALSE;
 static volatile BOOL	terminate_http_logging_thread=FALSE;
-static volatile	ulong	thread_count=0;
 static SOCKET	server_socket=INVALID_SOCKET;
 static char		revision[16];
 static char		root_dir[MAX_PATH+1];
@@ -122,7 +122,6 @@ static js_server_props_t js_server_props;
 static str_list_t recycle_semfiles;
 static str_list_t shutdown_semfiles;
 static str_list_t cgi_env;
-static volatile ulong session_threads=0;
 
 static named_string_t** mime_types;
 static named_string_t** cgi_handlers;
@@ -618,7 +617,7 @@ static void status(char* str)
 static void update_clients(void)
 {
 	if(startup!=NULL && startup->clients!=NULL)
-		startup->clients(startup->cbdata,active_clients.value);
+		startup->clients(startup->cbdata,protected_uint32_value(active_clients));
 }
 
 static void client_on(SOCKET sock, client_t* client, BOOL update)
@@ -635,15 +634,13 @@ static void client_off(SOCKET sock)
 
 static void thread_up(BOOL setuid)
 {
-	thread_count++;
 	if(startup!=NULL && startup->thread_up!=NULL)
 		startup->thread_up(startup->cbdata,TRUE, setuid);
 }
 
 static void thread_down(void)
 {
-	if(thread_count>0)
-		thread_count--;
+	protected_uint32_adjust(&thread_count,-1);
 	if(startup!=NULL && startup->thread_up!=NULL)
 		startup->thread_up(startup->cbdata,FALSE, FALSE);
 }
@@ -4939,11 +4936,14 @@ void http_output_thread(void *arg)
 	unsigned mss=OUTBUF_LEN;
 
 	SetThreadName("HTTP Output");
+	thread_up(TRUE /* setuid */);
+
 	obuf=&(session->outbuf);
 	/* Destroyed at end of function */
 	if((i=pthread_mutex_init(&session->outbuf_write,NULL))!=0) {
 		lprintf(LOG_DEBUG,"Error %d initializing outbuf mutex",i);
 		close_socket(&session->socket);
+		thread_down();
 		return;
 	}
 	session->outbuf_write_initialized=1;
@@ -4973,7 +4973,6 @@ void http_output_thread(void *arg)
 	}
 #endif
 
-	thread_up(TRUE /* setuid */);
 	/*
 	 * Do *not* exit on terminate_server... wait for session thread
 	 * to close the socket and set it to INVALID_SOCKET
@@ -5066,9 +5065,11 @@ void http_session_thread(void* arg)
 	session=*(http_session_t*)arg;	/* copies arg BEFORE it's freed */
 	FREE_AND_NULL(arg);
 
+	thread_up(TRUE /* setuid */);
+
 	socket=session.socket;
 	if(socket==INVALID_SOCKET) {
-		session_threads--;
+		thread_down();
 		return;
 	}
 	lprintf(LOG_DEBUG,"%04d Session thread started", session.socket);
@@ -5081,7 +5082,6 @@ void http_session_thread(void* arg)
 		PlaySound(startup->answer_sound, NULL, SND_ASYNC|SND_FILENAME);
 #endif
 
-	thread_up(TRUE /* setuid */);
 	session.finished=FALSE;
 
 	/* Start up the output buffer */
@@ -5090,12 +5090,12 @@ void http_session_thread(void* arg)
 		lprintf(LOG_ERR,"%04d Canot create output ringbuffer!", session.socket);
 		close_socket(&session.socket);
 		thread_down();
-		session_threads--;
 		return;
 	}
 
 	/* Destroyed in this block (before all returns) */
 	sem_init(&session.output_thread_terminated,0,0);
+	protected_uint32_adjust(&thread_count,1);
 	_beginthread(http_output_thread, 0, &session);
 
 	sbbs_srand();	/* Seed random number generator */
@@ -5128,7 +5128,6 @@ void http_session_thread(void* arg)
 			sem_destroy(&session.output_thread_terminated);
 			RingBufDispose(&session.outbuf);
 			thread_down();
-			session_threads--;
 			return;
 		}
 	}
@@ -5141,7 +5140,6 @@ void http_session_thread(void* arg)
 		sem_destroy(&session.output_thread_terminated);
 		RingBufDispose(&session.outbuf);
 		thread_down();
-		session_threads--;
 		return;
 	}
 
@@ -5268,14 +5266,13 @@ void http_session_thread(void* arg)
 	update_clients();
 	client_off(socket);
 
-	session_threads--;
 	thread_down();
 
 	if(startup->index_file_name==NULL || startup->cgi_ext==NULL)
 		lprintf(LOG_DEBUG,"%04d !!! ALL YOUR BASE ARE BELONG TO US !!!", socket);
 
 	lprintf(LOG_INFO,"%04d Session thread terminated (%u clients, %u threads remain, %lu served)"
-		,socket, clients_remain, thread_count, served);
+		,socket, clients_remain, protected_uint32_value(thread_count), served);
 
 }
 
@@ -5287,9 +5284,11 @@ void DLLCALL web_terminate(void)
 
 static void cleanup(int code)
 {
-	while(session_threads) {
-		lprintf(LOG_INFO,"#### Web Server waiting on %d active session threads",session_threads);
-		SLEEP(1000);
+	if(protected_uint32_value(thread_count) > 1) {
+		lprintf(LOG_DEBUG,"#### Web Server waiting for %d child threads to terminate", protected_uint32_value(thread_count)-1);
+		while(protected_uint32_value(thread_count) > 1) {
+			mswait(100);
+		}
 	}
 	free_cfg(&scfg);
 
@@ -5309,13 +5308,13 @@ static void cleanup(int code)
 		close_socket(&server_socket);
 	}
 
-	if(active_clients.value)
-		lprintf(LOG_WARNING,"#### Web Server terminating with %ld active clients", active_clients.value);
+	update_clients();	/* active_clients is destroyed below */
+
+	if(protected_uint32_value(active_clients))
+		lprintf(LOG_WARNING,"#### Web Server terminating with %ld active clients", protected_uint32_value(active_clients));
 	else
 		protected_uint32_destroy(active_clients);
 
-	update_clients();
-
 #ifdef _WINSOCKAPI_
 	if(WSAInitialized && WSACleanup()!=0) 
 		lprintf(LOG_ERR,"0000 !WSACleanup ERROR %d",ERROR_VALUE);
@@ -5325,8 +5324,6 @@ static void cleanup(int code)
 	status("Down");
 	if(terminate_server || code)
 		lprintf(LOG_INFO,"#### Web Server thread terminated (%lu clients served)", served);
-	if(thread_count)
-		lprintf(LOG_WARNING,"#### !Web Server threads (%u) remain after termination", thread_count);
 	if(startup!=NULL && startup->terminated!=NULL)
 		startup->terminated(startup->cbdata,code);
 }
@@ -5361,7 +5358,6 @@ void http_logging_thread(void* arg)
 	char	newfilename[MAX_PATH+1];
 	FILE*	logfile=NULL;
 
-	http_logging_thread_running=TRUE;
 	terminate_http_logging_thread=FALSE;
 
 	SAFECOPY(base,arg);
@@ -5542,9 +5538,11 @@ void DLLCALL web_server(void* arg)
 	startup->recycle_now=FALSE;
 	startup->shutdown_now=FALSE;
 	terminate_server=FALSE;
+	protected_uint32_init(&thread_count, 0);
 
 	do {
 
+		protected_uint32_adjust(&thread_count,1);
 		thread_up(FALSE /* setuid */);
 
 		status("Initializing");
@@ -5712,6 +5710,8 @@ void DLLCALL web_server(void* arg)
 			/********************/
 			/* Start log thread */
 			/********************/
+			http_logging_thread_running=TRUE;
+			protected_uint32_adjust(&thread_count,1);
 			_beginthread(http_logging_thread, 0, startup->logfile_base);
 		}
 
@@ -5753,7 +5753,7 @@ void DLLCALL web_server(void* arg)
 		while(server_socket!=INVALID_SOCKET && !terminate_server) {
 
 			/* check for re-cycle/shutdown semaphores */
-			if(active_clients.value==0) {
+			if(protected_uint32_value(thread_count) <= (2 /* web_server() and http_output_thread() */ + http_logging_thread_running)) {
 				if(!(startup->options&BBS_OPT_NO_RECYCLE)) {
 					if((p=semfile_list_check(&initialized,recycle_semfiles))!=NULL) {
 						lprintf(LOG_INFO,"%04d Recycle semaphore file (%s) detected"
@@ -5764,10 +5764,6 @@ void DLLCALL web_server(void* arg)
 						}
 						break;
 					}
-#if 0	/* unused */
-					if(startup->recycle_sem!=NULL && sem_trywait(&startup->recycle_sem)==0)
-						startup->recycle_now=TRUE;
-#endif
 					if(startup->recycle_now==TRUE) {
 						lprintf(LOG_INFO,"%04d Recycle semaphore signaled",server_socket);
 						startup->recycle_now=FALSE;
@@ -5808,7 +5804,7 @@ void DLLCALL web_server(void* arg)
 				/* Destroyed in http_session_thread */
 				pthread_mutex_init(&session->struct_filled,NULL);
 				pthread_mutex_lock(&session->struct_filled);
-				session_threads++;
+				protected_uint32_adjust(&thread_count,1);
 				_beginthread(http_session_thread, 0, session);
 			}
 
@@ -5873,7 +5869,7 @@ void DLLCALL web_server(void* arg)
 				continue;
 			}
 
-			if(startup->max_clients && active_clients.value>=startup->max_clients) {
+			if(startup->max_clients && protected_uint32_value(active_clients)>=startup->max_clients) {
 				lprintf(LOG_WARNING,"%04d !MAXIMUM CLIENTS (%d) reached, access denied"
 					,client_socket, startup->max_clients);
 				mswait(3000);
@@ -5910,14 +5906,14 @@ void DLLCALL web_server(void* arg)
 		}
 
 		/* Wait for active clients to terminate */
-		if(active_clients.value) {
+		if(protected_uint32_value(active_clients)) {
 			lprintf(LOG_DEBUG,"%04d Waiting for %d active clients to disconnect..."
-				,server_socket, active_clients.value);
+				,server_socket, protected_uint32_value(active_clients));
 			start=time(NULL);
-			while(active_clients.value) {
+			while(protected_uint32_value(active_clients)) {
 				if(time(NULL)-start>startup->max_inactivity) {
 					lprintf(LOG_WARNING,"%04d !TIMEOUT waiting for %d active clients"
-						,server_socket, active_clients.value);
+						,server_socket, protected_uint32_value(active_clients));
 					break;
 				}
 				mswait(100);
@@ -5961,4 +5957,6 @@ void DLLCALL web_server(void* arg)
 		}
 
 	} while(!terminate_server);
+
+	protected_uint32_destroy(thread_count);
 }
-- 
GitLab