node.c: Add node interrupt capability.

Allow node execution to be "interrupted",
which allows a sysop to manually kick a
node, not entirely, but just from whatever
handler it is currently executing. For
example, if a node is executing a door,
interrupting it will kick it back to
the menu from which it accessed that door.

README.rst

@@ -70,6 +70,7 @@ Key features and capabilities include:
 * Sysop capabilities
 
   * Node spying
+  * Interrupt nodes
   * Kick nodes
 
 Usage

bbs/bbs.c

@@ -695,6 +695,16 @@ static void __sigwinch_handler(int num)
 	}
 }
 
+static void __sigusr1_handler(int num)
+{
+	UNUSED(num);
+	/* By default, if we use pthread_kill to try to send SIGUSR1 to a thread,
+	 * it will terminate the entire BBS.
+	 * Installing this dummy signal handler that does nothing prevents that,
+	 * but still allows system calls to be interrupted, as desired. */
+	bbs_debug(3, "Received SIGUSR1\n");
+}
+
 /*!
  * \brief Request BBS shutdown
  * \param type
@@ -956,6 +966,7 @@ int main(int argc, char *argv[])
 	signal(SIGINT, __sigint_handler);
 	signal(SIGTERM, __sigint_handler);
 	signal(SIGWINCH, __sigwinch_handler);
+	signal(SIGUSR1, __sigusr1_handler);
 	sigaction(SIGPIPE, &ignore_sig_handler, NULL);
 
 #define CHECK_INIT(x) if ((x)) { bbs_shutdown(); exit(EXIT_FAILURE); }

bbs/menu.c

@@ -463,7 +463,7 @@ static int bbs_menu_run(struct bbs_node *node, const char *menuname, int stack,
 	struct bbs_menu *menu;
 	struct bbs_menu_item *menuitem;
 	char options[64]; /* No menu will ever have more than this many options... */
-	char menusequence[BBS_MAX_MENUSTACK + 1]; /* No point in reading more options than we can recuse */
+	char menusequence[BBS_MAX_MENUSTACK + 1]; /* No point in reading more options than we can use */
 	int neederror = 0;
 	int forcedrawmenu = 0;
 
@@ -637,6 +637,21 @@ static int bbs_menu_run(struct bbs_node *node, const char *menuname, int stack,
 			res = menu_handler_exec(node, handler, args);
 			node->inmenu = 1;
 		}
+
+		/* If another thread interrupted the node while it was executing a handler, -1 will get returned (probably by poll(2)).
+		 * This is handy, since the -1 return value should cleanly make the handler exit at that point, since it thinks the node is gone.
+		 * However, the node didn't really exit, we just wanted to interrupt it to get it out of the handler, so correct the return value now. */
+		if (bbs_node_interrupted(node)) {
+			bbs_node_interrupt_clear(node);
+			/* Drain any input received, in case poll was interrupt and there's data available,
+			 * to avoid using user input to interact with the menus. */
+			bbs_debug(2, "Flushing interrupted input\n");
+			bbs_node_unbuffer(node); /* Unbuffer input, so we can properly flush it */
+			bbs_node_flush_input(node);
+			bbs_node_buffer(node);
+			res = 0; /* Either 0 or -3 could make sense */
+		}
+
 		/* Intercept -3 and -2 return values from "return" */
 		if (res == -3) { /* Quit */
 			/* Keep returning -3 until we get to the top-level menu (stack == 1). Only then do we return from the menu system completely and exit normally. */

bbs/node.c

@@ -689,6 +689,85 @@ static int cli_nodes(struct bbs_cli_args *a)
 	return 0;
 }
 
