Fix race-condition during shutdown or recycle that could cause crash:

active_clients (protected integer) could be 0 at the time of recycle because
there can be delay at the beginning of the FTP ctrl thread (e.g. looking up
filtered IPs/hostnames) before the active_clients is incremented which could
be during/after it was destroyed by the cleanup() function. Now tracking
number of threads using protected integer which increments immediately upon
child thread creation and is not destroyed until the main thread terminates.
There is currently *not* timeout while waiting for child threads to terminate
upon shutdown/recycle, so this could result in a hung server thread if a child
thread is indefinitely hung - consider using a timeout.

src/sbbs3/ftpsrvr.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 2011 Rob Swindell - http://www.synchro.net/copyright.html		*
+ * Copyright 2012 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				*
@@ -80,6 +80,7 @@ static ftp_startup_t*	startup=NULL;
 static scfg_t	scfg;
 static SOCKET	server_socket=INVALID_SOCKET;
 static protected_int32_t active_clients;
+static protected_int32_t thread_count;
 static volatile time_t	uptime=0;
 static volatile ulong	served=0;
 static volatile BOOL	terminate_server=FALSE;
@@ -205,16 +206,20 @@ static void client_off(SOCKET sock)
 		startup->client_on(startup->cbdata,FALSE,sock,NULL,FALSE);
 }
 
-static void thread_up(BOOL setuid)
+static int32_t thread_up(BOOL setuid)
 {
+	int32_t	count =	protected_int32_adjust(&thread_count,1);
 	if(startup!=NULL && startup->thread_up!=NULL)
 		startup->thread_up(startup->cbdata,TRUE, setuid);
+	return count;
 }
 
-static void thread_down(void)
+static int32_t thread_down(void)
 {
+	int32_t count = protected_int32_adjust(&thread_count,-1);
 	if(startup!=NULL && startup->thread_up!=NULL)
 		startup->thread_up(startup->cbdata,FALSE, FALSE);
+	return count;
 }
 
 static SOCKET ftp_open_socket(int type)
@@ -4473,12 +4478,12 @@ static void ctrl_thread(void* arg)
 	ftp_close_socket(&tmp_sock,__LINE__);
 
 	{
-		int32_t	remain = protected_int32_adjust(&active_clients, -1);
+		int32_t	clients = protected_int32_adjust(&active_clients, -1);
+		int32_t	threads = thread_down();
 		update_clients();
 
-		thread_down();
-		lprintf(LOG_INFO,"%04d CTRL thread terminated (%ld clients remain, %lu served)"
-			,sock, remain, served);
+		lprintf(LOG_INFO,"%04d CTRL thread terminated (%ld clients and %ld threads remain, %lu served)"
+			,sock, clients, threads, served);
 	}
 }
 
@@ -4497,6 +4502,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);
+		}
+	}
+
 	if(active_clients.value)
 		lprintf(LOG_WARNING,"#### !FTP Server terminating with %ld active clients", active_clients.value);
 	else
@@ -4615,6 +4627,7 @@ void DLLCALL ftp_server(void* arg)
 	startup->recycle_now=FALSE;
 	startup->shutdown_now=FALSE;
 	terminate_server=FALSE;
+	protected_int32_init(&thread_count, 0);
 
 	do {
 
@@ -4641,7 +4654,7 @@ void DLLCALL ftp_server(void* arg)
 
 		if(!winsock_startup()) {
 			cleanup(1,__LINE__);
-			return;
+			break;
 		}
 
 		t=time(NULL);
@@ -4660,7 +4673,7 @@ void DLLCALL ftp_server(void* arg)
 			lprintf(LOG_CRIT,"!ERROR %s",error);
 			lprintf(LOG_CRIT,"!Failed to load configuration files");
 			cleanup(1,__LINE__);
-			return;
+			break;
 		}
 
 		if(startup->host_name[0]==0)
@@ -4683,7 +4696,7 @@ void DLLCALL ftp_server(void* arg)
 		if(!isdir(scfg.temp_dir)) {
 			lprintf(LOG_CRIT,"!Invalid temp directory: %s", scfg.temp_dir);
 			cleanup(1,__LINE__);
-			return;
+			break;
 		}
 
 		if(!startup->max_clients) {
@@ -4724,7 +4737,7 @@ void DLLCALL ftp_server(void* arg)
 		if((server_socket=ftp_open_socket(SOCK_STREAM))==INVALID_SOCKET) {
 			lprintf(LOG_CRIT,"!ERROR %d opening socket", ERROR_VALUE);
 			cleanup(1,__LINE__);
-			return;
+			break;
 		}
 
 		lprintf(LOG_DEBUG,"%04d FTP Server socket opened",server_socket);
@@ -4751,14 +4764,14 @@ void DLLCALL ftp_server(void* arg)
 		if(result!=0) {
 			lprintf(LOG_CRIT,"%04d %s", server_socket, BIND_FAILURE_HELP);
 			cleanup(1,__LINE__);
-			return;
+			break;
 		}
 
 		if((result=listen(server_socket, 1))!= 0) {
 			lprintf(LOG_CRIT,"%04d !ERROR %d (%d) listening on socket"
 				,server_socket, result, ERROR_VALUE);
 			cleanup(1,__LINE__);
-			return;
+			break;
 		}
 
 		lprintf(LOG_INFO,"%04d FTP Server listening on port %u",server_socket,startup->port);
@@ -4782,7 +4795,7 @@ void DLLCALL ftp_server(void* arg)
 
 		while(server_socket!=INVALID_SOCKET && !terminate_server) {
 
-			if(active_clients.value==0) {
+			if(thread_count.value <= 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);
@@ -4907,4 +4920,6 @@ void DLLCALL ftp_server(void* arg)
 		}
 
 	} while(!terminate_server);
+
+	protected_int32_destroy(thread_count);
 }