node
node copied to clipboard
nodejs
[v22.x] Revert "v8: enable maglev on supported architectures"
src: create handle scope in FastInternalModuleStat
It needs a handle scope for the context handle. Since the FastApiCallbackOptions struct doesn't have isolate on it in V8 12.4 on Node.js 22, use Isolate::TryGetCurrent() to get to the isolate needed for the handle scope creation and fallback to the slow callback if no isolate is entered.
Revert "v8: enable maglev on supported architectures"
This reverts commit 1a5acd0638579e687dde128cc6d4effe3ab070d1.
Reason to revert: we have seen several crashes/unexpected JS behaviors with maglev on v22 (which ships V8 v12.4). The bugs lie in the codegen so it would be difficult for users to work around them or even figure out where the bugs are coming from. Some bugs are fixed in the upstream while some others probably remain. As v22 will get stuck with V8 v12.4 as LTS, it will be increasingly difficult to backport patches for them even if the bugs are fixed. So disable it by default on v22 to reduce the churn and troubles for users.
Refs: https://github.com/nodejs/node/issues/52797
The https://github.com/nodejs/node/labels/notable-change label has been added by @richardlau.
Please suggest a text for the release notes if you'd like to include a more detailed summary, then proceed to update the PR description with the text or a link to the notable change suggested text comment. Otherwise, the commit will be placed in the Other Notable Changes section.
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
There appears to be a consistent crash in node-test-commit-arm-debug with this PR, e.g. https://ci.nodejs.org/job/node-test-commit-arm-debug/14293/nodes=ubuntu2004_debug-arm64/consoleFull
16:46:34 not ok 2405 parallel/test-npm-install
16:46:34 ---
16:46:34 duration_ms: 1636.99700
16:46:34 severity: fail
16:46:34 exitcode: 1
16:46:34 stack: |-
16:46:34 npm warn cli npm v10.8.2 does not support Node.js v22.6.1-pre. This version of npm supports the following node versions: `^18.17.0 || >=20.5.0`. You can find the latest version at https://nodejs.org/.
16:46:34 FATAL ERROR: v8::HandleScope::CreateHandle() Cannot create a handle without a HandleScope
16:46:34 ----- Native stack trace -----
16:46:34
16:46:34 1: 0xaaaaca4e6cdc node::DumpNativeBacktrace(_IO_FILE*) [npm install]
16:46:34 2: 0xaaaaca62ff58 node::OnFatalError(char const*, char const*) [npm install]
16:46:34 3: 0xaaaacaab6190 v8::Utils::ReportApiFailure(char const*, char const*) [npm install]
16:46:34 4: 0xaaaacae61a4c v8::internal::HandleScope::Extend(v8::internal::Isolate*) [npm install]
16:46:34 5: 0xaaaacaaa67c0 v8::internal::HandleScope::CreateHandle(v8::internal::Isolate*, unsigned long) [npm install]
16:46:34 6: 0xaaaacaaf7180 v8::Object::GetCreationContext() [npm install]
16:46:34 7: 0xaaaacaaf7210 v8::Object::GetCreationContextChecked() [npm install]
16:46:34 8: 0xaaaaca63e0bc [npm install]
16:46:34 9: 0xffffa80951d8
16:46:34 Aborted (core dumped)
16:46:34 node:assert:126
16:46:34 throw new AssertionError(obj);
16:46:34 ^
16:46:34
16:46:34 AssertionError [ERR_ASSERTION]: npm install got error code 134
16:46:34 at handleExit (/home/iojs/build/workspace/node-test-commit-arm-debug/test/parallel/test-npm-install.js:62:10)
16:46:34 at handleExit (/home/iojs/build/workspace/node-test-commit-arm-debug/test/common/index.js:488:15)
16:46:34 at ChildProcess.exithandler (node:child_process:429:5)
16:46:34 at ChildProcess.emit (node:events:520:28)
16:46:34 at maybeClose (node:internal/child_process:1105:16)
16:46:34 at Socket.<anonymous> (node:internal/child_process:457:11)
16:46:34 at Socket.emit (node:events:520:28)
16:46:34 at Pipe.<anonymous> (node:net:337:12) {
16:46:34 generatedMessage: false,
16:46:34 code: 'ERR_ASSERTION',
16:46:34 actual: 134,
16:46:34 expected: 0,
16:46:34 operator: 'strictEqual'
16:46:34 }
16:46:34
16:46:34 Node.js v22.6.1-pre
16:46:34 ...
which isn't occurring on v22.x-staging: https://ci.nodejs.org/job/node-test-commit-arm-debug/14291/
Added another commit to handle the missing handle scope assertion, this fixes the npm install crash locally for me - on the other hand does this mean that the fast path had been rarely called on v22 when maglev was still enabled by default? Yikes..
I was planning to land it on the next 22 proposal, but CI seems flaky. I will need to update v22.x-staging and then you'll need to rebase on top of it. If we get a green CI tomorrow (Monday 19) I can cherry-pick it.
@RafaelGSS I am not seeing any merge conflicts, has v22.x-staging been updated?
@RafaelGSS I am not seeing any merge conflicts, has v22.x-staging been updated?
It was updated 5 days ago and I didn't see that. I had to push just one commit today. It should be fine now.
It seems to keep running into infra failure. Starting one from scratch..
This has been failing due to something wrong with the python test runner..not sure what's going on
That was fixed by https://github.com/nodejs/node/commit/05bd3cfc73c0200a905d78c62a80edd85ba879e1 on main
It seems to be a different problem (Python test runner not able to decode the tap output?)
16:30:49 Traceback (most recent call last):
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 1830, in <module>
16:30:49 sys.exit(Main())
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 1801, in Main
16:30:49 result = RunTestCases(cases_to_run, options.progress, options.j, options.flaky_tests, options.measure_flakiness)
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 996, in RunTestCases
16:30:49 return progress.Run(tasks)
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 162, in Run
16:30:49 self.RunSingle(False, 0)
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 201, in RunSingle
16:30:49 output = case.Run()
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 607, in Run
16:30:49 result = self.RunCommand(self.GetCommand(), {
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 594, in RunCommand
16:30:49 output = Execute(full_command,
16:30:49 File "/home/iojs/build/workspace/node-test-commit-custom-suites-freestyle/tools/test.py", line 820, in Execute
16:30:49 output = open(outname, encoding='utf8').read()
16:30:49 File "/usr/lib/python3.10/codecs.py", line 322, in decode
16:30:49 (result, consumed) = self._buffer_decode(data, self.errors, final)
16:30:49 UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa0 in position 216441: invalid start byte
Actually I think something is wrong with the output of test/wpt/test-encoding.js and the python runner was trying to parse its stderr/stdout. But it doesn't look related to this PR.
Trying to cross-compare with v22.x-staging https://ci.nodejs.org/job/node-test-commit/73707/
I don't think this will make it to v22.8.0. Tried to start another CI on top of the fixed v22.x-staging, but it looks like the CI is still very overloaded today, better not to disturb with another branch switch to v22.x...