fix(state_portal): make shutdown() idempotent to prevent duplicate log lines (#22)

shutdown() fired twice on exit (explicit call in main.rs + Drop impl), causing "Total: N frames" and "StatePortal shutdown complete" to log twice 23us apart. Add `shutdown_started: bool` guard at function entry.

Plain bool (not AtomicBool) because &mut self already grants exclusive access. Guard is set BEFORE cleanup so Drop re-entry during panic unwinding is suppressed.

Includes scripts/test_shutdown_idempotency.sh as a live regression test (requires Wayland session). Verified PASS on KWin: both lines print exactly once.
This commit is contained in:
dailz
2026-06-20 18:45:53 +08:00
parent 0aba0e651e
commit 92760dd8ee
2 changed files with 159 additions and 2 deletions
+150
View File
@@ -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
+9 -2
View File
@@ -66,6 +66,7 @@ pub struct StatePortal {
last_fillable_frame: Option<CpuNv12Frame>, // cached last frame for filler duplication last_fillable_frame: Option<CpuNv12Frame>, // cached last frame for filler duplication
next_filler_at: Option<Instant>, // when to send next filler frame next_filler_at: Option<Instant>, // when to send next filler frame
filler_frames_sent: u64, filler_frames_sent: u64,
shutdown_started: bool, // idempotency guard; plain bool because &mut self is exclusive (not AtomicBool)
} }
impl StatePortal { impl StatePortal {
@@ -112,6 +113,7 @@ impl StatePortal {
last_fillable_frame: None, last_fillable_frame: None,
next_filler_at: None, next_filler_at: None,
filler_frames_sent: 0, filler_frames_sent: 0,
shutdown_started: false,
}) })
} }
@@ -586,10 +588,15 @@ impl StatePortal {
Ok(()) Ok(())
} }
/// 关闭状态:刷新编码器并清理资源 /// 关闭状态:刷新编码器并清理资源(幂等)。
/// ///
/// 使用 `enc.take()` 确保编码器只被 flush 一次,即使多次调用也安全(幂等) /// `shutdown_started` 守卫在清理之前置位——防止 panic 时 `Drop` 重入 unwinding
pub fn shutdown(&mut self) { pub fn shutdown(&mut self) {
if self.shutdown_started {
return;
}
self.shutdown_started = true;
self.last_fillable_frame = None; self.last_fillable_frame = None;
// 1. Stop encode thread (drops webrtc_tx → signals WebRTC thread to exit) // 1. Stop encode thread (drops webrtc_tx → signals WebRTC thread to exit)
if let Some(mut enc_thread) = self.enc_thread.take() { if let Some(mut enc_thread) = self.enc_thread.take() {