diff --git a/.claude/rules/architecture-map.md b/.claude/rules/architecture-map.md index 76ae2395..cb05c129 100644 --- a/.claude/rules/architecture-map.md +++ b/.claude/rules/architecture-map.md @@ -124,8 +124,11 @@ shell (or, in parallel, in per-test `.result` files aggregated at the end). ## Cross-cutting invariants - **Bash 3.0 floor** (`.claude/rules/bash-style.md`): no `[[`, `declare -A`, - `${var,,}`, `BASHPID`, negative indices. Subshells share `$$` and `RANDOM` - state — you cannot make a per-worker unique token without a fork (`mktemp`). + `${var,,}`, `BASHPID`, negative indices. Subshells share `$$` — you cannot + make a per-worker unique token without a fork (`mktemp`) or an ordinal + assigned before the fork. `RANDOM` is no help either: whether a subshell + reseeds it depends on the platform and the nesting depth (measured in + perf-fork-budget.md), so it is unreliable in both directions (#1354). - **Return-slot pattern** (`_BASHUNIT___OUT` globals) instead of `$()` captures on hot paths — bash-style.md documents it; `local` is dynamically scoped, so helpers must not write caller-named variables. diff --git a/.claude/rules/perf-fork-budget.md b/.claude/rules/perf-fork-budget.md index 3a5ee8d9..559abc96 100644 --- a/.claude/rules/perf-fork-budget.md +++ b/.claude/rules/perf-fork-budget.md @@ -271,8 +271,22 @@ ordinal the single-threaded dispatcher assigns just before each `&` (the fork inherits it), so it costs **no** `mktemp` + `mv` per test (#851; was 10 `mktemp` + 10 `mv`). This replaced the old sanitized-test-name scheme, whose deterministic names could collide (different provider args sanitize identically) -because Bash 3 workers can't mint a unique token — subshells inherit `$$` and -the `RANDOM` state, and `BASHPID` is 4.0+; an ordinal sidesteps that entirely. +because Bash 3 workers can't mint a unique token — subshells inherit `$$`, and +`BASHPID` is 4.0+; an ordinal sidesteps that entirely. + +This file used to add "and the `RANDOM` state" to that list. Measured, `RANDOM` +is neither reliably shared nor reliably reseeded (#1354): + +| Context | three consecutive `$( )` reads | +|---|---| +| Plain shell — 3.00.22, 3.2.57, 4.4, 5.2, 5.3 | differ | +| `--parallel` worker — Linux 3.0, 5.2 | differ | +| `--parallel` worker — macOS 3.2.57 | **identical** | + +So it depends on the platform *and* on how deeply nested the subshell is. Do +not build on it in either direction, and do not reuse "`RANDOM` is shared" as a +premise — it is right about the conclusion for the wrong reason. `$$` is the +part that genuinely is inherited. `wait_for_job_slot` already uses `wait -n` on Bash 4.3+ and an adaptive sleep-poll fallback — don't "fix" it. The spinner forks `sleep` ~1/s on non-tty; not worth chasing. diff --git a/src/runner/context.sh b/src/runner/context.sh index c2b55ad9..6421e7a0 100644 --- a/src/runner/context.sh +++ b/src/runner/context.sh @@ -118,8 +118,17 @@ function bashunit::runner::resolve_test_location() { local fn_name=$2 # Enable extdebug only inside the command-substitution subshell so it never - # leaks into the parent shell — globally toggling extdebug interferes with - # `set -e`/DEBUG-trap behavior under --strict. + # leaks into the parent shell. Turning it back off is not symmetric, and the + # asymmetry moved inside the supported range (#808, #1354): + # + # 3.00.22, 3.2.57, 4.0, 4.1, 4.2, 4.3 `shopt -u extdebug` leaves + # errtrace/functrace as they were + # 4.4, 5.2.37, 5.3.15 it clears both, even if they were + # on beforehand + # + # So from 4.4 on, disabling extdebug in this shell silently clears `set -E` + # and `set -T` -- which is exactly what --strict error tracing runs on. A + # rewrite that drops this subshell has to save and restore both. local def line="" def="$(shopt -s extdebug; declare -F "$fn_name" 2>/dev/null)" || true diff --git a/src/runner/discovery.sh b/src/runner/discovery.sh index e5fb8eac..5ce200fd 100644 --- a/src/runner/discovery.sh +++ b/src/runner/discovery.sh @@ -328,7 +328,17 @@ function bashunit::runner::functions_for_script() { local all_fn_names="$2" # Resolve " " for the given names, enabling extdebug only - # inside the capture subshell so the caller's setting is untouched. + # inside the capture subshell so the caller's setting is untouched. Turning + # it back off is not symmetric, and the asymmetry moved inside the supported + # range (#808, #1354): + # + # 3.00.22, 3.2.57, 4.0, 4.1, 4.2, 4.3 `shopt -u extdebug` leaves + # errtrace/functrace as they were + # 4.4, 5.2.37, 5.3.15 it clears both, even if they were + # on beforehand + # + # So from 4.4 on, disabling extdebug here would silently clear `set -E` and + # `set -T`, which is what --strict error tracing runs on. local declarations # shellcheck disable=SC2086 declarations=$( diff --git a/tests/unit/project/bash_compatibility_test.sh b/tests/unit/project/bash_compatibility_test.sh index 757e7f3f..315404af 100644 --- a/tests/unit/project/bash_compatibility_test.sh +++ b/tests/unit/project/bash_compatibility_test.sh @@ -496,3 +496,61 @@ EOF assert_empty "$failures" } + + +# --- shell facts the code is built on ------------------------------------- +# +# Both of these were stated wrongly in the rules files and relied on while +# planning performance work (#1354). A claim about the shell is worth a test +# precisely because nothing else notices when it stops being true. + +# The rules said a Bash 3 subshell inherits the `RANDOM` state, and gave that +# as the reason a --parallel worker cannot mint a unique token. Measured, the +# truth is messier than either that or its correction: +# +# plain shell, every supported version three `$( )` reads differ +# --parallel worker, Linux 3.0 and 5.2 three `$( )` reads differ +# --parallel worker, macOS 3.2.57 three `$( )` reads are IDENTICAL +# +# So RANDOM is neither reliably shared nor reliably reseeded: it depends on the +# platform and on how deeply nested the subshell is. Nothing may depend on it +# either way, which is why there is no assertion about it here -- pinning either +# direction would just make one platform red. The ordinal scheme (#851) stands, +# now for a stronger reason than the one originally written down (#1354). +# +# What IS stable is the half the design actually rests on: a subshell inherits +# `$$`, so a token built from it repeats across workers. +function test_a_subshell_inherits_the_parent_pid() { + assert_same "$$" "$(printf '%s' "$$")" +} + +# `shopt -s extdebug` turns on errtrace and functrace everywhere. Turning it +# back off does not behave the same across the supported range: up to 4.3 it +# leaves them as they were, from 4.4 it clears both. That is the concrete shape +# of the hazard #808 works around, and anything that stops doing this inside a +# subshell has to save and restore them. +function test_unsetting_extdebug_clears_error_tracing_from_bash_44() { + local state + state=$( + set -E + set -T + shopt -s extdebug + shopt -u extdebug + e=off + t=off + # Parameter expansion, not `case`: a `)` in a case pattern inside `$( )` + # is a parse error on Bash 3.2, which closes the substitution early. + if [ "${-#*E}" != "$-" ]; then e=on; fi + if [ "${-#*T}" != "$-" ]; then t=on; fi + echo "$e/$t" + ) + + local expected="on/on" + if [ "${BASH_VERSINFO[0]:-0}" -gt 4 ]; then + expected="off/off" + elif [ "${BASH_VERSINFO[0]:-0}" -eq 4 ] && [ "${BASH_VERSINFO[1]:-0}" -ge 4 ]; then + expected="off/off" + fi + + assert_same "$expected" "$state" +}