+int bbs_interrupt_node(unsigned int nodenum)
+{
+	int res = -1;
+	struct bbs_node *node = bbs_node_get(nodenum);
+
+	if (!node) {
+		return -1;
+	}
+
+	if (!node->thread) {
+		bbs_debug(1, "Node %u is not owned by a thread, and cannot be interrupted\n", nodenum);
+	} else if (!node->slavefd) {
+		/* If there's no PTY, bbs_node_poll can't be used anyways.
+		 * And if there's no PTY, it's a network protocol that doesn't make sense to interrupt.
+		 * Only terminal protocols should be interrupted. */
+		bbs_debug(1, "Node %u has no PTY\n", nodenum);
+	} else {
+		int err;
+		/* The node thread should never interrupt itself, this is only for other threads to
+		 * interrupt a blocking I/O call. */
+		bbs_assert(node->thread != pthread_self());
+		node->interruptack = 0;
+		node->interrupt = 1; /* Indicate that interrupt was requested */
+
+		bbs_node_kill_child(node); /* If executing an external program, kill it */
+
+		/* Make the I/O function (probably poll(2)) exit with EINTR.
+		 * Less overhead than always polling another alertpipe just for getting out of band alerts like this,
+		 * since we can easily enough check the interrupt status in the necessary places on EINTR. */
+		err = pthread_kill(node->thread, SIGUSR1); /* Uncaught signal, so the blocking I/O call will get interrupted */
+		if (err) {
+			bbs_warning("pthread_kill(%lu) failed: %s\n", node->thread, strerror(err));
+			bbs_node_unlock(node);
+			return 1;
+		}
+
+		bbs_verb(5, "Interrupted node %u\n", nodenum);
+		res = 0;
+	}
+
+	bbs_node_unlock(node);
+	return res;
+}
+
+void __bbs_node_interrupt_ack(struct bbs_node *node, const char *file, int line, const char *func)
+{
+	bbs_assert(node->thread == pthread_self());
+	bbs_debug(2, "Node %u acknowledged interrupt at %s:%d %s()\n", node->id, file, line, func);
+	node->interruptack = 1;
+}
+
+void bbs_node_interrupt_clear(struct bbs_node *node)
+{
+	node->interrupt = 0;
+	/* The interrupt should've been acknowledged (e.g. if poll was interrupted),
+	 * but it's entirely possible the node might have returned without ever calling poll,
+	 * in which case it might never have been acknowledged.
+	 * As far as the node thread is concerned, this doesn't matter.
+	 * Currently, we do nothing based on the value of this variable, but we may in the future... */
+	node->interruptack = 0;
+}
+
+int bbs_node_interrupted(struct bbs_node *node)
+{
+	return node->interrupt;
+}
+
+static int cli_interrupt(struct bbs_cli_args *a)
+{
+	int res, node = atoi(a->argv[1]);
+	if (node <= 0) {
+		bbs_dprintf(a->fdout, "Invalid node %s\n", a->argv[1]);
+		return -1;
+	}
+	res = bbs_interrupt_node((unsigned int) node);
+	bbs_dprintf(a->fdout, "%s node %d\n", res ? "Failed to interrupt" : "Successfully interrupted", node);
+	return res;
+}
+
 static int cli_kick(struct bbs_cli_args *a)
 {
 	int node = atoi(a->argv[1]);
@@ -1391,6 +1470,7 @@ static struct bbs_cli_entry cli_commands_nodes[] = {
 	/* Node commands */
 	BBS_CLI_COMMAND(cli_nodes, "nodes", 1, "List all nodes", NULL),
 	BBS_CLI_COMMAND(cli_node, "node", 2, "View information about specified node", "node <nodenum>"),
+	BBS_CLI_COMMAND(cli_interrupt, "interrupt", 2, "Interrupt specified node", "interrupt <nodenum>"),
 	BBS_CLI_COMMAND(cli_kick, "kick", 2, "Kick specified node", "kick <nodenum>"),
 	BBS_CLI_COMMAND(cli_kickall, "kickall", 1, "Kick all nodes", NULL),
 	BBS_CLI_COMMAND(cli_spy, "spy", 2, "Spy on specified node (^C to stop)", "spy <nodenum>"),

bbs/socket.c

@@ -1157,8 +1157,8 @@ int bbs_poll(int fd, int ms)
 
 /* XXX For INTERNAL_POLL_THRESHOLD stuff, if ms == -1, instead of asserting, just set ms to INT_MAX or loop forever, internally */
 
-/* XXX bbs_node_poll should really use bbs_multi_poll internally to avoid duplicating code. Might be a tiny performance hit? */
-int bbs_multi_poll(struct pollfd pfds[], int numfds, int ms)
+/* XXX Lots of code duplicated between bbs_node_poll and __bbs_multi_poll */
+static int __bbs_multi_poll(struct bbs_node *node, struct pollfd pfds[], int numfds, int ms)
 {
 	int i, res;
 
@@ -1208,6 +1208,10 @@ int bbs_multi_poll(struct pollfd pfds[], int numfds, int ms)
 				break;
 			}
 			bbs_debug(7, "poll interrupted\n");
+			if (node && node->interrupt) {
+				bbs_node_interrupt_ack(node);
+				return -1;
+			}
 			continue;
 		}
 		if (res > 0) {
@@ -1239,6 +1243,12 @@ int bbs_multi_poll(struct pollfd pfds[], int numfds, int ms)
 	return res;
 }
 
+/* XXX bbs_node_poll should really use bbs_multi_poll internally to avoid duplicating code. Might be a tiny performance hit? */
+int bbs_multi_poll(struct pollfd pfds[], int numfds, int ms)
+{
+	return __bbs_multi_poll(NULL, pfds, numfds, ms);
+}
+
 /* This is not an assertion, since it can legitimately happen sometimes and that would be overkill. */
 #define REQUIRE_SLAVE_FD(node) \
 	if (node->slavefd == -1) { \
@@ -1253,14 +1263,22 @@ int bbs_node_poll2(struct bbs_node *node, int ms, int fd)
 
 	REQUIRE_SLAVE_FD(node);
 
+	if (bbs_node_interrupted(node)) {
+		/* This could happen if the interrupt flag is set while the node thread isn't executing poll(2).
+		 * In that case, we'll be blissfully unaware of the interrupt until bbs_node_poll is called. */
+		bbs_debug(1, "Node %u still has an interrupt pending, declining to poll the slave\n", node->id);
+		bbs_node_interrupt_ack(node);
+		return -1;
+	}
+
 	/* Watch for data written from the master end of the PTY to the slave end. */
 	/* The lock/unlock to get node->slavefd here is a little silly. It just makes helgrind happy. */
 	bbs_node_lock(node);
 	pfds[0].fd = node->slavefd;
 	bbs_node_unlock(node);
 	pfds[1].fd = fd;
 
-	return bbs_multi_poll(pfds, 2, ms);
+	return __bbs_multi_poll(node, pfds, 2, ms);
 }
 
 int bbs_node_poll(struct bbs_node *node, int ms)
@@ -1273,6 +1291,14 @@ int bbs_node_poll(struct bbs_node *node, int ms)
 	/* We should never be polling indefinitely for a BBS node. */
 	bbs_assert(ms >= 0);
 
+	if (bbs_node_interrupted(node)) {
+		/* This could happen if the interrupt flag is set while the node thread isn't executing poll(2).
+		 * In that case, we'll be blissfully unaware of the interrupt until bbs_node_poll is called. */
+		bbs_debug(1, "Node %u still has an interrupt pending, declining to poll the slave\n", node->id);
+		bbs_node_interrupt_ack(node);
+		return -1;
+	}
+
 	/* Watch for data written from the master end of the PTY to the slave end. */
 	/* The lock/unlock to get node->slavefd here is a little silly. It just makes helgrind happy. */
 	bbs_node_lock(node);
@@ -1320,6 +1346,10 @@ int bbs_node_poll(struct bbs_node *node, int ms)
 				break;
 			}
 			bbs_debug(7, "poll interrupted\n");
