Fix compat with quickjs ng 0.16 - #1115
Merged
Merged
Conversation
3 tasks
bnoordhuis
reviewed
Aug 16, 2026
xeioex
force-pushed
the
fix-compat-with-quickjs-ng-0.16
branch
from
August 16, 2026 20:53
eec38c7 to
4dc76b8
Compare
Two separate problems prevented njs from working with the recent quickjs-ng releases. JS_NewArrayBuffer() has grown a "max_len" argument and its free callback has become a realloc one. The calls are wrapped now, the callback has to implement the whole realloc contract, because transfer() reallocates the data even for a buffer which is not resizable. The class ids used by njs started at 64, while the engine reserves the ids below JS_CLASS_INIT_COUNT, which has reached 67. JS_NewClass() failed with a collision and the context could not be created. The ids are moved above the engine range and the remaining unchecked JS_NewClass() calls now report the failure. This fixes nginx#1113 issue on Github.
An error created outside of a JS frame, from a fetch() completion callback for instance, records an empty stack. Up to QuickJS-NG 0.15.1 deleting the recorded "stack" property let the engine attach a real one where the error was thrown. Since 0.16.0 the stack is kept in the error internal data behind an accessor, the property to delete no longer exists and the empty stack became final. JS_NewError() is JS_NewObjectClass() plus the stack recording, and the first half alone is reachable through the public API. Creating the object that way records nothing, so the engine attaches the stack at the throw site. This works the same way for every supported engine, hence the feature test is gone.
xeioex
force-pushed
the
fix-compat-with-quickjs-ng-0.16
branch
from
August 17, 2026 16:40
4dc76b8 to
d000dcc
Compare
A static library has to be asked for explicitly since 0.16.0, otherwise the library is installed as a shared object outside of the loader search path, the configure test binary fails to start and the library is reported as not working.
xeioex
force-pushed
the
fix-compat-with-quickjs-ng-0.16
branch
from
August 20, 2026 15:55
d000dcc to
75e2606
Compare
VadimZhestikov
approved these changes
Aug 20, 2026
VadimZhestikov
left a comment
VadimZhestikov
left a comment
Contributor
There was a problem hiding this comment.
Looks good
Minor observations (non-blocking)
- The 128 offset is heuristic, not future-proof. It's JS_CLASS_INIT_COUNT (67)
plus headroom, and the comment says as much. If a future quickjs-ng grows
JS_CLASS_INIT_COUNT past 128, or heavy dynamic class allocation reaches 128
before njs registers, the collision returns. It's the one thing to re-check on
future engine bumps. A compile-time guard would be ideal but JS_CLASS_INIT_COUNT
isn't a stable public constant, so the pragmatic fixed offset is acceptable. - Commit 2 also changes behavior on older engines (it removed the version gate),
now always using the JS_NewObjectClass path. It's sound by construction (the
JS_NewError == JS_NewObjectClass + stack invariant holds across versions) and CI
covers older engines — I verified it on 0.16.2 but didn't run an older build. - External-buffer + transfer() at the same length moves the pool-backed pointer
into a new ArrayBuffer (engine won't free it — correct). The "retained past its
pool" lifetime concern there is the pre-existing external-buffer class (the
#1112/N2 territory), not introduced by this PR.
github-project-automation
Bot
moved this from New
to In Review
in NGINX OSS Unified Workspace
github-project-automation
Bot
moved this from In Review
to Done
in NGINX OSS Unified Workspace
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
4 participants
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.
No description provided.