variants/linux: harden the PTY console and add stdin input

examples/simple_repeater/main.cpp

@@ -18,12 +18,13 @@ SimpleMeshTables tables;
 
 MyMesh the_mesh(board, radio_driver, *new ArduinoMillis(), fast_rng, rtc_clock, tables);
 
-// CLI console stream. On Linux the interactive CLI runs over a Unix-domain
-// socket (so the socket carries only the CLI while Serial keeps the logs); on
-// MCU targets, and as the Linux fallback, it is just Serial.
+// CLI console stream. On Linux the interactive CLI runs over a pseudo-terminal
+// (so the PTY carries only the CLI while Serial keeps the logs) and over stdin
+// when meshcored runs in the foreground of a terminal; on MCU targets it is
+// just Serial.
 #if defined(ARDULINUX_PLATFORM) || defined(LINUX_PLATFORM)
+  #include <stdio.h>
   #include <helpers/LinuxConsole.h>
-  static LinuxConsole linux_console;
 #endif
 static Stream* console = &Serial;
 
@@ -65,6 +66,16 @@ static unsigned long userBtnDownAt = 0;
 #endif
 
 void setup() {
+#if defined(ARDULINUX_PLATFORM) || defined(LINUX_PLATFORM)
+  // Line-buffer the debug log. Serial is stdout here, and the C library gives
+  // stdout line buffering only when it is a terminal; under the systemd unit
+  // (StandardOutput=journal) it is a socket and defaults to a 4 KB block
+  // buffer, so `journalctl -u meshcored -f` would show nothing until 4 KB of
+  // log had piled up. Before the first write, which is why it is here and not
+  // in Console.begin(): board.begin() below already logs.
+  setvbuf(stdout, nullptr, _IOLBF, 0);
+#endif
+
   Serial.begin(115200);
   delay(1000);
 
@@ -75,15 +86,18 @@ void setup() {
 #endif
 
 #if defined(ARDULINUX_PLATFORM) || defined(LINUX_PLATFORM)
-  // Bring up the local CLI console (config path, or a per-user default). Log
-  // which console is active so a failure (e.g. an unwritable configured path) is
-  // diagnosable rather than a silent no-CLI daemon; on failure the CLI stays on
-  // Serial (stdin/stdout).
-  if (linux_console.begin(board.config.console_path)) {
-    console = &linux_console;
-    Serial.print("CLI console on "); Serial.println(linux_console.path());
+  // Bring up the local CLI console: the PTY at console_path (or the default
+  // search order), plus stdin when this is running in the foreground of a
+  // terminal. Log which so a failure (an unwritable configured path, say) is
+  // diagnosable rather than a silent no-CLI daemon. Serial's read() is a stub
+  // on Linux, so there is no fallback to it: without a PTY the CLI is
+  // reachable only from a terminal's stdin.
+  Console.begin(board.config.console_path);
+  console = &Console;
+  if (Console.hasPty()) {
+    Serial.print("CLI console on "); Serial.println(Console.path());
   } else {
-    Serial.println("CLI console: unavailable (see stderr), using stdio");
+    Serial.println("CLI console: no PTY (see stderr); stdin only, if this is a foreground terminal");
   }
 #endif
 

platformio.ini

@@ -233,6 +233,8 @@ build_src_filter =
   +<../src/Packet.cpp>
   +<../src/helpers/ConfigSerializer.cpp>
   +<../src/helpers/DynamicConfigSerializer.cpp>
+  +<../src/helpers/PtyConsole.cpp>
+  +<../src/helpers/LinuxConsole.cpp>
   +<../variants/linux/LinuxEventLoop.cpp>
   +<../variants/linux/LinuxRadioWait.cpp>
 lib_deps =

src/helpers/LinuxConsole.cpp

@@ -0,0 +1,198 @@
+#include "LinuxConsole.h"
+
+#if defined(ARDULINUX_PLATFORM) || defined(LINUX_PLATFORM) || defined(PIO_UNIT_TESTING)
+
+#include <errno.h>
+#include <poll.h>
+#include <signal.h>
+#include <stdio.h>
+#include <termios.h>
+#include <unistd.h>
+
+LinuxConsole Console;
+
+namespace {
+
+// stdin's terminal mode as we found it. This lives here rather than in the
+// object because the signal handler below has to reach it and a handler is
+// given no context of its own; there is exactly one console (see the header),
+// so there is exactly one terminal to remember.
+struct termios        g_orig_tty;
+volatile sig_atomic_t g_raw_active = 0;  // g_orig_tty holds something to restore
+
+// Async-signal-safe: tcsetattr() is on POSIX's list and nothing else here does
+// anything. Shared by end() and by the handler.
+void restore_stdin() {
+    if (g_raw_active) {
+        g_raw_active = 0;
+        tcsetattr(STDIN_FILENO, TCSANOW, &g_orig_tty);
+    }
+}
+
+// loop() never returns and nothing calls exit(), so the destructor is not how
+// this process ends -- SIGINT (Ctrl-C) and SIGTERM (systemctl stop) are. Left
+// to their default action they kill the daemon with the terminal still in raw
+// mode, and the operator is left typing blind at a shell with no echo until
+// they think to run `reset`. So: put the terminal back, then die exactly as we
+// would have -- same signal, default disposition, so the exit status still
+// reports it, and a shell still sees "terminated by SIGINT".
+void restore_tty_and_reraise(int sig) {
+    restore_stdin();
+    signal(sig, SIG_DFL);
+    raise(sig);
+}
+
+void install_restore_handler(int sig) {
+    struct sigaction old;
+    // Only take a signal nothing else is handling: replacing another handler
+    // would silently disable whatever it was for.
+    if (sigaction(sig, nullptr, &old) != 0 || old.sa_handler != SIG_DFL) return;
+    struct sigaction sa;
+    sa.sa_handler = restore_tty_and_reraise;
+    sigemptyset(&sa.sa_mask);
+    sa.sa_flags = 0;  // no SA_RESTART: the main loop's poll() should see EINTR
+    sigaction(sig, &sa, nullptr);
+}
+
+// True if putting stdin in raw mode and reading it cannot stop this process.
+//
+// isatty() alone is not enough, because it is just as true for `meshcored &`:
+// tcsetattr() from a process in a background process group of its *controlling*
+// terminal sends SIGTTOU to that whole group, and read() sends SIGTTIN, both of
+// which stop it by default. begin() runs inside setup(), so a backgrounded
+// daemon would report "Stopped" and never boot. tcgetpgrp() answers the
+// question directly. Failing with ENOTTY answers it too: the terminal on stdin
+// is not this process's controlling terminal, and job control does not apply to
+// it at all (stdin redirected from an unrelated tty, and the test suite's
+// pseudo-terminal, both land here).
+bool stdin_is_drivable() {
+    if (!isatty(STDIN_FILENO)) return false;
+    pid_t fg = tcgetpgrp(STDIN_FILENO);
+    return fg < 0 ? errno == ENOTTY : fg == getpgrp();
+}
+
+// A terminal in the VMIN=0 mode begin() sets returns 0 from read() both when
+// nothing has been typed and when it has gone away, so poll() is what tells the
+// two apart. POLLIN has to be asked for even though POLLHUP is reported
+// regardless: with an empty event mask macOS reports nothing at all.
+bool stdin_hung_up() {
+    struct pollfd p = { STDIN_FILENO, POLLIN, 0 };
+    return poll(&p, 1, 0) > 0 && (p.revents & (POLLHUP | POLLERR | POLLNVAL)) != 0;
+}
+
+}  // namespace
+
+LinuxConsole::~LinuxConsole() { end(); }
+
+bool LinuxConsole::begin(const char* link) {
+    bool pty_ok = _pty.begin(link);
+
+    // Job control must never stop the daemon. Ignoring these does not replace
+    // the check below -- with SIGTTOU ignored tcsetattr() is *allowed* to
+    // proceed, which is exactly the wrong outcome for a background job -- but
+    // it does mean that a session backgrounded after startup (Ctrl-Z, bg), or
+    // one writing to a terminal under `stty tostop`, gets an error return
+    // instead of a stopped process.
+    signal(SIGTTIN, SIG_IGN);
+    signal(SIGTTOU, SIG_IGN);
+
+    // Foreground use: read keystrokes one at a time, no kernel echo (we echo
+    // ourselves, exactly as the PTY path does), Enter delivered as the '\r' the
+    // CLI terminates on, and never block the mesh loop waiting for input --
+    // VMIN=0/VTIME=0 returns 0 from read() the moment there is nothing to read,
+    // which is why the descriptor is left alone: O_NONBLOCK belongs to the open
+    // file description, which fd 0 shares with the shell that started us, and
+    // it would still be set there after this process exits. ISIG stays on so
+    // Ctrl-C still stops the daemon.
+    bool drivable = stdin_is_drivable();
+    if (drivable && !g_raw_active && tcgetattr(STDIN_FILENO, &g_orig_tty) == 0) {
+        struct termios raw = g_orig_tty;
+        raw.c_lflag &= ~(ICANON | ECHO);
+        raw.c_iflag &= ~(ICRNL | INLCR);
+        raw.c_cc[VMIN]  = 0;
+        raw.c_cc[VTIME] = 0;
+        if (tcsetattr(STDIN_FILENO, TCSANOW, &raw) == 0) {
+            g_raw_active = 1;
+            install_restore_handler(SIGINT);
+            install_restore_handler(SIGTERM);
+            install_restore_handler(SIGHUP);
+        }
+    }
+    // Only a terminal that is actually in that mode may be read: a canonical
+    // one would block the entire daemon until somebody pressed Enter.
+    _stdin_tty = drivable && g_raw_active;
+    return pty_ok || _stdin_tty;
+}
+
+void LinuxConsole::end() {
+    _pty.end();
+    restore_stdin();
+    _stdin_tty = false;
+    _peeked    = -1;
+}
+
+int LinuxConsole::stdinFd() const {
+    return _stdin_tty ? STDIN_FILENO : -1;
+}
+
+// The next byte from either source, or -1. The PTY is asked first, but nothing
+// waits on it: whichever has a byte delivers it, so neither can starve the
+// other or leave itself readable and unread.
+int LinuxConsole::fetch() {
+    int c = _pty.read();  // already maps '\n' -> '\r'
+    if (c >= 0) { _src = FROM_PTY; return c; }
+    if (_stdin_tty) {
+        unsigned char b;
+        ssize_t       n = ::read(STDIN_FILENO, &b, 1);
+        if (n == 1) {
+            _src = FROM_STDIN;
+            return (b == '\n') ? '\r' : b;  // same mapping, same reason
+        }
+        // Stop watching a terminal that has gone away: `nohup meshcored &` over
+        // an ssh session that then drops (nohup does not redirect stdin). fd 0
+        // stays open and stays hung up forever, and a level condition is the
+        // one thing the event loop can only throttle, not clear -- the busy
+        // loop this console exists to remove, at a thousand wake-ups a second.
+        // idleUntilEvent() re-registers from stdinFd() on every iteration, so
+        // clearing the flag is what unregisters it.
+        if (n == 0 ? stdin_hung_up() : (errno != EINTR && errno != EAGAIN)) _stdin_tty = false;
+    }
+    return -1;
+}
+
+int LinuxConsole::available() {
+    if (_peeked < 0) _peeked = fetch();
+    return _peeked >= 0 ? 1 : 0;
+}
+
+int LinuxConsole::peek() {
+    if (_peeked < 0) _peeked = fetch();
+    return _peeked;
+}
+
+int LinuxConsole::read() {
+    int c = peek();
+    _peeked = -1;
+    return c;
+}
+
+size_t LinuxConsole::write(uint8_t c) {
+    putchar(c);                           // journald's copy, or the terminal
+    if (_src == FROM_PTY) _pty.write(c);  // and the attached console client
+    // Flushed per byte, unconditionally. At a terminal, a command being typed
+    // has no newline yet and its echo would sit in the buffer until Enter --
+    // the user types blind. Under systemd there is no line buffering to rely on
+    // at all: stdout is a socket, which the C library block-buffers (it
+    // line-buffers only a terminal), so a reply would wait behind 4 KB of log
+    // before journald saw it. One write() per CLI byte, and only on CLI traffic
+    // -- the debug log takes the buffered path set up in setup().
+    fflush(stdout);
+    return 1;
+}
+
+void LinuxConsole::flush() {
+    fflush(stdout);
+    _pty.flush();
+}
+
+#endif // ARDULINUX_PLATFORM || LINUX_PLATFORM || PIO_UNIT_TESTING

src/helpers/LinuxConsole.h

@@ -1,45 +1,99 @@
 #pragma once
 
-#if defined(ARDULINUX_PLATFORM) || defined(LINUX_PLATFORM)
+#if defined(ARDULINUX_PLATFORM) || defined(LINUX_PLATFORM) || defined(PIO_UNIT_TESTING)
 
 #include <Arduino.h>
-#include <stdio.h>
 #include "PtyConsole.h"
 
-// Arduino Stream adapter over PtyConsole.
+// Arduino Stream adapter over PtyConsole, plus stdin when that is a terminal.
 //
 // The repeater's text CLI reads from / writes to this instead of Serial, so the
-// PTY carries only the CLI while Serial (stdout) keeps the debug logs — the
-// clean log/CLI split. write() also mirrors each byte to stdout, so admin
-// commands and their replies are still recorded in the log (journald) alongside
-// the debug output, while the console stays free of log noise.
+// PTY carries only the CLI while Serial (stdout) keeps the debug logs -- the
+// clean log/CLI split. On Linux Serial's read() is a stub that never returns a
+// byte, so this is the only way a command reaches the daemon at all.
 //
-// A client attaches to the published PTY symlink, e.g.:
-//   meshcore-cli -r -s /run/meshcored/console
+// Two doors onto one CLI:
 //
-// All the PTY mechanics live in PtyConsole (host-unit-tested); this adapter is a
-// thin delegating shim.
+//   * the PTY, published at a stable path (see PtyConsole) for
+//     meshcore-cli -r -s <path> or any serial tool; and
+//   * stdin, when meshcored runs in a foreground terminal, so a developer can
+//     type at it without attaching anything. Only in the foreground: driving
+//     the terminal of a backgrounded `meshcored &` would stop the job (see
+//     begin()), so there stdin is left alone and the PTY is the only door.
+//
+// Both are drained on every read, so both can be watched by the event loop
+// permanently. Output follows the input: every byte is written to stdout (when
+// the command came over the PTY that is the copy journald keeps, alongside the
+// debug log), and to the PTY only when the current command arrived there -- a
+// foreground session's echo must not sit in the PTY's buffer waiting to greet
+// the next client.
 class LinuxConsole : public Stream {
+    enum Source { FROM_PTY, FROM_STDIN };
+
     PtyConsole _pty;
+    bool       _stdin_tty = false;     // stdin is a terminal, in raw mode, ours to read
+    int        _peeked    = -1;        // one-byte lookahead over both sources
+    Source     _src       = FROM_PTY;  // where the byte last read came from
+
+    int fetch();
 
 public:
-    // Open the PTY and publish the symlink (see PtyConsole::begin). Returns true
-    // on success; on failure the caller should fall back to Serial.
-    bool begin(const char *link) { return _pty.begin(link); }
-    void end() { _pty.end(); }
-    bool isOpen() const { return _pty.isOpen(); }
-    const char *path() const { return _pty.path(); }
-
-    int available() override { return _pty.available(); }
-    int read() override { return _pty.read(); }
-    int peek() override { return _pty.peek(); }
-    size_t write(uint8_t c) override {
-        putchar(c);       // mirror to stdout so commands/replies reach journald
-        _pty.write(c);    // and to the attached console client
-        return 1;
-    }
-    void flush() override { fflush(stdout); _pty.flush(); }
+    LinuxConsole() = default;
+    ~LinuxConsole();
+
+    // The descriptors below are owned, not shared, and stdin's terminal state is
+    // process-wide. There is exactly one console.
+    LinuxConsole(const LinuxConsole&) = delete;
+    LinuxConsole& operator=(const LinuxConsole&) = delete;
+
+    // Publish the PTY console (see PtyConsole::begin for how `link` and the
+    // default search order are used) and, when stdin is a terminal this process
+    // may drive, put it in raw mode for keystrokes. Never blocks. Returns true
+    // if at least one input is usable; a failure is reported on stderr either
+    // way.
+    //
+    // "may drive" is the foreground check: tcsetattr() and read() from a
+    // background process group of the controlling terminal raise SIGTTOU /
+    // SIGTTIN, which by default *stop* the process -- `meshcored &` would print
+    // "Stopped" in setup() and never boot. Both signals are also set to SIG_IGN
+    // here so that a session backgrounded later (Ctrl-Z, bg) degrades to a read
+    // error rather than a stopped daemon.
+    //
+    // Also arranges for the terminal to be handed back on the way out: a
+    // SIGINT / SIGTERM / SIGHUP handler restores it and re-raises. loop() never
+    // returns and nothing calls exit(), so those signals -- not the destructor
+    // -- are how this process actually ends, and a raw terminal survives it.
+    bool begin(const char* link);
+
+    // Unpublish and close the PTY and restore stdin's terminal mode. Idempotent.
+    //
+    // Exists for LinuxBoard::reboot(), which re-execs this process image: the
+    // descriptors are close-on-exec, but the symlink is on disk, and a symlink
+    // left pointing at a /dev/pts/N that the exec has just freed would name
+    // whatever terminal next reuses that number. Removing it here first makes
+    // the new image's begin() find nothing to reclaim rather than something to
+    // decline.
+    void end();
+
+    bool        hasPty() const { return _pty.isOpen(); }
+    const char* path()   const { return _pty.path(); }
+
+    // Descriptors to watch for readability: the PTY master (-1 when there is
+    // none) and stdin (-1 unless it is a terminal we are driving, and again
+    // once that terminal hangs up). Every byte either delivers is consumed by
+    // read(), so registering both permanently cannot leave a level-triggered
+    // POLLIN that nothing drains.
+    int ptyFd()   const { return _pty.fd(); }
+    int stdinFd() const;
+
+    int available() override;
+    int peek() override;
+    int read() override;
+    size_t write(uint8_t c) override;
+    void flush() override;
     using Print::write;  // pull in write(str) / write(buf, size)
 };
 
-#endif // ARDULINUX_PLATFORM || LINUX_PLATFORM
+extern LinuxConsole Console;
+
+#endif // ARDULINUX_PLATFORM || LINUX_PLATFORM || PIO_UNIT_TESTING