+			if (node->interrupt) {
+				bbs_node_interrupt_ack(node);
+				return -1;
+			}
 			continue;
 		}
 		if (res > 0) {
@@ -1415,6 +1445,7 @@ int bbs_node_tpoll(struct bbs_node *node, int ms)
 		/* This is the first poll if <= MIN_POLL_MS_FOR_WARNING, and possibly if >. */
 		if (!res) {
 			res = bbs_node_poll(node, ms);
+			bbs_debug(3, "XXX res %d\n", res);
 			if (res > 0 && warned) {
 				/* This was a response to the "Are you still there?" prompt, not whatever the BBS was doing.
 				 * Flush the response to ignore everything pending in the input buffer.
@@ -1698,6 +1729,7 @@ int bbs_node_readline(struct bbs_node *node, int ms, char *buf, size_t len)
 	}
 
 	for (;;) {
+		size_t bytes;
 		if (keep_trying) {
 			res = bbs_node_poll(node, 5);
 			if (res == 0) {
@@ -1713,12 +1745,12 @@ int bbs_node_readline(struct bbs_node *node, int ms, char *buf, size_t len)
 			bbs_debug(10, "Node %d: poll returned %d\n", node->id, res);
 			return res;
 		}
-		res = (int) read(node->slavefd, buf, len);
-		if (res <= 0) {
-			bbs_debug(10, "Node %d: read returned %d\n", node->id, res);
-			return res;
+		bytes = (size_t) bbs_node_read(node, buf, len);
+		if (bytes <= 0) {
+			bbs_debug(10, "Node %d: read returned %ld\n", node->id, bytes);
+			return (int) res;
 		}
-		nterm = memchr(buf, '\0', (size_t) res);
+		nterm = memchr(buf, '\0', bytes);
 		/* Telnet may send CR NUL or CR LF, so check CR first, then LF.
 		 * To make things even more confusing, Windows Telnet and SyncTERM seem to send LF LF.
 		 * (Though it could be the PTY line discipline converting things that results in this)
@@ -1731,14 +1763,14 @@ int bbs_node_readline(struct bbs_node *node, int ms, char *buf, size_t len)
 		 * In PuTTY with Telnet, there are ^@'s at the beginning of lines after this function returns.
 		 */
 		if (!term) { /* In the case where we poll again for a few ms (below), this will be true, don't do this again. */
-			term = memchr(buf, '\r', (size_t) res); /* There is no strnchr function. Use memchr. */
+			term = memchr(buf, '\r', bytes); /* There is no strnchr function. Use memchr. */
 			if (!term) {
-				term = memchr(buf, '\n', (size_t) res);
+				term = memchr(buf, '\n', bytes);
 			}
 		}
-		buf += res;
-		left -= (size_t) res;
-		bytes_read += res;
+		buf += bytes;
+		left -= bytes;
+		bytes_read += (int) bytes;
 #ifdef DEBUG_TEXT_IO
 		for (i = 0; i < bytes_read; i++) {
 			bbs_debug(10, "read[%d] %d / '%c'\n", i, startbuf[i], isprint(startbuf[i]) ? startbuf[i] : ' ');

include/node.h

@@ -58,6 +58,8 @@ struct bbs_node {
 	unsigned int active:1;		/*!< Active or not */
 	unsigned int buffered:1;	/*!< TTY currently buffered */
 	unsigned int echo:1;		/*!< TTY echo enabled */
+	unsigned int interrupt:1;	/*!< Interrupt request active */
+	unsigned int interruptack:1;/*!< Interrupt request acknowledged by interrupted function */
 	unsigned int spy:1;			/*!< Target of active node spy */
 	unsigned int skipjoin:1;	/*!< If node_shutdown should not join the node thread */
 	unsigned int inmenu:1;		/*!< Whether actively displaying a menu */
@@ -246,6 +248,38 @@ unsigned int bbs_node_shutdown_mod(void *mod);
  */
 int bbs_node_shutdown_all(int shutdown);
 
+/*!
+ * \brief Asynchronously interrupt a blocking system call on a BBS node
+ * \param nodenum Number of node to interrupt
+ * \retval 0 on success, -1 if node does not exist or cannot be interrupted, 1 on failure to interrupt
+ * \note This function must not be called from a node's own thread.
+ * \note This function only works for nodes with a PTY.
+ */
+int bbs_interrupt_node(unsigned int nodenum);
+
+/*!
+ * \brief Whether or not this node was interrupted by another thread
+ * \param node
+ * \note This function may only be called frm a node's own thread.
+ * \retval 1 if interrupted, 0 if not interrupted
+ */
+int bbs_node_interrupted(struct bbs_node *node);
+
+/*!
+ * \brief Clear the interrupt status for a node
+ * \param node
+ */
+void bbs_node_interrupt_clear(struct bbs_node *node);
+
+/*!
+ * \brief Acknowledge an interrupt system call
+ * \param node
+ * \note Must only be called from a node's own thread
+ */
+#define bbs_node_interrupt_ack(node) __bbs_node_interrupt_ack(node, __FILE__, __LINE__, __func__)
+
+void __bbs_node_interrupt_ack(struct bbs_node *node, const char *file, int line, const char *func);
+
 /*!
  * \brief Check whether a user is active on any nodes
  * \param userid User ID of user to check for activity