75d2caf451
* chore: update to Node.js v18 * child_process: improve argument validation https://github.com/nodejs/node/pull/41305 * bootstrap: support configure-time user-land snapshot https://github.com/nodejs/node/pull/42466 * chore: update GN patch * src: disambiguate terms used to refer to builtins and addons https://github.com/nodejs/node/pull/44135 * src: use a typed array internally for process._exiting https://github.com/nodejs/node/pull/43883 * chore: lib/internal/bootstrap -> lib/internal/process * src: disambiguate terms used to refer to builtins and addons https://github.com/nodejs/node/pull/44135 * chore: remove redudant browserGlobals patch * chore: update BoringSSL patch * src: allow embedder-provided PageAllocator in NodePlatform https://github.com/nodejs/node/pull/38362 * chore: fixup Node.js crypto tests - https://github.com/nodejs/node/pull/44171 - https://github.com/nodejs/node/pull/41600 * lib: add Promise methods to avoid-prototype-pollution lint rule https://github.com/nodejs/node/pull/43849 * deps: update V8 to 10.1 https://github.com/nodejs/node/pull/42657 * src: add kNoBrowserGlobals flag for Environment https://github.com/nodejs/node/pull/40532 * chore: consolidate asar initialization patches * deps: update V8 to 10.1 https://github.com/nodejs/node/pull/42657 * deps: update V8 to 9.8 https://github.com/nodejs/node/pull/41610 * src,crypto: remove AllocatedBuffers from crypto_spkac https://github.com/nodejs/node/pull/40752 * build: enable V8's shared read-only heap https://github.com/nodejs/node/pull/42809 * src: fix ssize_t error from nghttp2.h https://github.com/nodejs/node/pull/44393 * chore: fixup ESM patch * chore: fixup patch indices * src: merge NativeModuleEnv into NativeModuleLoader https://github.com/nodejs/node/pull/43824 * [API] Pass OOMDetails to OOMErrorCallback https://chromium-review.googlesource.com/c/v8/v8/+/3647827 * src: iwyu in cleanup_queue.cc * src: return Maybe from a couple of functions https://github.com/nodejs/node/pull/39603 * src: clean up embedder API https://github.com/nodejs/node/pull/35897 * src: refactor DH groups to delete crypto_groups.h https://github.com/nodejs/node/pull/43896 * deps,src: use SIMD for normal base64 encoding https://github.com/nodejs/node/pull/39775 * chore: remove deleted source file * chore: update patches * chore: remove deleted source file * lib: add fetch https://github.com/nodejs/node/pull/41749 * chore: remove nonexistent node specs * test: split report OOM tests https://github.com/nodejs/node/pull/44389 * src: trace fs async api https://github.com/nodejs/node/pull/44057 * http: trace http request / response https://github.com/nodejs/node/pull/44102 * test: split test-crypto-dh.js https://github.com/nodejs/node/pull/40451 * crypto: introduce X509Certificate API https://github.com/nodejs/node/pull/36804 * src: split property helpers from node::Environment https://github.com/nodejs/node/pull/44056 * https://github.com/nodejs/node/pull/38905 bootstrap: implement run-time user-land snapshots via --build-snapshot and --snapshot-blob * lib,src: implement WebAssembly Web API https://github.com/nodejs/node/pull/42701 * fixup! deps,src: use SIMD for normal base64 encoding * fixup! src: refactor DH groups to delete crypto_groups.h * chore: fixup base64 GN file * fix: check that node::InitializeContext() returns true * chore: delete _noBrowserGlobals usage * chore: disable fetch in renderer procceses * dns: default to verbatim=true in dns.lookup() https://github.com/nodejs/node/pull/39987 Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
96 lines
5.3 KiB
Diff
96 lines
5.3 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Jeremy Rose <nornagon@nornagon.net>
|
|
Date: Fri, 11 Mar 2022 11:17:29 -0800
|
|
Subject: be compatible with cppgc
|
|
|
|
This fixes a crash that happens sporadically when Node is used in the same V8
|
|
Isolate as Blink. For example:
|
|
|
|
#
|
|
# Fatal error in ../../v8/src/objects/js-objects-inl.h, line 306
|
|
# Debug check failed: static_cast<unsigned>(index) < static_cast<unsigned>(GetEmbedderFieldCount()) (1 vs. 1).
|
|
#
|
|
#
|
|
#
|
|
#FailureMessage Object: 0x7ffee46fd1c0
|
|
0 Electron Framework 0x00000001181c78d9 base::debug::CollectStackTrace(void**, unsigned long) + 9
|
|
1 Electron Framework 0x00000001180ea633 base::debug::StackTrace::StackTrace() + 19
|
|
2 Electron Framework 0x000000011a04decd gin::(anonymous namespace)::PrintStackTrace() + 45
|
|
3 Electron Framework 0x0000000119a9b416 V8_Fatal(char const*, int, char const*, ...) + 326
|
|
4 Electron Framework 0x0000000119a9aeb5 v8::base::(anonymous namespace)::DefaultDcheckHandler(char const*, int, char const*) + 21
|
|
5 Electron Framework 0x000000011530763f v8::internal::JSObject::GetEmbedderFieldOffset(int) + 207
|
|
6 Electron Framework 0x00000001155f68e6 v8::internal::LocalEmbedderHeapTracer::EmbedderWriteBarrier(v8::internal::Heap*, v8::internal::JSObject) + 150
|
|
7 Electron Framework 0x00000001152cd34f v8::Object::SetAlignedPointerInInternalField(int, void*) + 639
|
|
8 Electron Framework 0x000000011d18df35 node::BaseObject::BaseObject(node::Environment*, v8::Local<v8::Object>) + 101
|
|
9 Electron Framework 0x000000011d347b6e node::crypto::DiffieHellman::DiffieHellman(node::Environment*, v8::Local<v8::Object>) + 14
|
|
10 Electron Framework 0x000000011d348413 node::crypto::DiffieHellman::New(v8::FunctionCallbackInfo<v8::Value> const&) + 147
|
|
[...]
|
|
|
|
This crash happens because this V8 isolate has cppgc enabled. When cppgc is
|
|
enabled, V8 assumes that the first embedder field is a "type" pointer, the
|
|
first 16 bits of which are the embedder ID. Node did not adhere to this
|
|
requirement. Sometimes--mostly, even--this worked _by accident_. If the first
|
|
field in the BaseObject was a pointer to a bit of memory that happened to
|
|
contain the two-byte little-endian value 0x0001, however, V8 would take that to
|
|
mean that the object was a Blink object[1], and attempt to read the pointer in
|
|
the second embedder slot, which would result in a CHECK.
|
|
|
|
This change adds an "embedder id" pointer as the first embedder field in all
|
|
Node-managed objects. This ensures that cppgc will always skip over Node
|
|
objects.
|
|
|
|
This patch should be upstreamed to Node.
|
|
|
|
[1]: https://source.chromium.org/chromium/chromium/src/+/main:gin/public/gin_embedders.h;l=20;drc=5a758a97032f0b656c3c36a3497560762495501a
|
|
|
|
See also: https://source.chromium.org/chromium/chromium/src/+/main:v8/include/v8-cppgc.h;l=70-76;drc=5a758a97032f0b656c3c36a3497560762495501a
|
|
|
|
diff --git a/src/base_object.h b/src/base_object.h
|
|
index a17879be5b452aa208caf1b29c86f8e5d9baa693..c48d7a7e9d0535cabb4eb84b5700d0cf537ddc88 100644
|
|
--- a/src/base_object.h
|
|
+++ b/src/base_object.h
|
|
@@ -40,7 +40,7 @@ class TransferData;
|
|
|
|
class BaseObject : public MemoryRetainer {
|
|
public:
|
|
- enum InternalFields { kSlot, kInternalFieldCount };
|
|
+ enum InternalFields { kWrapperType, kSlot, kInternalFieldCount };
|
|
|
|
// Associates this object with `object`. It uses the 0th internal field for
|
|
// that, and in particular aborts if there is no such field.
|
|
diff --git a/src/env.cc b/src/env.cc
|
|
index 24aeb329c593bfd5a35877d6f4e2b7afa9848306..de41e5b7f6ff9f818c661484a93b74db7569e31f 100644
|
|
--- a/src/env.cc
|
|
+++ b/src/env.cc
|
|
@@ -2013,11 +2013,20 @@ void Environment::RunWeakRefCleanup() {
|
|
isolate()->ClearKeptObjects();
|
|
}
|
|
|
|
+// This just has to be different from the Chromium ones:
|
|
+// https://source.chromium.org/chromium/chromium/src/+/main:gin/public/gin_embedders.h;l=18-23;drc=5a758a97032f0b656c3c36a3497560762495501a
|
|
+// Otherwise, when Node is loaded in an isolate which uses cppgc, cppgc will
|
|
+// misinterpret the data stored in the embedder fields and try to garbage
|
|
+// collect them.
|
|
+uint16_t kNodeEmbedderId = 0x90de;
|
|
+
|
|
// Not really any better place than env.cc at this moment.
|
|
BaseObject::BaseObject(Environment* env, Local<Object> object)
|
|
: persistent_handle_(env->isolate(), object), env_(env) {
|
|
CHECK_EQ(false, object.IsEmpty());
|
|
- CHECK_GT(object->InternalFieldCount(), 0);
|
|
+ CHECK_GT(object->InternalFieldCount(), BaseObject::kSlot);
|
|
+ object->SetAlignedPointerInInternalField(BaseObject::kWrapperType,
|
|
+ &kNodeEmbedderId);
|
|
object->SetAlignedPointerInInternalField(BaseObject::kSlot,
|
|
static_cast<void*>(this));
|
|
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
|
|
@@ -2071,7 +2080,8 @@ void BaseObject::MakeWeak() {
|
|
void BaseObject::LazilyInitializedJSTemplateConstructor(
|
|
const FunctionCallbackInfo<Value>& args) {
|
|
DCHECK(args.IsConstructCall());
|
|
- DCHECK_GT(args.This()->InternalFieldCount(), 0);
|
|
+ DCHECK_GT(args.This()->InternalFieldCount(), BaseObject::kSlot);
|
|
+ args.This()->SetAlignedPointerInInternalField(BaseObject::kWrapperType, &kNodeEmbedderId);
|
|
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
|
|
}
|
|
|