diff --git a/CMakeLists.txt b/CMakeLists.txt index 22eb61f..15c6b34 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -44,6 +44,7 @@ pkg_check_modules(JXL REQUIRED IMPORTED_TARGET libjxl libjxl_threads) add_library(fgc_core STATIC src/core/Config.cpp src/core/Paths.cpp + src/core/SingleInstance.cpp src/core/Logger.cpp src/core/Geometry.cpp src/core/ScanGrid.cpp diff --git a/README.md b/README.md index 6ad0ca2..902f034 100644 --- a/README.md +++ b/README.md @@ -47,6 +47,33 @@ Dependencies and the Vimba X SDK setup are in [docs/build-and-setup.md](docs/bui ## Run +On a tower the program must outlive the ssh session that started it, so **`scripts/fgc`** runs it +detached inside a tmux session: + +```bash +scripts/fgc start --start # start detached; returns immediately +scripts/fgc attach # watch the dashboard — Ctrl-b d leaves it running +scripts/fgc status # running? since when? which PID? +scripts/fgc stop # SIGTERM -> clean camera/motor teardown +scripts/fgc peek # snapshot of the pane without attaching +``` + +Disconnecting from ssh while attached — or closing the laptop — is invisible to the program: tmux owns +the pty. **When attached, the dashboard's `q` key exits the program; `Ctrl-b d` is what detaches.** + +From a dev box, `./deploy.sh --run` builds on the device and starts it this way; `--attach`, `--status` +and `--stop` drive it afterwards without re-syncing. + +**Only one instance may run at a time.** The binary takes an exclusive `flock` on +`/run/user//fire_gimbal_control.lock` (or `/tmp/fire_gimbal_control-.lock`) at startup and +exits with status 2 and `another instance is already running (PID N)` if it is taken — two instances +would fight over the serial port, the camera and the exposure store. The check is in the binary, so a +bare `./build/fire_gimbal_control` is refused too. Nothing needs cleaning up after a crash or a power +cut: the kernel drops the lock when the process dies. `--no-lock` bypasses it for a second mock-only +instance during development. + +For development, or for a foreground run you intend to babysit, use `run.sh` directly: + ```bash # Real deployment (needs cameras, motor MCU on the configured serial port, MQTT broker): scripts/run.sh --start @@ -83,6 +110,8 @@ cp config/config.example.ini config.ini # then edit | `--mock-camera` | Use a simulated camera — no Vimba hardware needed. | | `--mock-serial` | Use a simulated motor controller — no serial hardware needed. | | `--tui` / `--no-tui` | Show the full-screen terminal dashboard / force the headless console (overrides `[UI] enable_tui`). | +| `--no-lock` | Skip the single-instance lock (dev only — for a second mock-only instance). | +| `--lock-file ` | Use a specific lockfile instead of the default per-uid path. | | `--log-level ` | `trace`, `debug`, `info`, `warn`, `error`, or `off`. | | `--trace ` | Verbatim wire tracing; comma list of `serial,mqtt,camera,control,all,none`. | | `-h, --help` | Print options and exit. | diff --git a/deploy.sh b/deploy.sh index 7246f13..f5e684c 100755 --- a/deploy.sh +++ b/deploy.sh @@ -7,11 +7,20 @@ # # Usage: # ./deploy.sh rsync + remote configure + build -# ./deploy.sh --run ... then run the app over ssh (RUN_ARGS, ctrl-c to stop) +# ./deploy.sh --run ... then start the app on the device, detached, and return # ./deploy.sh --run --home ... and run the endstop-finding home sequence at startup +# ./deploy.sh --run --attach ... start it, then attach straight away (the usual one) +# ./deploy.sh --attach attach to the app already running (no sync, no build) +# ./deploy.sh --status is it running on the device? +# ./deploy.sh --stop shut the running app down cleanly # ./deploy.sh --clean wipe the remote build dir first (fresh configure) # ./deploy.sh --check-deps only check the remote build dependencies # ./deploy.sh --run --force skip the placeholder-config guard before running +# ./deploy.sh --run --foreground run attached to this terminal instead (debugging) +# +# --run starts the app inside a tmux session on the device (scripts/fgc), so it +# survives this ssh connection closing — reattach later with --attach. Only one +# instance can run at a time; the binary itself enforces that with a lockfile. # # Flags combine, e.g. ./deploy.sh --clean --run # @@ -40,6 +49,8 @@ RUN=0 CHECK_DEPS=0 FORCE=0 HOME_INIT=0 +FOREGROUND=0 +SESSION_CMD="" for arg in "$@"; do case "$arg" in --clean) CLEAN=1 ;; @@ -47,10 +58,35 @@ for arg in "$@"; do --home|--init) HOME_INIT=1 ;; --check-deps) CHECK_DEPS=1 ;; --force) FORCE=1 ;; + --foreground) FOREGROUND=1 ;; + --attach) SESSION_CMD="attach" ;; + --status) SESSION_CMD="status" ;; + --stop) SESSION_CMD="stop" ;; *) echo "unknown option: $arg" >&2; exit 1 ;; esac done +if [[ "$SESSION_CMD" == "attach" && "$FOREGROUND" == "1" ]]; then + echo "error: --attach and --foreground are contradictory (--foreground never detaches)" >&2 + exit 1 +fi +if [[ -n "$SESSION_CMD" && "$SESSION_CMD" != "attach" && "$RUN" == "1" ]]; then + echo "error: --$SESSION_CMD cannot be combined with --run" >&2 + exit 1 +fi + +# Talk to the app already on the device — no sync, no build. `--run --attach` is +# the exception: there the attach happens at the very end, after the build and +# start, so fall through to those instead of exiting here. +if [[ -n "$SESSION_CMD" ]] && [[ "$SESSION_CMD" != "attach" || "$RUN" != "1" ]]; then + # -t only for attach: it needs a real tty for the dashboard. status/stop + # produce plain output and should stay pipeable. + TTY_FLAG=(); [[ "$SESSION_CMD" == "attach" ]] && TTY_FLAG=(-t) + # shellcheck disable=SC2029 + exec ssh "${TTY_FLAG[@]}" "$REMOTE_HOST" \ + "cd '$REMOTE_DIR' && exec scripts/fgc $SESSION_CMD" +fi + # Homing is opt-in: append --init only when --home is passed, so a plain run # brings up the TUI without driving the endstop-finding sequence. if [[ $HOME_INIT -eq 1 && "$RUN_ARGS" != *--init* ]]; then @@ -204,9 +240,29 @@ if [[ "$RUN" == "1" ]]; then fi fi - echo ">> running on $REMOTE_HOST (ctrl-c to stop)" - # -t for a real tty so the capture loop's logs stream and ctrl-c propagates. - # cwd = REMOTE_DIR so ./config.ini and the output dir resolve. + if [[ "$FOREGROUND" == "1" ]]; then + echo ">> running on $REMOTE_HOST, attached to this terminal (ctrl-c to stop)" + # -t for a real tty so the capture loop's logs stream and ctrl-c propagates. + # cwd = REMOTE_DIR so ./config.ini and the output dir resolve. + # shellcheck disable=SC2029 + ssh -t "$REMOTE_HOST" "$RESOLVE_VIMBA_ENV cd '$REMOTE_DIR' && exec ./build/fire_gimbal_control $RUN_ARGS" + exit + fi + + echo ">> starting on $REMOTE_HOST, detached" + # No -t and no RESOLVE_VIMBA_ENV here: scripts/fgc puts the app in a tmux + # session on the device and resolves the Vimba environment inside the pane, + # so it neither needs this ssh session's tty nor inherits its environment. + # This ssh call returns as soon as the app is up (or reports why it isn't). # shellcheck disable=SC2029 - ssh -t "$REMOTE_HOST" "$RESOLVE_VIMBA_ENV cd '$REMOTE_DIR' && exec ./build/fire_gimbal_control $RUN_ARGS" + ssh "$REMOTE_HOST" "cd '$REMOTE_DIR' && exec scripts/fgc start $RUN_ARGS" + + if [[ "$SESSION_CMD" == "attach" ]]; then + echo ">> attaching (Ctrl-b d detaches and leaves it running; 'q' quits the app)" + # shellcheck disable=SC2029 + exec ssh -t "$REMOTE_HOST" "cd '$REMOTE_DIR' && exec scripts/fgc attach" + fi + + echo ">> attach with: ./deploy.sh --attach (Ctrl-b d detaches, 'q' quits the app)" + echo ">> stop with: ./deploy.sh --stop" fi diff --git a/docs/deployment.md b/docs/deployment.md index 120e043..0ed0159 100644 --- a/docs/deployment.md +++ b/docs/deployment.md @@ -141,8 +141,80 @@ Watch for: `Loaded config`, `Serial controller started`, `Opened camera ...`, `M `Saved .../RGB/.jxl` after the first capture. Type `exit` (or Ctrl-D) to stop. To validate without cameras first: add `--mock-camera --mock-serial --no-mqtt`. +This run is attached to your terminal and dies with the ssh session — fine for a first check, not for +leaving it running. For that, use section 6b. + +### 6b. Running detached (`scripts/fgc`) + +The tower is operated over ssh, so the program has to survive the connection closing. `scripts/fgc` runs +it inside a tmux session, which owns the pty: + +```bash +cd /opt/fire_gimbal_control +scripts/fgc start --start # starts detached, returns as soon as it is up +scripts/fgc status # running? since when? which PID? +scripts/fgc attach # the dashboard — Ctrl-b d detaches, leaves it running +scripts/fgc stop # SIGTERM -> the same clean teardown as `exit` +scripts/fgc peek # snapshot of the pane without attaching +``` + +**`Ctrl-b d` detaches. The dashboard's own `q` key exits the program.** That is the one thing to get +right while attached. + +`scripts/fgc stop` sends SIGTERM and waits up to 30s; the program handles it with the same shutdown path +as the `exit` command (stop capture, close the camera, save the exposure store). Do **not** `kill -9` — +an unclean exit leaves the camera wedged for the next run. If it will not stop, attach and look at why. + +`scripts/fgc start` refuses if a run is already up, and prints the previous run's last output if it died +rather than silently discarding it. + +Quitting the dashboard with `q` while attached ends the tmux session too, so you land straight back in +your shell — `exit` then closes ssh. A run that *fails* is the exception: its pane is kept so you can read +why (`remain-on-exit failed`), and `fgc status` shows it until `fgc stop` or the next `fgc start` clears it. + +A kept pane has **no process behind it** — keys and Ctrl-C do nothing, which reads as a hung ssh session. +`Ctrl-b d` still detaches (tmux, not the pane, handles it); the session's status line spells that out along +the bottom of the screen. As a last resort `Enter ~ .` closes the ssh client itself from your end. + +> **Why `_run` waits on the app instead of `exec`ing it.** When the app is itself the pane's process, tmux +> never records an exit status for it — `#{pane_dead_status}` comes back empty and the process lingers as a +> zombie — so `remain-on-exit failed` cannot tell success from failure and keeps *every* finished pane, +> including a clean `q`. That is what used to strand an operator on a frozen dead pane. Interposing a shell +> that reaps the child and exits with its status gives tmux a real status. Measured on the device: 3/3 panes +> kept when exec'd, 4/4 sessions closed cleanly when parented. The wrapper forwards SIGTERM/SIGINT/SIGHUP, +> so `fgc stop` still reaches the app and runs the full teardown. + +### 6c. The single-instance lock + +Two instances would fight over the motor's serial port, the camera, and the exposure store. The binary +prevents that itself: at startup it takes an exclusive `flock` on + +- `/run/user//fire_gimbal_control.lock` when that directory exists, else +- `/tmp/fire_gimbal_control-.lock` + +and, if it is already held, exits with status **2** and: + +``` +Cannot start: another instance is already running (PID 1234); attach to it with: scripts/fgc attach +``` + +The path is derived from the uid and not from `$XDG_RUNTIME_DIR` on purpose: that variable is set in an +interactive ssh shell but not in a non-interactive `ssh host cmd` one, and a lock the two launch routes +disagree about would exclude nothing. + +Because the check lives in the binary, it covers every route in — `scripts/fgc`, systemd, or a bare +`./fire_gimbal_control`. Nothing needs cleaning up after a crash or a power cut: the kernel releases an +`flock` when the holder dies, so a leftover lockfile with a dead PID in it never blocks a launch. Use +`--no-lock` only to run a second mock-only instance during development, and `--lock-file ` to +point at a different lockfile. + ## 7. Install the systemd service +An **alternative** to section 6b, not a companion to it: systemd gives you start-at-boot and +restart-on-crash, but no attachable dashboard — you get `journalctl` instead. Pick one. If you run both by +accident the second one to start fails on the single-instance lock (section 6c), so the failure is loud +rather than a pair of processes corrupting each other's state. + ```bash sudo cp scripts/fire-gimbal-control.service /etc/systemd/system/ sudoedit /etc/systemd/system/fire-gimbal-control.service @@ -202,3 +274,6 @@ If `CMakeLists.txt` or options changed, re-run the `cmake -B build ...` configur | `No camera found` / Vimba startup error | SDK installed, GenTL path set, `ldconfig`, NIC subnet, camera IDs in config | | `MQTT connect failed` (degraded mode) | broker reachable, credentials env, firewall | | Runs but no images | capture started (`--start` or `start` command), `output_dir` writable by `ggs` | +| `another instance is already running` (exit 2) | `scripts/fgc status`; a systemd unit and a tmux session both running (section 6c) | +| Program dies when ssh disconnects | started with `run.sh`/`--foreground` instead of `scripts/fgc start` (section 6b) | +| `No camera found` only when started via `scripts/fgc` | GenTL path not resolved in the pane — check `scripts/fgc peek`, and `/etc/profile.d/*Vimba*GenTL*.sh` or `VIMBA_CTI_PATH` | diff --git a/include/fgc/SingleInstance.h b/include/fgc/SingleInstance.h new file mode 100644 index 0000000..e458a44 --- /dev/null +++ b/include/fgc/SingleInstance.h @@ -0,0 +1,67 @@ +#pragma once + +#include + +namespace fgc { + +// Refuses to let two copies of the program run at once. Two instances would +// fight over the motor's serial port, the camera, and the exposure store, so +// the second launch has to fail loudly rather than half-work. +// +// The mechanism is an advisory flock(2) on a lockfile holding the owner's PID. +// The lock lives in the binary (not the launcher script) so every route in - +// scripts/fgc, systemd, or a bare ./fire_gimbal_control over ssh - is guarded. +// +// There is deliberately no stale-lock cleanup: the kernel drops an flock when +// the holding process dies, including a SIGKILL or a power cut, so a lockfile +// left behind with a dead PID in it never blocks the next launch. The PID +// written inside is purely for the "already running as PID N" message. +// +// auto lock = SingleInstance::acquire(SingleInstance::defaultLockPath()); +// if (!lock) { std::cerr << lock.error() << "\n"; return 2; } +// ... lock stays in scope for the lifetime of the run ... +class SingleInstance { +public: + // Default lockfile: /run/user//fire_gimbal_control.lock when that + // directory exists, else /tmp/fire_gimbal_control-.lock. Derived from + // the uid rather than $XDG_RUNTIME_DIR so it is identical whether the + // program was launched from an interactive shell, a non-interactive + // `ssh host cmd`, or a tmux pane - see the note in the .cpp. + static std::string defaultLockPath(); + + // Try to take the lock. Never blocks. On failure the returned object is + // falsey and error() says why (including the holder's PID when we could + // read it). Always check before use. + static SingleInstance acquire(const std::string& path); + + // A default-constructed instance holds nothing; useful as a placeholder + // when locking is disabled (--no-lock). + SingleInstance() = default; + ~SingleInstance(); + + SingleInstance(SingleInstance&& other) noexcept; + SingleInstance& operator=(SingleInstance&& other) noexcept; + SingleInstance(const SingleInstance&) = delete; + SingleInstance& operator=(const SingleInstance&) = delete; + + bool held() const { return fd_ >= 0; } + explicit operator bool() const { return held(); } + + // Human-readable reason the acquire failed; empty when held(). + const std::string& error() const { return error_; } + // PID of the process holding the lock, or 0 if unknown. Only meaningful + // after a failed acquire. + int holderPid() const { return holder_pid_; } + const std::string& path() const { return path_; } + + // Release early (also done by the destructor). Safe to call twice. + void release(); + +private: + int fd_ = -1; + int holder_pid_ = 0; + std::string path_; + std::string error_; +}; + +} // namespace fgc diff --git a/main.cpp b/main.cpp index ddb3857..39ece91 100644 --- a/main.cpp +++ b/main.cpp @@ -2,6 +2,7 @@ #include "fgc/Config.h" #include "fgc/Logger.h" #include "fgc/Paths.h" +#include "fgc/SingleInstance.h" #include @@ -25,6 +26,9 @@ int main(int argc, char* argv[]) { "use a simulated ambient temp/humidity sensor (implies enable_env)") ("tui", po::bool_switch(), "show the full-screen terminal dashboard") ("no-tui", po::bool_switch(), "force the headless line console (overrides config)") + ("no-lock", po::bool_switch(), + "skip the single-instance lock (dev only: for a second mock-only instance)") + ("lock-file", po::value(), "path to the single-instance lockfile") ("log-level", po::value(), "trace|debug|info|warn|error|off") ("trace", po::value(), "verbatim wire trace, comma list: serial,mqtt,camera,control,all,none"); @@ -61,6 +65,22 @@ int main(int argc, char* argv[]) { } LOG_INFO << "Loaded config: " << *cfg_path; + // Only one instance may run: a second would fight the first over the motor's + // serial port, the camera, and the exposure store. The lock is held for the + // whole run and released when this scope ends (or the process dies). + SingleInstance lock; + if (!vm["no-lock"].as()) { + const std::string lock_path = vm.count("lock-file") + ? paths::expandUser(vm["lock-file"].as()) + : SingleInstance::defaultLockPath(); + lock = SingleInstance::acquire(lock_path); + if (!lock) { + std::cerr << "Cannot start: " << lock.error() << "\n"; + return 2; // distinct from 1 so scripts/fgc can tell this apart + } + LOG_INFO << "Single-instance lock held: " << lock.path(); + } + RuntimeOptions opts; opts.init = vm["init"].as(); opts.start = vm["start"].as(); diff --git a/scripts/fgc b/scripts/fgc new file mode 100755 index 0000000..fc55a2b --- /dev/null +++ b/scripts/fgc @@ -0,0 +1,342 @@ +#!/usr/bin/env bash +# +# fgc — run fire_gimbal_control detached from the terminal, in a tmux session. +# +# The tower is operated over ssh. Launched directly, the program is a child of +# the ssh session's shell and dies with it — mid-scan, and with the camera torn +# down uncleanly (which wedges it for the next run). Here tmux owns the pty, so +# disconnecting is invisible to the program. +# +# scripts/fgc start [args...] start it detached (args go to the binary) +# scripts/fgc attach open the dashboard; Ctrl-b d to leave it running +# scripts/fgc status is it running, since when, as which PID +# scripts/fgc stop clean shutdown (camera + motor teardown) +# scripts/fgc peek print what the pane shows right now +# scripts/fgc restart [args...] stop, then start +# +# Detaching is Ctrl-b d. NOTE: the dashboard's own `q` key *exits the program*, +# it does not detach — that is the one thing to get right when attached. +# +# Only one instance may run. The binary enforces that itself with a lockfile +# (see src/core/SingleInstance.cpp), so a bare ./build/fire_gimbal_control is +# refused too, not just a second `fgc start`. +# +# Env: FGC_SESSION overrides the tmux session name (default "fgc"). +set -euo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +repo="$(cd "$here/.." && pwd)" +session="${FGC_SESSION:-fgc}" +target="=$session" # "=" makes tmux match the name exactly, not as a prefix + +die() { echo "fgc: $*" >&2; exit 1; } + +need_tmux() { + command -v tmux >/dev/null 2>&1 || die "tmux is not installed (apt-get install tmux)" +} + +session_exists() { tmux has-session -t "$target" 2>/dev/null; } + +# "1" if the pane's program has exited, "0" if alive, "" if there is no session. +# remain-on-exit keeps a finished pane around, so a session existing is not on +# its own proof that the program is running. +pane_dead() { tmux list-panes -t "$target" -F '#{pane_dead}' 2>/dev/null | head -1; } + +running() { [[ "$(pane_dead)" == "0" ]]; } + +# PID of the pane's process: the `_run` wrapper shell, which forwards signals to +# the app and reports its exit status to tmux. For the app's own PID use +# app_pid() — the lockfile is written by the program itself. +pane_pid() { tmux list-panes -t "$target" -F '#{pane_pid}' 2>/dev/null | head -1; } + +# PID of fire_gimbal_control itself, from the lockfile it writes at startup. +app_pid() { + local f; f="$(lock_path)" + [[ -e "$f" ]] || return 1 + local p; read -r p <"$f" 2>/dev/null || return 1 + [[ -n "$p" ]] && echo "$p" +} + +# capture-pane needs a pane target; a session name alone gets "can't find pane". +# %N pane ids are unambiguous, so resolve one rather than guessing at "fgc:0.0". +pane_id() { tmux list-panes -t "$target" -F '#{pane_id}' 2>/dev/null | head -1; } + +# Mirrors SingleInstance::defaultLockPath(). Kept env-independent for the same +# reason it is there: $XDG_RUNTIME_DIR is absent in a non-interactive ssh shell. +lock_path() { + local uid; uid="$(id -u)" + if [[ -d "/run/user/$uid" ]]; then + echo "/run/user/$uid/fire_gimbal_control.lock" + else + echo "/tmp/fire_gimbal_control-$uid.lock" + fi +} + +# Is the lock actually held right now? Asked by trying to take it, not by +# checking whether the recorded PID exists: with remain-on-exit set, tmux does +# not reap the pane's process until the pane is destroyed, so a finished run +# leaves a zombie that `kill -0` happily reports as alive. The lock itself is +# the only thing that answers the question, and the kernel drops it on death. +lock_held() { + local f="$1" + [[ -e "$f" ]] || return 1 + ! flock -n "$f" true 2>/dev/null +} + +# Vimba X needs the GenTL transport-layer dir on GENICAM_GENTL64_PATH or +# VmbStartup() fails with "Could not start Vimba X API". The SDK installs that +# via /etc/profile.d, which non-interactive ssh shells don't source. Resolving it +# *here* — i.e. inside the pane, at launch — rather than in the caller's shell +# matters: a tmux server left over from an earlier, differently-configured ssh +# session would otherwise hand the new session its stale environment. +resolve_vimba_env() { + local s d + # The SDK's profile.d script reads GENICAM_GENTL64_PATH before setting it, so + # sourcing it under `set -u` aborts with "unbound variable" and the app never + # launches. Relax nounset for the duration of the source only. + set +u + for s in /etc/profile.d/*Vimba*GenTL*.sh; do + [[ -f "$s" ]] && . "$s" + done + set -u + if [[ -z "${GENICAM_GENTL64_PATH:-}" ]]; then + for d in "${VIMBA_CTI_PATH:-}" /opt/VimbaX/cti /opt/VimbaX_*/cti; do + if [[ -n "$d" && -d "$d" ]]; then export GENICAM_GENTL64_PATH="$d"; break; fi + done + fi +} + +cmd_start() { + need_tmux + if running; then + die "already running (session '$session') — use 'fgc attach', or 'fgc restart'" + fi + if session_exists; then + # A dead pane left over by remain-on-exit from a previous run. Clear it, + # but say so — it is the only trace of how that run ended. + echo "fgc: clearing the finished session from a previous run" >&2 + tmux kill-session -t "$target" 2>/dev/null || true + fi + + # tmux hands the command to `sh -c`, so quote each forwarded argument rather + # than relying on the split surviving the round trip. No `exec` here, and no + # exec in `_run` either — see the comment there for why the wrapper shell has + # to stay in place. `fgc stop` signals the wrapper, which forwards. + local launch + launch="$(printf '%q ' "$here/fgc" _run "$@")" + + # remain-on-exit=failed, not =on: a *failed* start must leave its output on + # screen rather than destroying the session and looking like nothing + # happened, but a clean exit must take the session with it. With =on, quitting + # the dashboard left an operator staring at a frozen dead pane with no process + # behind it — keys and Ctrl-C did nothing, and it read as a hung ssh session. + # + # It is a *window* option, and a pane that dies immediately takes the window + # with it before the option can be set — so create the session on a + # placeholder, set the option, then respawn the pane with the real command. + # -c: the program resolves ./config.ini and the output dir from the cwd. The + # pane runs this script's hidden `_run`, so the Vimba environment is resolved + # inside the pane; run.sh finds the binary, one place only. + tmux new-session -d -s "$session" -c "$repo" 'sleep 86400' + # A window option needs a window target; a session name alone gets + # "no such window". Resolve the window's own id (@N) and use that. + local win; win="$(tmux list-windows -t "$target" -F '#{window_id}' | head -1)" + local pane; pane="$(pane_id)" + # "failed" needs tmux >= 3.2; fall back to "on" rather than leaving it unset, + # where a failed start would vanish without a trace. + tmux set-option -w -t "$win" remain-on-exit failed >/dev/null 2>&1 \ + || tmux set-option -w -t "$win" remain-on-exit on >/dev/null + # Spell the way out along the bottom of the screen. Without it there is no + # clue on screen that Ctrl-b d exists — and if the program dies, the pane + # freezes with no process behind it, so keys and Ctrl-C do nothing and it + # reads as a hung ssh session. tmux's status line stays alive either way. + # These are *session* options, and set-option rejects the "=name" exact-match + # form that every other subcommand here takes ("no such session: =fgc"), so + # target the pane and let tmux resolve the session from it. + tmux set-option -t "$pane" status on >/dev/null + tmux set-option -t "$pane" status-style 'bg=colour24,fg=colour255' >/dev/null + tmux set-option -t "$pane" status-left ' fire-gimbal-control ' >/dev/null + tmux set-option -t "$pane" status-left-length 30 >/dev/null + tmux set-option -t "$pane" status-right-length 80 >/dev/null + tmux set-option -t "$pane" status-right \ + ' Ctrl-b d = detach, keeps running | q = quit the program ' >/dev/null + + tmux respawn-pane -k -t "$win" -c "$repo" "$launch" >/dev/null + + # Give it a moment to fail fast (bad config, lock already held, no camera). + sleep 2 + local dead; dead="$(pane_dead)" + if [[ -z "$dead" ]]; then + # No session at all: the program exited 0 within the 2s window, so + # remain-on-exit=failed tore it down and took the output with it. + die "session '$session' is gone — the program exited immediately (status 0). + Reproduce it in the foreground to see why: scripts/run.sh $*" + fi + if [[ "$dead" == "1" ]]; then + local status + status="$(tmux list-panes -t "$target" -F '#{pane_dead_status}' | head -1)" + echo "fgc: failed to start (exit ${status:-?}):" >&2 + tmux capture-pane -p -t "$(pane_id)" | sed '/^$/d' | tail -20 >&2 + tmux kill-session -t "$target" 2>/dev/null || true + exit "${status:-1}" + fi + + echo "fgc: started detached as PID $(pane_pid) (session '$session')" + echo " attach: scripts/fgc attach (Ctrl-b d to detach; 'q' quits the program)" +} + +# Hidden: the command tmux runs inside the pane. Not for direct use. +# +# The app is run as a CHILD and waited on, rather than exec'd. That looks like a +# pointless extra process, and it is load-bearing: when the app is itself the +# pane's process, tmux never records an exit status for it (#{pane_dead_status} +# comes back empty and the process lingers as a zombie), so remain-on-exit=failed +# cannot tell success from failure and keeps *every* finished pane — which is +# what left an operator staring at a frozen dead pane after pressing q. With a +# shell in between to reap the child and exit with its status, tmux gets a real +# status and tears the session down on a clean quit. Measured: 3/3 kept when +# exec'd, 3/3 clean when parented. +cmd__run() { + resolve_vimba_env + + # <&0 is required: bash redirects a background command's stdin from + # /dev/null unless it is given one explicitly, which would leave the + # dashboard unable to read a single keystroke. + "$here/run.sh" "$@" <&0 & + local child=$! + + # Forward the signals `fgc stop` and a pane kill use, so the app still runs + # its own clean teardown (stop capture, close camera, save exposure store) + # instead of being killed under the wrapper. + trap 'kill -TERM "$child" 2>/dev/null' TERM + trap 'kill -INT "$child" 2>/dev/null' INT + trap 'kill -HUP "$child" 2>/dev/null' HUP + + # A trapped signal makes `wait` return 128+n while the child is still + # shutting down, so keep waiting until it is actually gone before reporting. + local rc=0 + wait "$child"; rc=$? + while kill -0 "$child" 2>/dev/null; do + wait "$child"; rc=$? + done + exit "$rc" +} + +cmd_attach() { + need_tmux + running || die "not running — start it with 'fgc start'" + if [[ -n "${TMUX:-}" ]]; then + die "already inside tmux; use 'tmux switch-client -t $session'" + fi + echo "fgc: attaching — Ctrl-b d detaches and leaves it running ('q' would quit it)" >&2 + exec tmux attach-session -t "$target" +} + +cmd_status() { + need_tmux + local lock; lock="$(lock_path)" + if ! running; then + if session_exists; then + echo "fgc: not running — session '$session' holds a finished pane:" + tmux capture-pane -p -t "$(pane_id)" | sed '/^$/d' | tail -10 | sed 's/^/ /' + echo " clear it with 'fgc stop' (or just 'fgc start')" + else + echo "fgc: not running (no tmux session '$session')" + fi + # The lock outliving the session means an instance is running that fgc + # did not start — systemd, or someone's bare ./fire_gimbal_control. + if lock_held "$lock"; then + local pid; read -r pid <"$lock" || pid="?" + echo " but $lock is held by PID $pid — an instance is running outside tmux" + return 0 + fi + return 1 + fi + local app; app="$(app_pid || echo "$(pane_pid)")" + echo "fgc: running (session '$session', PID $app)" + ps -o lstart=,etime=,args= -p "$app" 2>/dev/null | sed 's/^/ /' + echo " lockfile: $lock" +} + +cmd_stop() { + need_tmux + if ! running; then + if session_exists; then + tmux kill-session -t "$target" 2>/dev/null || true + echo "fgc: cleared the finished session '$session'" + else + echo "fgc: not running" + fi + return 0 + fi + local pid; pid="$(pane_pid)" + + # The pane should be our `_run` wrapper, which forwards the signal to the + # app. Signalling something else would look like a clean stop while the + # program kept running, so check rather than assume. + if ! ps -o args= -p "$pid" 2>/dev/null | grep -qE 'fgc _run|fire_gimbal'; then + echo "fgc: pane process $pid is not the fgc launcher:" >&2 + ps -o args= -p "$pid" 2>/dev/null | sed 's/^/ /' >&2 + die "refusing to signal it — attach and shut it down by hand" + fi + + # SIGTERM, not a keystroke: Application installs a SIGTERM handler that runs + # the same clean teardown as the `exit` command (stop capture, close the + # camera, save the exposure store, park the motor), and unlike sending keys + # it works the same in TUI and headless mode. Never SIGKILL — an unclean + # exit leaves the camera wedged for the next run. + echo "fgc: stopping fire_gimbal_control (PID $(app_pid || echo '?')) via SIGTERM…" + kill -TERM "$pid" 2>/dev/null || true + + # Wait on tmux's view of the pane, not on `kill -0`: while a pane is kept + # after exit, tmux holds the finished process as a zombie until the pane is + # destroyed, so `kill -0` would report it alive forever and every stop would + # "time out". `running` covers both endings — a clean exit destroys the + # session outright, a failed one leaves the pane behind with dead=1. + # + # 120s, because shutdown is not instant: the control loop only notices the + # flag between ticks, and a capture attempt that is timing out on the camera + # (3 tries x 2s) holds it for several seconds before that. + local i + for i in $(seq 1 120); do + running || break + sleep 1 + done + if running; then + echo "fgc: PID $pid still running 120s after SIGTERM — attach and look at why;" >&2 + echo " do NOT kill -9, that wedges the camera." >&2 + return 1 + fi + tmux kill-session -t "$target" 2>/dev/null || true + echo "fgc: stopped" +} + +cmd_peek() { + need_tmux + running || die "not running" + # In TUI mode FTXUI draws on the alternate screen, so this is a snapshot of + # the live dashboard; in headless mode it is the log scrollback. + tmux capture-pane -p -S - -t "$(pane_id)" +} + +cmd_restart() { + cmd_stop + cmd_start "$@" +} + +sub="${1:-}" +[[ $# -gt 0 ]] && shift +case "$sub" in + start) cmd_start "$@" ;; + attach) cmd_attach ;; + status) cmd_status ;; + stop) cmd_stop ;; + peek|logs) cmd_peek ;; + restart) cmd_restart "$@" ;; + _run) cmd__run "$@" ;; + ""|-h|--help|help) + # The header block above, minus the shebang: one source of truth. + awk 'NR>2 && /^#/ {sub(/^# ?/, ""); print; next} NR>2 {exit}' "${BASH_SOURCE[0]}" + [[ -z "$sub" ]] && exit 1 || exit 0 ;; + *) die "unknown command: $sub (try 'fgc --help')" ;; +esac diff --git a/src/core/SingleInstance.cpp b/src/core/SingleInstance.cpp new file mode 100644 index 0000000..6f211a2 --- /dev/null +++ b/src/core/SingleInstance.cpp @@ -0,0 +1,112 @@ +#include "fgc/SingleInstance.h" + +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace fgc { + +namespace { + +// PID currently recorded in an already-locked file, or 0 if it can't be read. +// Best-effort: the message is nicer with a PID but correctness doesn't need it. +int readPid(int fd) { + char buf[32] = {}; + ssize_t n = ::pread(fd, buf, sizeof(buf) - 1, 0); + if (n <= 0) return 0; + buf[n] = '\0'; + return static_cast(std::strtol(buf, nullptr, 10)); +} + +} // namespace + +std::string SingleInstance::defaultLockPath() { + const std::string uid = std::to_string(::getuid()); + + // Deliberately derived from the uid rather than read from $XDG_RUNTIME_DIR: + // that variable is set in an interactive ssh shell but *not* in a + // non-interactive `ssh host cmd` one, so reading it would give the two + // launch paths two different lockfiles - and a lock the launchers disagree + // about is no lock at all. The directory itself is there either way. + std::error_code ec; + const std::string run_dir = "/run/user/" + uid; + if (std::filesystem::is_directory(run_dir, ec)) + return run_dir + "/fire_gimbal_control.lock"; + + return "/tmp/fire_gimbal_control-" + uid + ".lock"; +} + +SingleInstance SingleInstance::acquire(const std::string& path) { + SingleInstance self; + self.path_ = path; + + // O_NOFOLLOW: the /tmp fallback path is world-writable, so refuse to follow a + // symlink another user could have planted there. O_CLOEXEC keeps the lock + // from leaking into anything we exec. + int fd = ::open(path.c_str(), O_RDWR | O_CREAT | O_CLOEXEC | O_NOFOLLOW, 0600); + if (fd < 0) { + self.error_ = "cannot open lockfile " + path + ": " + std::strerror(errno); + return self; + } + + if (::flock(fd, LOCK_EX | LOCK_NB) != 0) { + if (errno == EWOULDBLOCK) { + self.holder_pid_ = readPid(fd); + self.error_ = "another instance is already running"; + if (self.holder_pid_ > 0) self.error_ += " (PID " + std::to_string(self.holder_pid_) + ")"; + self.error_ += "; attach to it with: scripts/fgc attach"; + } else { + self.error_ = "cannot lock " + path + ": " + std::strerror(errno); + } + ::close(fd); + return self; + } + + // Record our PID for the benefit of the next launch's error message and of + // `fgc status`. Failures here are not fatal - the lock itself is what counts. + const std::string pid = std::to_string(::getpid()) + "\n"; + if (::ftruncate(fd, 0) == 0) { + ssize_t ignored = ::pwrite(fd, pid.data(), pid.size(), 0); + (void)ignored; + } + + self.fd_ = fd; + return self; +} + +SingleInstance::~SingleInstance() { release(); } + +SingleInstance::SingleInstance(SingleInstance&& other) noexcept + : fd_(std::exchange(other.fd_, -1)), + holder_pid_(other.holder_pid_), + path_(std::move(other.path_)), + error_(std::move(other.error_)) {} + +SingleInstance& SingleInstance::operator=(SingleInstance&& other) noexcept { + if (this != &other) { + release(); + fd_ = std::exchange(other.fd_, -1); + holder_pid_ = other.holder_pid_; + path_ = std::move(other.path_); + error_ = std::move(other.error_); + } + return *this; +} + +void SingleInstance::release() { + if (fd_ < 0) return; + // Closing the descriptor releases the flock. The file is left in place; + // recreating it on every launch would race with a concurrent acquire. + ::close(fd_); + fd_ = -1; +} + +} // namespace fgc diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 9197a42..67a36fd 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -20,6 +20,7 @@ unset(CMAKE_POLICY_VERSION_MINIMUM) add_executable(fgc_tests doctest_main.cpp test_paths.cpp + test_singleinstance.cpp test_config.cpp test_telemetry.cpp test_command.cpp diff --git a/tests/test_singleinstance.cpp b/tests/test_singleinstance.cpp new file mode 100644 index 0000000..ec62b6d --- /dev/null +++ b/tests/test_singleinstance.cpp @@ -0,0 +1,120 @@ +#include + +#include "fgc/SingleInstance.h" + +#include +#include +#include +#include +#include + +using namespace fgc; + +namespace { + +// A unique lockfile path per test case, cleaned up by the caller. +std::string tempLockPath(const char* tag) { + return (std::filesystem::temp_directory_path() / + ("fgc_test_" + std::string(tag) + "_" + std::to_string(::getpid()) + ".lock")) + .string(); +} + +} // namespace + +TEST_CASE("acquire succeeds on a fresh path and records our PID") { + const std::string path = tempLockPath("fresh"); + std::filesystem::remove(path); + + auto lock = SingleInstance::acquire(path); + REQUIRE(lock.held()); + CHECK(static_cast(lock)); + CHECK(lock.error().empty()); + CHECK(lock.path() == path); + + std::ifstream in(path); + int pid = 0; + in >> pid; + CHECK(pid == ::getpid()); + + lock.release(); + std::filesystem::remove(path); +} + +TEST_CASE("a second acquire on the same path is refused") { + // flock(2) is per open-file-description, not per process, so a second + // open+flock from this same test binary contends exactly as another + // process would - which is what makes this testable without forking. + const std::string path = tempLockPath("contend"); + std::filesystem::remove(path); + + auto first = SingleInstance::acquire(path); + REQUIRE(first.held()); + + auto second = SingleInstance::acquire(path); + CHECK_FALSE(second.held()); + CHECK_FALSE(static_cast(second)); + CHECK(second.error().find("already running") != std::string::npos); + CHECK(second.holderPid() == ::getpid()); + + first.release(); + std::filesystem::remove(path); +} + +TEST_CASE("releasing lets a later acquire through") { + const std::string path = tempLockPath("release"); + std::filesystem::remove(path); + + { + auto lock = SingleInstance::acquire(path); + REQUIRE(lock.held()); + } // destructor releases + + auto again = SingleInstance::acquire(path); + CHECK(again.held()); + again.release(); + CHECK_FALSE(again.held()); + again.release(); // idempotent + + std::filesystem::remove(path); +} + +TEST_CASE("moving transfers the lock without releasing it") { + const std::string path = tempLockPath("move"); + std::filesystem::remove(path); + + auto lock = SingleInstance::acquire(path); + REQUIRE(lock.held()); + auto moved = std::move(lock); + CHECK(moved.held()); + CHECK_FALSE(lock.held()); // NOLINT(bugprone-use-after-move) - checking the moved-from state + + // Still locked against a fresh attempt. + CHECK_FALSE(SingleInstance::acquire(path).held()); + + moved.release(); + std::filesystem::remove(path); +} + +TEST_CASE("acquire reports an unopenable lockfile instead of claiming the lock") { + auto lock = SingleInstance::acquire("/nonexistent-dir-fgc-test/x.lock"); + CHECK_FALSE(lock.held()); + CHECK(lock.error().find("cannot open") != std::string::npos); +} + +TEST_CASE("defaultLockPath is per-uid and independent of the environment") { + const std::string uid = std::to_string(::getuid()); + const std::string run = "/run/user/" + uid; + const std::string expected = std::filesystem::is_directory(run) + ? run + "/fire_gimbal_control.lock" + : "/tmp/fire_gimbal_control-" + uid + ".lock"; + CHECK(SingleInstance::defaultLockPath() == expected); + + // $XDG_RUNTIME_DIR is set in an interactive ssh shell but not in a + // non-interactive `ssh host cmd` one. If the path tracked it, those two + // launch routes would take different locks and neither would exclude the + // other - so the path must not move when the variable does. + setenv("XDG_RUNTIME_DIR", "/run/user/4242", 1); + CHECK(SingleInstance::defaultLockPath() == expected); + unsetenv("XDG_RUNTIME_DIR"); + CHECK(SingleInstance::defaultLockPath() == expected); +}