From e28ab10be189fa2ee989b2a4f37285194a9870c8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Deuc=D0=B5?= <shurd@sasktel.net>
Date: Mon, 6 Jan 2025 10:22:55 -0500
Subject: [PATCH] Tie all the input and output threads together with an
 atomic_bool

Since we're using it in one place, use it everywhere.  While we're
here, read()/recv() of 0 after select indicates a closed connection
as well, an ensure we check terminate flags in inner loops.

This should help with SyncTERM issues where it appears to be
connected until you press a key, or it "hangs" and you have to
manually disconnect due to the connection actually having been
torn down at one end or the other.
---
 src/syncterm/conn_pty.c    | 24 ++++++++++++++++--------
 src/syncterm/conn_telnet.c |  1 +
 src/syncterm/modem.c       | 14 ++++++++++----
 src/syncterm/rlogin.c      | 21 ++++++++++++++++-----
 src/syncterm/rlogin.h      |  1 +
 5 files changed, 44 insertions(+), 17 deletions(-)

diff --git a/src/syncterm/conn_pty.c b/src/syncterm/conn_pty.c
index 8680ff7d34..a4c408fd2e 100644
--- a/src/syncterm/conn_pty.c
+++ b/src/syncterm/conn_pty.c
@@ -5,6 +5,7 @@
 #ifdef __unix__
 
 #include <signal.h>   // kill()
+#include <stdatomic.h>
 #include <sys/wait.h> // WEXITSTATUS
 #include <unistd.h>   /* _POSIX_VDISABLE - needed when termios.h is broken */
 
@@ -146,6 +147,7 @@
 #include "uifcinit.h"
 #include "window.h"
 extern int default_font;
+static atomic_bool terminated;
 
 #ifdef NEEDS_CFMAKERAW
 
@@ -315,7 +317,7 @@ pty_input_thread(void *args)
 
 	SetThreadName("PTY Input");
 	conn_api.input_thread_running = 1;
-	while (master != -1 && !conn_api.terminate) {
+	while (master != -1 && !conn_api.terminate && !terminated) {
 		if ((i = waitpid(child_pid, &status, WNOHANG)))
 			break;
 		FD_ZERO(&rds);
@@ -330,17 +332,18 @@ pty_input_thread(void *args)
 		}
 		if (rd == 1) {
 			rd = read(master, conn_api.rd_buf, conn_api.rd_buf_size);
-			if (rd < 0)
-				continue;
+			if (rd <= 0)
+				break;
 		}
 		buffered = 0;
-		while (buffered < rd) {
+		while (buffered < rd && !conn_api.terminate && !terminated) {
 			pthread_mutex_lock(&(conn_inbuf.mutex));
 			buffer = conn_buf_wait_free(&conn_inbuf, rd - buffered, 100);
 			buffered += conn_buf_put(&conn_inbuf, conn_api.rd_buf + buffered, buffer);
 			pthread_mutex_unlock(&(conn_inbuf.mutex));
 		}
 	}
+	terminated = true;
 	conn_api.input_thread_running = 2;
 }
 
@@ -359,7 +362,7 @@ pty_output_thread(void *args)
 
 	SetThreadName("PTY Output");
 	conn_api.output_thread_running = 1;
-	while (master != -1 && !conn_api.terminate) {
+	while (master != -1 && !conn_api.terminate && !terminated) {
 		if (waitpid(child_pid, &status, WNOHANG))
 			break;
 		pthread_mutex_lock(&(conn_outbuf.mutex));
@@ -369,7 +372,7 @@ pty_output_thread(void *args)
 			wr = conn_buf_get(&conn_outbuf, conn_api.wr_buf, conn_api.wr_buf_size);
 			pthread_mutex_unlock(&(conn_outbuf.mutex));
 			sent = 0;
-			while (master != -1 && sent < wr) {
+			while (master != -1 && sent < wr && !conn_api.terminate && !terminated) {
 				FD_ZERO(&wds);
 				FD_SET(master, &wds);
 				tv.tv_sec = 0;
@@ -382,8 +385,10 @@ pty_output_thread(void *args)
 				}
 				if (ret == 1) {
 					ret = write(master, conn_api.wr_buf + sent, wr - sent);
-					if (ret == -1)
-						continue;
+					if (ret <= 0) {
+						ret = -1;
+						break;
+					}
 					sent += ret;
 				}
 			}
@@ -394,6 +399,7 @@ pty_output_thread(void *args)
 		if (ret == -1)
 			break;
 	}
+	terminated = true;
 	conn_api.output_thread_running = 2;
 }
 
@@ -558,6 +564,7 @@ pty_connect(struct bbslist *bbs)
 	}
 	conn_api.wr_buf_size = BUFFER_SIZE;
 
+	terminated = false;
 	_beginthread(pty_output_thread, 0, NULL);
 	_beginthread(pty_input_thread, 0, NULL);
 
