From 0b6eeff3faab261d3409a05cde2057fd705e3814 Mon Sep 17 00:00:00 2001 From: InterLinked1 <24227567+InterLinked1@users.noreply.github.com> Date: Tue, 26 Sep 2023 06:44:42 -0400 Subject: [PATCH] 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 | 1 + bbs/bbs.c | 11 +++++++ bbs/menu.c | 17 ++++++++++- bbs/node.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++++++ bbs/socket.c | 58 ++++++++++++++++++++++++++++-------- include/node.h | 34 +++++++++++++++++++++ 6 files changed, 187 insertions(+), 14 deletions(-) diff --git a/README.rst b/README.rst index 8a75ca4..158c5b7 100644 --- a/README.rst +++ b/README.rst @@ -70,6 +70,7 @@ Key features and capabilities include: * Sysop capabilities * Node spying + * Interrupt nodes * Kick nodes Usage diff --git a/bbs/bbs.c b/bbs/bbs.c index 71a55a2..fbaf497 100644 --- a/bbs/bbs.c +++ b/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); } diff --git a/bbs/menu.c b/bbs/menu.c index e2b81ab..4e6825d 100644 --- a/bbs/menu.c +++ b/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. */ diff --git a/bbs/node.c b/bbs/node.c index d6ca129..dfe9ecf 100644 --- a/bbs/node.c +++ b/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 "), + BBS_CLI_COMMAND(cli_interrupt, "interrupt", 2, "Interrupt specified node", "interrupt "), BBS_CLI_COMMAND(cli_kick, "kick", 2, "Kick specified node", "kick "), 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 "), diff --git a/bbs/socket.c b/bbs/socket.c index c02d2f8..8b1604a 100644 --- a/bbs/socket.c +++ b/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,6 +1263,14 @@ 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); @@ -1260,7 +1278,7 @@ int bbs_node_poll2(struct bbs_node *node, int ms, int fd) 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] : ' '); diff --git a/include/node.h b/include/node.h index 4713f5b..5cfc677 100644 --- a/include/node.h +++ b/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