Add interrupt checks to more Array methods - #1756
Open
yspbwx2010 wants to merge 1 commit into
Open
yspbwx2010 wants to merge 1 commit into
yspbwx2010 wants to merge 1 commit into
Conversation
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
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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, andArray.prototype.slice.call({length: 2**32-1})andnew 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 pathJS_CopySubArray(copyWithin, shift, unshift, splice)js_array_slice, the generic copy loop (slice, splice) and splice's delete loopjs_array_concat, the loop over a spreadable argumentjs_array_sort, the gather loop and the loop that deletes trailing holesJS_FlattenIntoArray(flat, flatMap)js_array_filljs_array_from, the array-like pathThe check is the same as in #1674. Each error exit follows the function's existing one:
goto failinjs_array_join, so the string buffer is freed.goto failin the secondjs_array_sortloop, because the value array is already freed there.return -1inJS_FlattenIntoArray.goto exceptioneverywhere 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 theqjs:set-interrupt-handlerflag. Without this change each of them runs for minutes; with it each finishes in well under a second.fillandArray.fromget 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.confreports no errors with the change, and it builds without new warnings under the CMake warning flags.Fixes #1753