From bdc795acbfcdf2fcf05a86be99145da9f05e93c2 Mon Sep 17 00:00:00 2001 From: Chemaclass Date: Wed, 9 Sep 2026 13:03:54 +0200 Subject: [PATCH] docs(rules): correct the RANDOM and extdebug claims with measurements Two claims were relied on while planning performance work. Both are wrong, and one of the proposed corrections is wrong too. RANDOM. 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. The issue's correction -- that bash reseeds RANDOM per subshell on every supported version -- holds in a plain shell but not in the context the claim is about: plain shell, 3.00.22 / 3.2.57 / 4.4 / 5.2 / 5.3 three reads differ --parallel worker, Linux 3.0 and 5.2 three reads differ --parallel worker, macOS 3.2.57 three reads IDENTICAL So RANDOM is neither reliably shared nor reliably reseeded: it depends on the platform and on nesting depth. The ordinal scheme (#851) stands, for a stronger reason than either version of the claim. Nothing is asserted about RANDOM in a test, because pinning either direction would make one platform red; `$$` inheritance, which is the half the design rests on, is pinned. extdebug. `shopt -u extdebug` clears errtrace and functrace on newer builds, silently turning off what --strict error tracing runs on. The issue put the boundary at 5.2; it is 4.4: 3.00.22, 3.2.57, 4.0, 4.1, 4.2, 4.3 leaves them as they were 4.4, 5.2.37, 5.3.15 clears both The table now sits at the two call sites that depend on it, and a test pins it per version -- verified green on 3.0, 4.3, 4.4 and 5.2, so both sides of the boundary are exercised rather than assumed. Closes #1354 Claude-Session: https://claude.ai/code/session_01EXYWTGLjf7qM8Ru3GakDRm --- .claude/rules/architecture-map.md | 7 ++- .claude/rules/perf-fork-budget.md | 18 +++++- src/runner/context.sh | 13 ++++- src/runner/discovery.sh | 12 +++- tests/unit/project/bash_compatibility_test.sh | 58 +++++++++++++++++++ 5 files changed, 101 insertions(+), 7 deletions(-) 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" +}