Skip to content

Issue #832: bound TermWidth probes and honor $COLUMNS#833

Open
Yaminyam wants to merge 3 commits into
TACC:mainfrom
Yaminyam:fix-832-termwidth-hang-fallbacks
Open

Issue #832: bound TermWidth probes and honor $COLUMNS#833
Yaminyam wants to merge 3 commits into
TACC:mainfrom
Yaminyam:fix-832-termwidth-hang-fallbacks

Conversation

@Yaminyam

Copy link
Copy Markdown

Fixes #832.

Implements both fixes suggested in the issue discussion, scoped to tools/TermWidth.lua so the global capture() callers are unaffected (per @mrcawood's note in #832).

Changes

  • l_askSystem honors $COLUMNS before the stty/tput probes. Interactive shells already export COLUMNS from their SIGWINCH handler, so the common path skips the subprocess probes entirely. This is the cheapest fix and also covers wedged-PTY environments where the shell still has a sensible COLUMNS.

  • stty size / tput cols capture() calls are wrapped with timeout (GNU coreutils) when the binary is on PATH. Detection happens once, lazily, and the result is cached. A broken PTY whose ioctl(TIOCGWINSZ) never returns can no longer hang module indefinitely — the probe is killed after one second and the next fallback (or the caller's default) is used. When timeout is not on PATH the prefix is empty, so the pre-fix behavior is preserved on systems without coreutils (no regression).

Both changes are local to TermWidth.lua; capture() and its other callers are untouched.

Regression test

Adds rt/termWidthFallback/. It installs a fake stty/tput on PATH that report a sentinel width (999) and log every invocation, then runs module -D avail twice — once with $COLUMNS set, once without. The log is expected to be empty in the COLUMNS-set scenario (subprocess probes skipped) and non-empty when COLUMNS is unset (probes executed and returned the sentinel).

Note on goldens: my local macOS doesn't currently have a full Lmod build environment ready (lua + luaposix + luafilesystem + Hermes), so the out.txt / err.txt for the new RT aren't included in this commit yet. Happy to follow up with a bless commit once CI shows the actual output, or if a maintainer can run tm -bless rt/termWidthFallback on their end I can fold the result in.

Verification done

  • Reproduced the original hang in the issue's docker setup (stty sizesleep 999999), and verified that module use ... returns normally after this patch, with both LMOD_TERM_WIDTH unset and COLUMNS unset (relying on the timeout wrapper) — fallback to default 80 columns.
  • With COLUMNS=120 set, confirmed the fake stty/tput probes are not invoked at all.
  • Re-read the existing call sites of capture() to confirm nothing outside TermWidth.lua changes behavior.

Yaminyam and others added 3 commits June 25, 2026 15:14
Two changes scoped to tools/TermWidth.lua so the global capture() callers
are unaffected:

* l_askSystem now reads $COLUMNS before the stty/tput subprocess probes.
  Interactive shells already export it from their SIGWINCH handler, so
  the common case skips the probes entirely. This also covers wedged-PTY
  environments where the shell happens to have a sensible value.

* The remaining stty size / tput cols capture() calls are wrapped with
  `timeout` when the GNU coreutils binary is on PATH, detected once and
  cached. A broken PTY whose ioctl(TIOCGWINSZ) never returns can no
  longer hang `module` indefinitely; the probe is killed after one
  second and the next fallback (or the caller-supplied default) is used.
  When `timeout` is not available the prefix is empty, preserving the
  pre-fix behavior so no regression is introduced.

Adds rt/termWidthFallback to lock the behavior in. It installs a fake
stty/tput on PATH that report a sentinel width and log each invocation,
then runs `module -D avail` with and without $COLUMNS set. The log is
expected to stay empty in the COLUMNS-set scenario (subprocess probes
skipped) and to be non-empty when COLUMNS is unset (probes executed and
returned the sentinel). Golden out.txt/err.txt for the new RT will be
generated via the standard bless flow once CI runs the test.
Replace GNU coreutils timeout with a TermWidth-local fork/pipe capture
bounded by SIGALRM (1s). Read $COLUMNS in TermWidth() before stty/tput
probes so interactive shells skip subprocess calls entirely.

Extend rt/termWidthFallback with Hermes-safe assertions and a wedged-stty
scenario. Golden files blessed.
…luaposix 35

The SIGALRM-bounded probe indexed `posix.signal.kill` on the merged `posix`
table. On luaposix 35 (the version packaged for EL9 / Rocky / Alma 9)
`posix.signal` is the signal() *function*, not a table, so this raised
"attempt to index a function value (field 'signal')" at require time and
Lmod failed to start entirely. (The same file already called
`posix.signal(SIGALRM, ...)` as a function elsewhere, so the two uses were
inconsistent.)

Require the `posix.signal` submodule and take kill / signal / SIGALRM /
SIGKILL from it, which is a table exposing both the functions and the
constants across supported luaposix versions.

Verified on Rocky 9 (lua-posix 35.0): before this change `lmod --version`
aborts at TermWidth.lua load; after it, Lmod loads and the TACCGH-832 wedged-stty
reproducer is bounded to ~1s (falls back to the 80-column default) with
$COLUMNS and LMOD_TERM_WIDTH also honored.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@Yaminyam

Copy link
Copy Markdown
Author

Thanks @mrcawood — pulled the updated branch and tested it in a clean Rocky Linux 9 container (lua-posix 35.0, the EL9-packaged luaposix). Two findings: the width-bounding logic works, but the branch as-is (dbca31f7f) does not load on EL9. I've pushed a small follow-up (d097496) that fixes the load and kept everything else as you wrote it.

1. Load-time break on luaposix 35 (EL9/Rocky/Alma 9)

dbca31f7f indexes posix.signal.kill on the merged posix table:

lua: tools/TermWidth.lua:41: attempt to index a function value (field 'signal')
stack traceback:
	tools/TermWidth.lua:41: in main chunk
	[C]: in function 'require'
	src/lmod.in.lua:108: in main chunk

On luaposix 35 posix.signal is the signal() function, not a table, so .kill errors at require time and lmod --version aborts before doing anything — Lmod won't start at all on EL9. (Interestingly the same file already calls posix.signal(SIGALRM, ...) as a function at lines 117/134, so the two uses were inconsistent.)

My follow-up requires the posix.signal submodule and pulls kill / signal / SIGALRM / SIGKILL from it — a table exposing both the functions and the constants across the luaposix versions Lmod supports:

local psignal = require("posix.signal")
local kill    = psignal.kill
local SIGALRM = psignal.SIGALRM
local SIGKILL = psignal.SIGKILL
...
local oldSig = psignal.signal(SIGALRM, function() ... end)
...
psignal.signal(SIGALRM, oldSig)

2. With that fix, the bounding works

Reproducer = my original wedged stty from the issue (hang on size), timing real lmod bash avail against a 15 s hard kill:

case elapsed result
naive stty size under the wrapper (control) killed at 3 s wrapper genuinely hangs
wedged stty, COLUMNS and LMOD_TERM_WIDTH unset ~1 s SIGALRM fires → 80-col default ✅
wedged stty, COLUMNS=120 0 s probes skipped entirely ✅
wedged stty, LMOD_TERM_WIDTH=100 0 s short-circuits as before ✅

Before the fix, case 1 hung indefinitely; after, it's bounded to the 1 s alarm. $COLUMNS and LMOD_TERM_WIDTH are both honored.

The follow-up commit is on the branch. 1 s felt fine in my docker / ttyd / etty setups. Happy to adjust anything else you'd like before merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Lmod hangs indefinitely in TermWidth when stty/tput blocks (broken PTY / unresponsive TTY environments)

2 participants