diff --git a/core/core.func b/core/core.func index ce46d35..14c9ae0 100644 --- a/core/core.func +++ b/core/core.func @@ -958,7 +958,11 @@ _cs_reattach_output() { exec >"$log" 2>&1 } -_silent_exec() { +# A closed console sends SIGHUP to every process in the foreground group, and a +# program that does not handle it dies. Run the long ones through here: the +# subshell sets the signal to ignore, which exec carries into the program, so +# it finishes even though the terminal is gone. +_cs_shield_hangup() { ( trap '' HUP exec "$@" @@ -999,7 +1003,7 @@ silent() { local rc run=("$@") case "$(type -t "$1" 2>/dev/null)" in function | builtin | "") ;; - *) run=(_silent_exec "$@") ;; + *) run=(_cs_shield_hangup "$@") ;; esac if [[ "${VERBOSE:-no}" == "yes" ]]; then # Not a pipe: the command must stay in this shell so functions keep their side effects. diff --git a/core/error_handler.func b/core/error_handler.func index dcba566..dc13f16 100644 --- a/core/error_handler.func +++ b/core/error_handler.func @@ -889,7 +889,7 @@ on_hangup() { [[ -n "$log" ]] || log="${INSTALL_LOG:-${BUILD_LOG:-/dev/null}}" exec >"$log" 2>&1 export _CS_HANGUP=1 CS_ANIMATION_OK=0 _CS_HANGUP_LOG="$log" - echo "[WARN] $(date '+%Y-%m-%d %H:%M:%S') The terminal hung up (SIGHUP); continuing, output follows here. (pid $$, subshell ${BASH_SUBSHELL}, during: ${BASH_COMMAND:-?})" + echo "[WARN] $(date '+%Y-%m-%d %H:%M:%S') The terminal closed; continuing, output follows here. (pid $$, subshell ${BASH_SUBSHELL}, during: ${BASH_COMMAND:-?})" } # ============================================================================== diff --git a/docs/backends.md b/docs/backends.md index 2d09cb5..0380c5c 100644 --- a/docs/backends.md +++ b/docs/backends.md @@ -16,9 +16,10 @@ wizard calls. | [`vm-core.func`](../pve/vm-core.func) | VM creation via `qm`, including disk import and template handling | | [`vm-app.func`](../pve/vm-app.func) | Deploys an application that normally goes into an LXC container into a full VM instead | -A closed console is not an abort. SIGHUP reaches the host script and the +A closed console is not an abort. The hangup reaches the host script and the install inside the container; both carry on with their output in their logs -(`on_hangup`), external commands under `$STD` ignore the signal, and when +(`on_hangup`), the long-running commands on either side run through +`_cs_shield_hangup` so the signal cannot reach them, and when lxc-attach dies with the terminal the host waits for the container's `.done`/`.failed` markers (`_pve_wait_for_detached_install`) before it reports. Ctrl-C still aborts. diff --git a/incus/build.func b/incus/build.func index c6ba587..123622b 100644 --- a/incus/build.func +++ b/incus/build.func @@ -642,7 +642,7 @@ _incus_on_hangup() { [[ -n "$log" ]] || log="/tmp/incus-hangup-${SESSION_ID:-$$}.log" exec >"$log" 2>&1 export _CS_HANGUP=1 _CS_HANGUP_LOG="$log" CS_STDIN_OK=0 - echo "[WARN] $(date '+%Y-%m-%d %H:%M:%S') The terminal hung up (SIGHUP); continuing, output follows here." + echo "[WARN] $(date '+%Y-%m-%d %H:%M:%S') The terminal closed; continuing, output follows here." } trap 'api_exit_script' EXIT diff --git a/pve/backend.func b/pve/backend.func index a6e19f4..cc0f3ff 100644 --- a/pve/backend.func +++ b/pve/backend.func @@ -138,7 +138,7 @@ _pct_create() { local prev rc prev="$(umask)" umask 022 - pct create "$@" + _cs_shield_hangup pct create "$@" rc=$? umask "$prev" return $rc @@ -2726,7 +2726,7 @@ create_lxc_container() { if [[ ! -f "$fallback_path" ]]; then msg_info "Downloading $os_type $fallback_ver template" - if ! pveam download "$TEMPLATE_STORAGE" "$fallback_template" >>"${BUILD_LOG:-/dev/null}" 2>&1; then + if ! _cs_shield_hangup pveam download "$TEMPLATE_STORAGE" "$fallback_template" >>"${BUILD_LOG:-/dev/null}" 2>&1; then msg_error "Failed to download $os_type $fallback_ver template." return 1 fi @@ -3024,7 +3024,7 @@ create_lxc_container() { if [[ -n "$CUSTOM_TEMPLATE_PATH" ]]; then download_catalog_template "$dest" else - pveam download "$TEMPLATE_STORAGE" "$TEMPLATE" >>"${BUILD_LOG:-/dev/null}" 2>&1 || { + _cs_shield_hangup pveam download "$TEMPLATE_STORAGE" "$TEMPLATE" >>"${BUILD_LOG:-/dev/null}" 2>&1 || { msg_error "Failed to download template '$TEMPLATE' to storage '$TEMPLATE_STORAGE'" exit 222 } @@ -3446,7 +3446,7 @@ create_lxc_container() { local _dl_out="" _fresh="" for attempt in {1..3}; do msg_info "Attempt $attempt: Downloading template $TEMPLATE to $TEMPLATE_STORAGE" - _dl_out="$(pveam download "$TEMPLATE_STORAGE" "$TEMPLATE" 2>&1)" || true + _dl_out="$(_cs_shield_hangup pveam download "$TEMPLATE_STORAGE" "$TEMPLATE" 2>&1)" || true printf '%s\n' "$_dl_out" >>"${BUILD_LOG:-/dev/null}" # pveam can report a failed download and still exit 0, so only the # template in storage counts as success. @@ -3655,7 +3655,7 @@ create_lxc_container() { if [[ -n "$CUSTOM_TEMPLATE_PATH" ]]; then download_catalog_template "$LOCAL_TEMPLATE_PATH" else - pveam download local "$TEMPLATE" >>"${BUILD_LOG:-/dev/null}" 2>&1 + _cs_shield_hangup pveam download local "$TEMPLATE" >>"${BUILD_LOG:-/dev/null}" 2>&1 fi msg_ok "Template downloaded to local" else diff --git a/tests/console-hangup.sh b/tests/console-hangup.sh index ea447f1..39cb90e 100644 --- a/tests/console-hangup.sh +++ b/tests/console-hangup.sh @@ -6,7 +6,7 @@ set -uo pipefail CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" -eval "$(awk '/^(silent|_silent_exec|_cs_reattach_output)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" +eval "$(awk '/^(silent|_cs_shield_hangup|_cs_reattach_output)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" eval "$(awk '/^(on_hangup|on_exit|_eh_quiet_spinner)[(][)] [{]/,/^}/' "$CORE_DIR/core/error_handler.func")" eval "$(awk '/^_pve_wait_for_detached_install[(][)] [{]/,/^}/' "$CORE_DIR/pve/backend.func")" @@ -53,7 +53,7 @@ rc=$? expect "the shell survives the hangup" 0 "$rc" expect "nothing more reaches the terminal" "" "$out" expect "later output lands in the log" 1 "$(grep -c 'after hangup' "$LOG")" -expect "the hangup is noted in the log" 1 "$(grep -c SIGHUP "$LOG")" +expect "the hangup is noted in the log" 1 "$(grep -c 'The terminal closed' "$LOG")" expect "the run knows it is detached" 1 "$(grep -c 'flag=1' "$LOG")" # on_exit(): a clean install leaves .done for the host; a failed one does not. @@ -137,5 +137,33 @@ expect "and not back to the original target" 0 "$(grep -c 'after the redirected expect "without a hangup nothing is redirected" 0 "$(grep -c 'no hangup' "$AFTER" 2>/dev/null)" expect "it stays where it was" 1 "$(grep -c 'no hangup' "$TEST_DIR/orig2.log" 2>/dev/null)" +# The telemetry service decides what counts as a user abort by searching the +# log text, so a word in one of our own messages is enough to file a failure +# as "the operator pressed Ctrl-C". A closed console is not that, and 81 of +# 300 aborted records were this message rather than a person. +CLASSIFIER_TOKENS="sigint ctrl+c ctrl-c sighup aborted by user user abort cancelled by user no changes have been made" +hangup_text="$(awk '/^on_hangup[(][)] [{]/,/^}/' "$CORE_DIR/core/error_handler.func") +$(awk '/^_incus_on_hangup[(][)] [{]/,/^}/' "$CORE_DIR/incus/build.func")" +hits=0 +while IFS= read -r token; do + [[ -n "$token" ]] || continue + if printf '%s' "${hangup_text,,}" | grep -qF "$token"; then + echo " token in a hangup message: $token" >&2 + hits=$((hits + 1)) + fi +done <<<"$(printf '%s +' $CLASSIFIER_TOKENS | sed 's/^ *//')" +expect "no hangup message reads as a user abort" 0 "$hits" +expect "both hangup handlers were found" 2 "$(grep -c 'continuing, output follows here' <<<"$hangup_text")" + +# The helper the long host commands go through has to survive the signal. +: >"$LOG" +_cs_shield_hangup sh -c 'kill -HUP $$; echo shielded >>"'"$LOG"'"' +expect "a shielded command survives SIGHUP" 0 "$?" +expect "and ran to the end" 1 "$(grep -c shielded "$LOG")" +unshielded_rc=0 +{ sh -c 'kill -HUP $$; echo unshielded' >/dev/null 2>&1 || unshielded_rc=$?; } 2>/dev/null +expect "without the shield the same command dies" 129 "$unshielded_rc" + echo "$passed passed, $failed failed" ((failed == 0)) diff --git a/tests/failed-command-output.sh b/tests/failed-command-output.sh index e454509..3e6c486 100644 --- a/tests/failed-command-output.sh +++ b/tests/failed-command-output.sh @@ -2,7 +2,7 @@ set -uo pipefail CORE_DIR="$(cd -- "$(dirname -- "${BASH_SOURCE[0]}")/.." && pwd)" -eval "$(awk '/^(silent|set_std_mode|_silent_exec|_cs_reattach_output)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" +eval "$(awk '/^(silent|set_std_mode|_cs_shield_hangup|_cs_reattach_output)[(][)] [{]/,/^}/' "$CORE_DIR/core/core.func")" eval "$(awk '/^_eh_print_failed_output[(][)] [{]/,/^}/' "$CORE_DIR/core/error_handler.func")" TEST_DIR="$(mktemp -d)" diff --git a/ui/API.txt b/ui/API.txt index 32fea30..82899f4 100644 --- a/ui/API.txt +++ b/ui/API.txt @@ -15,6 +15,7 @@ _cs_run_os_section _cs_run_update _cs_runtime_cache_dir _cs_scripts_ref_line +_cs_shield_hangup _cs_stdin_source _dev_step_end _dev_step_key @@ -51,7 +52,6 @@ _pve_version _pveam_available _pveam_update _sanitize_value -_silent_exec _stop_container_if_installing _telemetry_report_exit _tm_clean_log