Skip to content

Add interrupt checks to more Array methods - #1756

Open
yspbwx2010 wants to merge 1 commit into
quickjs-ng:masterfrom
yspbwx2010:array-interrupt-checks
Open

yspbwx2010 wants to merge 1 commit into
quickjs-ng:masterfrom
yspbwx2010:array-interrupt-checks

Conversation

@yspbwx2010

Copy link
Copy Markdown

Follow-up to #1674 (issue #1672), which added js_poll_interrupts() to the scan loops of the iterating Array methods. A number of other Array loops walk [0, length) in C without calling into JS, so they still never consult the interrupt handler. On a sparse array or an array-like with a large length, Ctrl-C in the REPL has no effect on them. For example, Array.prototype.join.call({length: 2**32-1}, "") runs for four to five minutes, and Array.prototype.slice.call({length: 2**32-1}) and new Array(2**32-1).reverse() behave the same way.

This adds a poll at the top of those loops:

  • js_array_join (join, toLocaleString)
  • js_array_reverse, generic path
  • JS_CopySubArray (copyWithin, shift, unshift, splice)
  • js_array_slice, the generic copy loop (slice, splice) and splice's delete loop
  • js_array_concat, the loop over a spreadable argument
  • js_array_sort, the gather loop and the loop that deletes trailing holes
  • JS_FlattenIntoArray (flat, flatMap)
  • js_array_fill
  • js_array_from, the array-like path

The check is the same as in #1674. Each error exit follows the function's existing one:

  • goto fail in js_array_join, so the string buffer is freed.
  • goto fail in the second js_array_sort loop, because the value array is already freed there.
  • return -1 in JS_FlattenIntoArray.
  • goto exception everywhere else.

Tests: eight negative tests next to the existing ones in tests/bug1672/, for join, reverse, copyWithin, slice, splice, concat, sort and flat. They have the same form as the existing ones: a one-liner on {length: 2**32-1} with the qjs:set-interrupt-handler flag. Without this change each of them runs for minutes; with it each finishes in well under a second.

fill and Array.from get no test of this kind. They allocate per element and run-test262 sets no memory limit, so a regression would show up as unbounded memory growth rather than a hang. Under a memory limit, the out-of-memory InternalError would make such a test pass anyway.

run-test262 -c tests.conf reports no errors with the change, and it builds without new warnings under the CMake warning flags.

Fixes #1753

Follow-up to quickjs-ng#1674. join, reverse, copyWithin, concat, flat, slice,
splice and sort still walk [0, length) in C without calling into JS, so
on an array-like with a large length neither Ctrl-C in the REPL nor an
embedder's interrupt handler could stop them. Poll at the top of each
loop, as quickjs-ng#1674 did, with a test per method in tests/bug1672/.

Fixes quickjs-ng#1753

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

This branch has not been deployed

No deployments
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.

More Array builtins never consult the interrupt handler (follow-up to #1672)

1 participant