diff --git a/scripts/test_shutdown_idempotency.sh b/scripts/test_shutdown_idempotency.sh new file mode 100755 index 0000000..3d16423 --- /dev/null +++ b/scripts/test_shutdown_idempotency.sh @@ -0,0 +1,150 @@ +#!/usr/bin/env bash +# Regression test for issue #22: "Total frames" log printed twice on shutdown. +# +# Runs the wl-webrtc binary briefly, sends SIGINT, then asserts that +# - "Total: N frames in ..." appears exactly once +# - "StatePortal shutdown complete" appears exactly once +# +# Pre-fix: both lines printed twice (explicit shutdown + Drop re-entry). +# Post-fix: both lines printed once (shutdown_started guard). +# +# Usage: +# ./scripts/test_shutdown_idempotency.sh # WebRTC mode, build first +# ./scripts/test_shutdown_idempotency.sh --mode file # --output mode instead +# ./scripts/test_shutdown_idempotency.sh --skip-build # skip cargo build --release +# ./scripts/test_shutdown_idempotency.sh --signal TERM # use SIGTERM instead of SIGINT +# +# Requires: a Wayland session (WAYLAND_DISPLAY). The script will warn but +# proceed if unset; capture will simply fail and the test will report FAIL. + +set -euo pipefail + +MODE="webrtc" +SKIP_BUILD=0 +SIGNAL="INT" +while [[ $# -gt 0 ]]; do + case "$1" in + --mode) MODE="$2"; shift 2 ;; + --mode=*) MODE="${1#*=}"; shift ;; + --skip-build) SKIP_BUILD=1; shift ;; + --signal) SIGNAL="$2"; shift 2 ;; + --signal=*) SIGNAL="${1#*=}"; shift ;; + -h|--help) + sed -n '2,18p' "$0"; exit 0 ;; + *) echo "Unknown arg: $1" >&2; exit 2 ;; + esac +done + +if [[ -t 1 ]]; then + GREEN=$'\e[32m'; RED=$'\e[31m'; YELLOW=$'\e[33m'; BOLD=$'\e[1m'; RESET=$'\e[0m' +else + GREEN=""; RED=""; YELLOW=""; BOLD=""; RESET="" +fi + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$REPO_ROOT" + +if [[ -z "${WAYLAND_DISPLAY:-}" ]]; then + echo "${YELLOW}WARNING${RESET}: WAYLAND_DISPLAY not set; live capture likely to fail." >&2 +fi + +if [[ $SKIP_BUILD -eq 0 ]]; then + echo "${BOLD}Building release binary...${RESET}" + cargo build --release +fi + +BIN="$REPO_ROOT/target/release/wl-webrtc" +if [[ ! -x "$BIN" ]]; then + echo "${RED}FAIL${RESET}: $BIN not found. Run without --skip-build first." >&2 + exit 1 +fi + +case "$MODE" in + webrtc) + PORT=56666 + RUN_ARGS=(--port "$PORT" -v) + EXTRA_CLEANUP=() + ;; + file) + OUTPUT_FILE="$(mktemp --tmpdir "wl22-test-XXXXXX.mp4")" + RUN_ARGS=(--output "$OUTPUT_FILE" -v) + EXTRA_CLEANUP=("rm -f "$OUTPUT_FILE"") + ;; + *) + echo "Invalid --mode: $MODE (use 'webrtc' or 'file')" >&2 + exit 2 + ;; +esac + +LOG="$(mktemp --tmpdir "wl22-test-XXXXXX.log")" +SERVER_PID="" + +cleanup() { + if [[ -n "$SERVER_PID" ]] && kill -0 "$SERVER_PID" 2>/dev/null; then + kill "$SERVER_PID" 2>/dev/null || true + wait "$SERVER_PID" 2>/dev/null || true + fi + rm -f "$LOG" + for cmd in "${EXTRA_CLEANUP[@]}"; do eval "$cmd" 2>/dev/null || true; done +} +trap cleanup EXIT + +echo "${BOLD}Running${RESET}: $BIN ${RUN_ARGS[*]}" +"$BIN" "${RUN_ARGS[@]}" >"$LOG" 2>&1 & +SERVER_PID=$! + +# Give the server time to initialize, capture at least one frame, and stabilize. +sleep 3 + +if ! kill -0 "$SERVER_PID" 2>/dev/null; then + echo "${RED}FAIL${RESET}: server exited before SIGINT could be sent." >&2 + echo "----- Log -----" >&2 + cat "$LOG" >&2 + exit 1 +fi + +echo "${BOLD}Sending SIG${SIGNAL}...${RESET}" +kill -"$SIGNAL" "$SERVER_PID" + +# Wait up to 3s for graceful exit. +for _ in $(seq 1 30); do + if ! kill -0 "$SERVER_PID" 2>/dev/null; then + break + fi + sleep 0.1 +done + +if kill -0 "$SERVER_PID" 2>/dev/null; then + echo "${YELLOW}WARN${RESET}: process still alive 3s after SIG${SIGNAL}; force-killing" + kill -TERM "$SERVER_PID" 2>/dev/null || true + wait "$SERVER_PID" 2>/dev/null || true +fi + +SERVER_PID="" + +# grep -c exits 1 on zero matches, which would abort under `set -e`; swallow it. +TOTAL_COUNT=$(grep -c 'Total:.*frames in' "$LOG" || true) +COMPLETE_COUNT=$(grep -c 'StatePortal shutdown complete' "$LOG" || true) + +echo +echo "${BOLD}----- Last 8 log lines -----${RESET}" +tail -n 8 "$LOG" +echo "${BOLD}------------------------------${RESET}" +echo +echo "\"Total: N frames in ...\": $TOTAL_COUNT occurrence(s) (expected 1)" +echo "\"StatePortal shutdown complete\": $COMPLETE_COUNT occurrence(s) (expected 1)" + +if [[ "$TOTAL_COUNT" -eq 1 && "$COMPLETE_COUNT" -eq 1 ]]; then + echo + echo "${GREEN}${BOLD}PASS${RESET}: shutdown is idempotent (issue #22 fixed)" + exit 0 +fi + +echo +echo "${RED}${BOLD}FAIL${RESET}: shutdown logged multiple times (issue #22 not fixed)" +[[ "$TOTAL_COUNT" -ge 2 ]] && echo " - \"Total:\" printed $TOTAL_COUNT times" +[[ "$COMPLETE_COUNT" -ge 2 ]] && echo " - \"shutdown complete\" printed $COMPLETE_COUNT times" +echo +echo "Debug hint: to compare before/after the fix, run:" +echo " git stash && cargo build --release && $0 --skip-build && git stash pop" +exit 1 diff --git a/src/state_portal.rs b/src/state_portal.rs index a410efd..ba83c39 100644 --- a/src/state_portal.rs +++ b/src/state_portal.rs @@ -66,6 +66,7 @@ pub struct StatePortal { last_fillable_frame: Option, // cached last frame for filler duplication next_filler_at: Option, // when to send next filler frame filler_frames_sent: u64, + shutdown_started: bool, // idempotency guard; plain bool because &mut self is exclusive (not AtomicBool) } impl StatePortal { @@ -112,6 +113,7 @@ impl StatePortal { last_fillable_frame: None, next_filler_at: None, filler_frames_sent: 0, + shutdown_started: false, }) } @@ -586,10 +588,15 @@ impl StatePortal { Ok(()) } - /// 关闭状态：刷新编码器并清理资源 + /// 关闭状态：刷新编码器并清理资源（幂等）。 /// - /// 使用 `enc.take()` 确保编码器只被 flush 一次，即使多次调用也安全（幂等）。 + /// `shutdown_started` 守卫在清理之前置位——防止 panic 时 `Drop` 重入 unwinding。 pub fn shutdown(&mut self) { + if self.shutdown_started { + return; + } + self.shutdown_started = true; + self.last_fillable_frame = None; // 1. Stop encode thread (drops webrtc_tx → signals WebRTC thread to exit) if let Some(mut enc_thread) = self.enc_thread.take() {