Check signals in loops that dispatch no commands

[ki3v]

The problem

A pending signal only aborts a running script at the per-command check (Jim_CheckSignal, right after JimInvokeCommand in Jim_EvalObj) and at catch/try entry.

But a loop whose body and condition dispatch no command reaches neither, so catch -signal { ... } with an incoming signal like SIGINT or ALRM can't break out of loops such as:

while 1 {}
while {$i < $n} {incr i}     ;# the incr fast-path dispatches nothing
for {} {1} {} {}

The empty-body and incr fast-paths in Jim_EvalObj return before the command loop, and the C-level loop constructs (while, for, loop, foreach/lmap) don't check signals themselves, so these spin until the process is killed (with an untrappable signal).

Degenerate cases, maybe. But they can arise in novice code, generated code, etc.

A signal already breaks any loop that dispatches a command each iteration in the body or the condition. while {[ready]} {...}, or even while {[expr {1+1}]} {}, are already interruptible because of the command substitution in the condition.

The issue is loops where neither the condition nor the body dispatches a command, like a pure-expression condition with an empty or fast-path body such as the ones I list in the block above.

Reproduction

$ jimsh -e 'set i 0; signal handle ALRM; alarm 0.3; puts [catch -signal { while {$i < 4000000000} {incr i} } msg]/$msg'
5/SIGALRM

With the proposed change it prints 5/SIGALRM.

On master that same line produces no output: the loop never checks the signal and runs for tens of seconds, completing the loop instead of breaking out at the ALRM.

The patch

I propose adding a Jim_CheckSignal test at the iteration boundary of while, for (both the general and optimized paths), loop, and foreach/lmap. A pending signal breaks the loop with JIM_SIGNAL, propagated by the existing loop return-code handling. Jim_CheckSignal is the existing macro:

#define Jim_CheckSignal(i) ((i)->signal_level && (i)->sigmask)

Testing

make test, full suite, identical on master and with the patch:

Totals: Total 5908  Passed 5736  Skipped 172  Failed 0

Performance

The concern is the fast loop paths, so we don't make them un-fast. The added check is one signal_level indirect load and a not-taken branch per iteration.

Across 5 runs interleaving master and patched and totalling 15 minutes:

for {set i 0}  {$i < 3000000000} {incr i} {}
# master 9.28 ns/iter, patch +0.31 (+3.3%)
# runs +0.333 +0.244 +0.313 +0.355 +0.286 - jitter ~0.04 (0.43%)

set i 0; while {$i < 1500000000} {incr i}
# master ~19.9 ns/iter, patch +0.16 (+0.8%)
# runs +0.099 +0.339 +0.148 +0.303 -0.069 - jitter ~0.15 (0.75%)

set i 0; while {$i <  500000000} {set x 1; incr i}
# master 60.7 ns/iter, patch +0.50 (+0.8%)
# runs +0.857 +0.790 +0.424 +0.649 -0.221 - jitter ~0.39 (0.64%)

The last two come in under 1%, but some of the patched runs came in faster due to measurement jitter, and you can see that at any rate we are pulling numbers out of noise. I don't claim this to be a rigorous quantitative analysis.

For useful loops, I expect the cost is genuinely negligible.

Context

I'm experimenting with embedding Jim Tcl into a microcontroller project (Teensy 4.1 platform, bare metal) as a configuration shell and for user script callbacks in the application.

Here, I'm overriding Jim_CheckSignal so it also calls a hook where I can pump my background services loop tightly. These empty loop cases could lead to starvation on my setup and eventually a watchdog reboot.

I also noticed they cause signal deafness on the OS-hosted case, so I wanted to propose this patch back upstream.

[ki3v]

I noticed a CI fail, so I wanted to check to make sure I didn't send you a Windows-incompatible patch. I ran it past Claude Fable, as I'm on the road at a trade show and don't have my whole environment with me or time for a deep dive myself this week. I wanted to offer the results in case they are useful.

Do please note, even back at my desk I don't have a Windows build env, so while this "fix" is targeted on Windows, it's only been tested as not regressing on macOS (and assuming Linux would be the same). Discount accordingly.

Executive summary: it seems the failing test is one you've been having trouble with on Windows. Fable believes that differences in timer granularity on the GitHub Windows CI containers combined with Windows CI build details are exposing a real issue and not a flawed test: an underlying issue in vwait sometimes calls the script even when no event fired, especially when woken early due to imprecise timers in certain build configurations.

While I did a superficial review, I can't quite vouch for the analysis (unlike the PR), but it sounds plausible enough to me that I'm forwarding it on in case it is helpful to you.

Patch is viewable at: ki3v/jimtcl@64bb005

Fable's explanation follows:

---

The build-windows failure is event-15.1, and it appears unrelated to this
PR's diff (which touches only the while/for/loop/foreach back-edges —
and is inert on the Windows build anyway, since the signal extension isn't
compiled there). The mechanism, each step checked in source:

1. The mingw32 CI build ends up without HAVE_SELECT: the auto.def probe
   (cc-check-functions ... select ...) is a bare link test, and on mingw
   select needs -lws2_32, so the probe never links. The eventloop
   therefore waits with usleep() (the #else path in Jim_ProcessEvents).
2. mingw's usleep rides on Sleep() — millisecond truncation, scheduler-tick
   granularity — while the timer due-test compares microseconds. Near a
   deadline the loop can wake with the timer not yet due, so
   Jim_ProcessEvents returns having processed zero events.
3. vwait evaluates its script on every Jim_ProcessEvents return,
   including zero-event ones. event-15.1 breaks after 5 script evaluations
   but asserts 2–5 waiter firings — on a coarse-sleep host the 5-eval budget
   can be spent on empty wakeups after a single firing, hence "Expected 2-5
   iterations, got 1". On Linux the sleep is precise, returns ≈ events, and
   it passes. This would also explain why 0806f30 and f6714f5 each helped
   without converging: the wakeup:event ratio is unbounded on coarse-timer
   hosts, so no fixed window holds.

Since the docs say "If 'script' is given it is evaluated after each event",
the minimal fix is to gate the evaluation on events actually having been
processed: ki3v/jimtcl@64bb005 — if (scriptObjPtr && rc > 0), +6/−2,
five of the six added lines comment.

Tested on macOS (the HAVE_SELECT / precise-timer case): full suite green —
event.test 22/22, no regressions. With the gate, script evaluations pair
1:1 with processed event batches, so event-15.1's original exact
{5 {0 1 2 3 4}} expectation from d593124 should hold again everywhere —
though I couldn't test on Windows itself.

If per-return evaluation is actually the intended semantic (the doc wording
is genuinely open to either reading), the test-side alternative is to count
waiter firings instead of script evaluations (break on
[llength $result] >= 3, expect an exact {0 1 2}).

One adjacent observation, offered only in case it's useful: the missed
select probe also means the Windows build has no file-event support at
all, and a winsock-aware probe would then meet Winsock's requirement of at
least one socket in select's fd sets, which the empty-set sleep idiom
doesn't satisfy.

— Claude Fable (analysis not yet vetted by Dave)

[ki3v]

I am not very up on GitHub Actions, but with Claude's help I enabled the CI on my fork and the Windows CI build/test seems to pass with the patch referenced above.