pty.c: Fix broken node spying.

This fixes node spying, which has been broken for some time now:

* Fix deadlock created by double lock (instead of lock/unlock).
  This regression was introduced in commit 70ec483.
* Don't cause the PTY master thread to abort when a remote sysop
  console spying on a node disconnects. Instead, just continue looping.
  This has likely been broken since the PTY code was first written.
* Disable logging to STDOUT for a sysop console before spying,
  to avoid logging interfering with the spy session. mod_sysop will
  restore this logging after spying ends, if applicable.

bbs/node.c

@@ -1436,12 +1436,16 @@ void *bbs_node_handler(void *varg)
 
 static int cli_spy(struct bbs_cli_args *a)
 {
+	int res;
 	int node = atoi(a->argv[1]);
 	if (node <= 0) {
 		bbs_dprintf(a->fdout, "Invalid node %s\n", a->argv[1]);
 		return -1;
 	}
-	return bbs_node_spy(a->fdin, a->fdout, (unsigned int) node);
+	bbs_cli_set_stdout_logging(a->fdout, 0);
+	res = bbs_node_spy(a->fdin, a->fdout, (unsigned int) node);
+	/* Let mod_sysop re-enable logging to stdout, if configured */
+	return res;
 }
 
 static int cli_user(struct bbs_cli_args *a)

bbs/pty.c

@@ -275,12 +275,18 @@ int bbs_node_spy(int fdin, int fdout, unsigned int nodenum)
 	node = bbs_node_get(nodenum); /* This returns node locked */
 	if (!node) {
 		bbs_dprintf(fdout, "No such node: %u\n", nodenum);
+		if (fgconsole) {
+			bbs_alertpipe_close(spy_alert_pipe);
+		}
 		return 0;
 	}
 
 	if (!NODE_HAS_PTY(node)) {
 		bbs_dprintf(fdout, "Node %d does not have a PTY attached (protocol: %s)\n", node->id, node->protname);
 		bbs_node_unlock(node);
+		if (fgconsole) {
+			bbs_alertpipe_close(spy_alert_pipe);
+		}
 		return 0;
 	}
 
@@ -299,7 +305,7 @@ int bbs_node_spy(int fdin, int fdout, unsigned int nodenum)
 		bbs_sigint_set_alertpipe(spy_alert_pipe);
 	}
 	bbs_unbuffer_input(fdin, 0); /* Unbuffer input, so that sysop can type on the node's TTY in real time, not just flushed after line breaks. */
-	bbs_node_pty_lock(node);
+	bbs_node_pty_unlock(node);
 	bbs_node_unlock(node); /* We're done with the node. */
 
 	bbs_verb(3, "Spying begun on node %d\n", nodenum);
@@ -350,7 +356,8 @@ int bbs_node_spy(int fdin, int fdout, unsigned int nodenum)
 				break;
 			}
 		} while (res >= 0); /* poll should not return 0, since we passed -1 for ms */
-		/* Remote console socket is now closed */
+		/* Remote console socket is now closed.
+		 * Because it's closed from rsysop towards the BBS, we can't reset the terminal here. */
 	}
 
 	/* Log this message after the screen was cleared, as the sysop will likely see it,
@@ -359,6 +366,7 @@ int bbs_node_spy(int fdin, int fdout, unsigned int nodenum)
 
 	node = bbs_node_get(nodenum);
 	if (!node) {
+		bbs_debug(3, "Node %d disappeared while we were spying on it\n", nodenum);
 		return 0;
 	}
 	node->spy = 0;
@@ -623,7 +631,7 @@ void *pty_master(void *varg)
 			/* Don't relay user input to sysop for spying here. If we're supposed to, it'll get echoed back in the output. */
 			if (bytes_wrote != bytes_read) {
 				bbs_error("Expected to write %ld bytes, only wrote %ld\n", bytes_read, bytes_wrote);
-				return NULL;
+				break;
 			}
 			if (bytes_read == 1 && *buf >= 1 && *buf <= 26) {
 				/* In general, we should not intercept messages between 1 and 26 (^A through ^Z).
@@ -759,7 +767,7 @@ void *pty_master(void *varg)
 			bytes_read = read(spyfdin, readbuf, sizeof(readbuf));
 			if (bytes_read <= 0) {
 				bbs_debug(10, "pty spy_in read returned %ld (%s)\n", bytes_read, strerror(errno));
-				break; /* We'll read 0 bytes upon disconnect */
+				continue; /* We'll read 0 bytes upon disconnect */
 			}
 #ifdef DEBUG_PTY
 			if (isprint(*buf)) {
@@ -783,7 +791,7 @@ void *pty_master(void *varg)
 				bytes_wrote = write(amaster, buf, (size_t) bytes_read);
 				if (bytes_wrote != bytes_read) {
 					bbs_error("Expected to write %ld bytes, only wrote %ld\n", bytes_read, bytes_wrote);
-					return NULL;
+					break;
 				}
 			} else {
 				bbs_warning("Got spy input for node %d, but not a spy target?\n", nodeid);