@@ -571,6 +578,7 @@ pty_close(void)
 	char   garbage[1024];
 	int oldmaster;
 
+	terminated = true;
 	conn_api.terminate = 1;
 	start = time(NULL);
 	kill(child_pid, SIGHUP);
diff --git a/src/syncterm/conn_telnet.c b/src/syncterm/conn_telnet.c
index 680146ca94..7ca50c5eac 100644
--- a/src/syncterm/conn_telnet.c
+++ b/src/syncterm/conn_telnet.c
@@ -149,6 +149,7 @@ telnet_connect(struct bbslist *bbs)
 	conn_api.rx_parse_cb = telnet_rx_parse_cb;
 	conn_api.tx_parse_cb = telnet_tx_parse_cb;
 
+	rlogin_clear_terminated();
 	_beginthread(rlogin_output_thread, 0, NULL);
 	_beginthread(rlogin_input_thread, 0, bbs);
 	// Suppress Go Aheads (both directions)
diff --git a/src/syncterm/modem.c b/src/syncterm/modem.c
index 77a6dd7e44..1d5b214aba 100644
--- a/src/syncterm/modem.c
+++ b/src/syncterm/modem.c
@@ -2,6 +2,7 @@
 
 /* $Id: modem.c,v 1.32 2020/06/27 08:27:39 deuce Exp $ */
 
+#include <stdatomic.h>
 #include <stdbool.h>
 #include <stdlib.h>
 
@@ -16,6 +17,7 @@
 
 static COM_HANDLE com = COM_HANDLE_INVALID;
 static bool seven_bits = false;
+static atomic_bool terminated;
 
 void
 modem_input_thread(void *args)
@@ -31,7 +33,7 @@ modem_input_thread(void *args)
 		if ((comGetModemStatus(com) & COM_DSR) == 0)
 			monitor_dsr = false;
 	}
-	while (com != COM_HANDLE_INVALID && !conn_api.terminate) {
+	while (com != COM_HANDLE_INVALID && !conn_api.terminate && !terminated) {
 		rd = comReadBuf(com, (char *)conn_api.rd_buf, conn_api.rd_buf_size, NULL, 100);
 		// Strip high bits... we *should* check the parity
 		if (seven_bits) {
@@ -39,7 +41,7 @@ modem_input_thread(void *args)
 				conn_api.rd_buf[i] &= 0x7f;
 		}
 		buffered = 0;
-		while (com != COM_HANDLE_INVALID && buffered < rd) {
+		while (com != COM_HANDLE_INVALID && buffered < rd && !conn_api.terminate && !terminated) {
 			pthread_mutex_lock(&(conn_inbuf.mutex));
 			buffer = conn_buf_wait_free(&conn_inbuf, rd - buffered, 100);
 			buffered += conn_buf_put(&conn_inbuf, conn_api.rd_buf + buffered, buffer);
@@ -54,6 +56,7 @@ modem_input_thread(void *args)
 				break;
 		}
 	}
+	terminated = true;
 	if (args != NULL)
 		comLowerDTR(com);
 	conn_api.input_thread_running = 2;
@@ -74,7 +77,7 @@ modem_output_thread(void *args)
 		if ((comGetModemStatus(com) & COM_DSR) == 0)
 			monitor_dsr = false;
 	}
-	while (com != COM_HANDLE_INVALID && !conn_api.terminate) {
+	while (com != COM_HANDLE_INVALID && !conn_api.terminate && !terminated) {
 		pthread_mutex_lock(&(conn_outbuf.mutex));
 		wr = conn_buf_wait_bytes(&conn_outbuf, 1, 100);
 		if (wr) {
@@ -85,7 +88,7 @@ modem_output_thread(void *args)
 					conn_api.wr_buf[i] &= 0x7f;
 			}
 			sent = 0;
-			while (com != COM_HANDLE_INVALID && sent < wr && !conn_api.terminate) {
+			while (com != COM_HANDLE_INVALID && sent < wr && !conn_api.terminate && !terminated) {
 				// coverity[overflow:SUPPRESS]
 				ret = comWriteBuf(com, conn_api.wr_buf + sent, wr - sent);
 				if (ret > 0)
@@ -106,6 +109,7 @@ modem_output_thread(void *args)
 				break;
 		}
 	}
+	terminated = true;
 	conn_api.output_thread_running = 2;
 }
 
@@ -382,6 +386,7 @@ modem_connect(struct bbslist *bbs)
 	}
 	conn_api.wr_buf_size = BUFFER_SIZE;
 
+	terminated = false;
 	if ((bbs->conn_type == CONN_TYPE_SERIAL) || (bbs->conn_type == CONN_TYPE_SERIAL_NORTS)) {
 		_beginthread(modem_output_thread, 0, (void *)-1);
 		_beginthread(modem_input_thread, 0, (void *)-1);
@@ -419,6 +424,7 @@ modem_close(void)
 	char   garbage[1024];
 	COM_HANDLE oldcom;
 
+	terminated = true;
 	conn_api.terminate = 1;
 
 	if ((comGetModemStatus(com) & COM_DCD) == 0) /* DCD already low */
diff --git a/src/syncterm/rlogin.c b/src/syncterm/rlogin.c
index ca1f3be7cc..d5523083cb 100644
--- a/src/syncterm/rlogin.c
+++ b/src/syncterm/rlogin.c
@@ -2,6 +2,7 @@
 
 /* $Id: rlogin.c,v 1.38 2020/06/27 00:04:50 deuce Exp $ */
 
+#include <stdatomic.h>
 #include <stdlib.h>
 
 #include "bbslist.h"
@@ -10,6 +11,12 @@
 #include "uifcinit.h"
 
 SOCKET rlogin_sock = INVALID_SOCKET;
+static bool terminated;
+
+void rlogin_clear_terminated(void)
+{
+	terminated = false;
+}
 
 #ifdef __BORLANDC__
  #pragma argsused
@@ -24,13 +31,13 @@ rlogin_input_thread(void *args)
 
 	SetThreadName("RLogin Input");
 	conn_api.input_thread_running = 1;
-	while (rlogin_sock != INVALID_SOCKET && !conn_api.terminate) {
+	while (rlogin_sock != INVALID_SOCKET && !conn_api.terminate && !terminated) {
 		if (socket_readable(rlogin_sock, 100)) {
 			rd = recv(rlogin_sock, conn_api.rd_buf, conn_api.rd_buf_size, 0);
 			if (rd <= 0)
 				break;
 			buffered = 0;
-			while (rlogin_sock != INVALID_SOCKET && buffered < rd) {
+			while (rlogin_sock != INVALID_SOCKET && buffered < rd && !conn_api.terminate && !terminated) {
 				pthread_mutex_lock(&(conn_inbuf.mutex));
 				buffer = conn_buf_wait_free(&conn_inbuf, rd - buffered, 1000);
 				buffered += conn_buf_put(&conn_inbuf, conn_api.rd_buf + buffered, buffer);
@@ -38,6 +45,7 @@ rlogin_input_thread(void *args)
 			}
 		}
 	}
+	terminated = true;
 	conn_api.input_thread_running = 2;
 }
 
@@ -54,7 +62,7 @@ rlogin_output_thread(void *args)
 
 	SetThreadName("RLogin Output");
 	conn_api.output_thread_running = 1;
-	while (rlogin_sock != INVALID_SOCKET && !conn_api.terminate) {
+	while (rlogin_sock != INVALID_SOCKET && !conn_api.terminate && !terminated) {
 		pthread_mutex_lock(&(conn_outbuf.mutex));
 		ret = 0;
 		wr = conn_buf_wait_bytes(&conn_outbuf, 1, 100);
@@ -62,13 +70,13 @@ rlogin_output_thread(void *args)
 			wr = conn_buf_get(&conn_outbuf, conn_api.wr_buf, conn_api.wr_buf_size);
 			pthread_mutex_unlock(&(conn_outbuf.mutex));
 			sent = 0;
-			while (rlogin_sock != INVALID_SOCKET && sent < wr && !conn_api.terminate) {
+			while (rlogin_sock != INVALID_SOCKET && sent < wr && !conn_api.terminate && !terminated) {
 				if (socket_writable(rlogin_sock, 100)) {
 					// coverity[overflow:SUPPRESS]
 					ret = sendsocket(rlogin_sock, conn_api.wr_buf + sent, wr - sent);
 					if (ret > 0)
 						sent += ret;
-					if (ret < 0)
+					else
 						break;
 				}
 			}
@@ -79,6 +87,7 @@ rlogin_output_thread(void *args)
 		if (ret < 0)
 			break;
 	}
+	terminated = true;
 	conn_api.output_thread_running = 2;
 }
 
@@ -199,6 +208,7 @@ rlogin_connect(struct bbslist *bbs)
 			return -1;
 	}
 
+	terminated = false;
 	_beginthread(rlogin_output_thread, 0, NULL);
 	_beginthread(rlogin_input_thread, 0, NULL);
 
@@ -214,6 +224,7 @@ rlogin_close(void)
 	char garbage[1024];
 	SOCKET oldsock;
 
+	terminated = true;
 	conn_api.terminate = 1;
 	oldsock = rlogin_sock;
 	rlogin_sock = INVALID_SOCKET;
diff --git a/src/syncterm/rlogin.h b/src/syncterm/rlogin.h
index e7fc52640a..b4574eb944 100644
--- a/src/syncterm/rlogin.h
+++ b/src/syncterm/rlogin.h
@@ -2,6 +2,7 @@
 
 #ifndef _RLOGIN_H_
 #define _RLOGIN_H_
+void rlogin_clear_terminated(void);
 int rlogin_connect(struct bbslist *bbs);
 int rlogin_close(void);
 void rlogin_input_thread(void *args);
-- 
GitLab