| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
1 parent 8044051 commit 0be79f4
6 files changed
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -62,31 +62,6 @@ inline uv_loop_t* IsolateData::event_loop() const { | |||
| 62 | 62 | return event_loop_; | |
| 63 | 63 | } | |
| 64 | 64 | ||
| 65 | - inline void IsolateData::SetCppgcReference(v8::Isolate* isolate, | ||
| 66 | - v8::Local<v8::Object> object, | ||
| 67 | - void* wrappable) { | ||
| 68 | - v8::CppHeap* heap = isolate->GetCppHeap(); | ||
| 69 | - CHECK_NOT_NULL(heap); | ||
| 70 | - v8::WrapperDescriptor descriptor = heap->wrapper_descriptor(); | ||
| 71 | - uint16_t required_size = std::max(descriptor.wrappable_instance_index, | ||
| 72 | - descriptor.wrappable_type_index); | ||
| 73 | - CHECK_GT(object->InternalFieldCount(), required_size); | ||
| 74 | - | ||
| 75 | - uint16_t* id_ptr = nullptr; | ||
| 76 | - { | ||
| 77 | - Mutex::ScopedLock lock(isolate_data_mutex_); | ||
| 78 | - auto it = | ||
| 79 | - wrapper_data_map_.find(descriptor.embedder_id_for_garbage_collected); | ||
| 80 | - CHECK_NE(it, wrapper_data_map_.end()); | ||
| 81 | - id_ptr = &(it->second->cppgc_id); | ||
| 82 | - } | ||
| 83 | - | ||
| 84 | - object->SetAlignedPointerInInternalField(descriptor.wrappable_type_index, | ||
| 85 | - id_ptr); | ||
| 86 | - object->SetAlignedPointerInInternalField(descriptor.wrappable_instance_index, | ||
| 87 | - wrappable); | ||
| 88 | - } | ||
| 89 | - | ||
| 90 | 65 | inline uint16_t* IsolateData::embedder_id_for_cppgc() const { | |
| 91 | 66 | return &(wrapper_data_->cppgc_id); | |
| 92 | 67 | } | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -23,6 +23,7 @@ | |||
| 23 | 23 | #include "util-inl.h" | |
| 24 | 24 | #include "v8-cppgc.h" | |
| 25 | 25 | #include "v8-profiler.h" | |
| 26 | + #include "v8-sandbox.h" // v8::Object::Wrap(), v8::Object::Unwrap() | ||
| 26 | 27 | ||
| 27 | 28 | #include <algorithm> | |
| 28 | 29 | #include <atomic> | |
@@ -71,7 +72,6 @@ using v8::TryCatch; | |||
| 71 | 72 | using v8::Uint32; | |
| 72 | 73 | using v8::Undefined; | |
| 73 | 74 | using v8::Value; | |
| 74 | - using v8::WrapperDescriptor; | ||
| 75 | 75 | using worker::Worker; | |
| 76 | 76 | ||
| 77 | 77 | int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64; | |
@@ -533,6 +533,14 @@ void IsolateData::CreateProperties() { | |||
| 533 | 533 | CreateEnvProxyTemplate(this); | |
| 534 | 534 | } | |
| 535 | 535 | ||
| 536 | + // Previously, the general convention of the wrappable layout for cppgc in | ||
| 537 | + // the ecosystem is: | ||
| 538 | + // [ 0 ] -> embedder id | ||
| 539 | + // [ 1 ] -> wrappable instance | ||
| 540 | + // Now V8 has deprecated this layout-based tracing enablement, embedders | ||
| 541 | + // should simply use v8::Object::Wrap() and v8::Object::Unwrap(). We preserve | ||
| 542 | + // this layout only to distinguish internally how the memory of a Node.js | ||
| 543 | + // wrapper is managed or whether a wrapper is managed by Node.js. | ||
| 536 | 544 | constexpr uint16_t kDefaultCppGCEmbedderID = 0x90de; | |
| 537 | 545 | Mutex IsolateData::isolate_data_mutex_; | |
| 538 | 546 | std::unordered_map<uint16_t, std::unique_ptr<PerIsolateWrapperData>> | |
@@ -570,36 +578,16 @@ IsolateData::IsolateData(Isolate* isolate, | |||
| 570 | 578 | v8::CppHeap* cpp_heap = isolate->GetCppHeap(); | |
| 571 | 579 | ||
| 572 | 580 | uint16_t cppgc_id = kDefaultCppGCEmbedderID; | |
| 573 | - if (cpp_heap != nullptr) { | ||
| 574 | - // The general convention of the wrappable layout for cppgc in the | ||
| 575 | - // ecosystem is: | ||
| 576 | - // [ 0 ] -> embedder id | ||
| 577 | - // [ 1 ] -> wrappable instance | ||
| 578 | - // If the Isolate includes a CppHeap attached by another embedder, | ||
| 579 | - // And if they also use the field 0 for the ID, we DCHECK that | ||
| 580 | - // the layout matches our layout, and record the embedder ID for cppgc | ||
| 581 | - // to avoid accidentally enabling cppgc on non-cppgc-managed wrappers . | ||
| 582 | - v8::WrapperDescriptor descriptor = cpp_heap->wrapper_descriptor(); | ||
| 583 | - if (descriptor.wrappable_type_index == BaseObject::kEmbedderType) { | ||
| 584 | - cppgc_id = descriptor.embedder_id_for_garbage_collected; | ||
| 585 | - DCHECK_EQ(descriptor.wrappable_instance_index, BaseObject::kSlot); | ||
| 586 | - } | ||
| 587 | - // If the CppHeap uses the slot we use to put non-cppgc-traced BaseObject | ||
| 588 | - // for embedder ID, V8 could accidentally enable cppgc on them. So | ||
| 589 | - // safe guard against this. | ||
| 590 | - DCHECK_NE(descriptor.wrappable_type_index, BaseObject::kSlot); | ||
| 591 | - } else { | ||
| 592 | - cpp_heap_ = CppHeap::Create( | ||
| 593 | - platform, | ||
| 594 | - CppHeapCreateParams{ | ||
| 595 | - {}, | ||
| 596 | - WrapperDescriptor( | ||
| 597 | - BaseObject::kEmbedderType, BaseObject::kSlot, cppgc_id)}); | ||
| 598 | - isolate->AttachCppHeap(cpp_heap_.get()); | ||
| 599 | - } | ||
| 600 | 581 | // We do not care about overflow since we just want this to be different | |
| 601 | 582 | // from the cppgc id. | |
| 602 | 583 | uint16_t non_cppgc_id = cppgc_id + 1; | |
| 584 | + if (cpp_heap == nullptr) { | ||
| 585 | + cpp_heap_ = CppHeap::Create(platform, v8::CppHeapCreateParams{{}}); | ||
| 586 | + // TODO(joyeecheung): pass it into v8::Isolate::CreateParams and let V8 | ||
| 587 | + // own it when we can keep the isolate registered/task runner discoverable | ||
| 588 | + // during isolate disposal. | ||
| 589 | + isolate->AttachCppHeap(cpp_heap_.get()); | ||
| 590 | + } | ||
| 603 | 591 | ||
| 604 | 592 | { | |
| 605 | 593 | // GC could still be run after the IsolateData is destroyed, so we store | |
@@ -631,11 +619,12 @@ IsolateData::~IsolateData() { | |||
| 631 | 619 | } | |
| 632 | 620 | } | |
| 633 | 621 | ||
| 634 | - // Public API | ||
| 622 | + // Deprecated API, embedders should use v8::Object::Wrap() directly instead. | ||
| 635 | 623 | void SetCppgcReference(Isolate* isolate, | |
| 636 | 624 | Local<Object> object, | |
| 637 | 625 | void* wrappable) { | |
| 638 | - IsolateData::SetCppgcReference(isolate, object, wrappable); | ||
| 626 | + v8::Object::Wrap<v8::CppHeapPointerTag::kDefaultTag>( | ||
| 627 | + isolate, object, wrappable); | ||
| 639 | 628 | } | |
| 640 | 629 | ||
| 641 | 630 | void IsolateData::MemoryInfo(MemoryTracker* tracker) const { | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -175,10 +175,6 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { | |||
| 175 | 175 | uint16_t* embedder_id_for_cppgc() const; | |
| 176 | 176 | uint16_t* embedder_id_for_non_cppgc() const; | |
| 177 | 177 | ||
| 178 | - static inline void SetCppgcReference(v8::Isolate* isolate, | ||
| 179 | - v8::Local<v8::Object> object, | ||
| 180 | - void* wrappable); | ||
| 181 | - | ||
| 182 | 178 | inline uv_loop_t* event_loop() const; | |
| 183 | 179 | inline MultiIsolatePlatform* platform() const; | |
| 184 | 180 | inline const SnapshotData* snapshot_data() const; | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1552,24 +1552,14 @@ void RegisterSignalHandler(int signal, | |||
| 1552 | 1552 | bool reset_handler = false); | |
| 1553 | 1553 | #endif // _WIN32 | |
| 1554 | 1554 | ||
| 1555 | - // Configure the layout of the JavaScript object with a cppgc::GarbageCollected | ||
| 1556 | - // instance so that when the JavaScript object is reachable, the garbage | ||
| 1557 | - // collected instance would have its Trace() method invoked per the cppgc | ||
| 1558 | - // contract. To make it work, the process must have called | ||
| 1559 | - // cppgc::InitializeProcess() before, which is usually the case for addons | ||
| 1560 | - // loaded by the stand-alone Node.js executable. Embedders of Node.js can use | ||
| 1561 | - // either need to call it themselves or make sure that | ||
| 1562 | - // ProcessInitializationFlags::kNoInitializeCppgc is *not* set for cppgc to | ||
| 1563 | - // work. | ||
| 1564 | - // If the CppHeap is owned by Node.js, which is usually the case for addon, | ||
| 1565 | - // the object must be created with at least two internal fields available, | ||
| 1566 | - // and the first two internal fields would be configured by Node.js. | ||
| 1567 | - // This may be superseded by a V8 API in the future, see | ||
| 1568 | - // https://bugs.chromium.org/p/v8/issues/detail?id=13960. Until then this | ||
| 1569 | - // serves as a helper for Node.js isolates. | ||
| 1570 | - NODE_EXTERN void SetCppgcReference(v8::Isolate* isolate, | ||
| 1571 | - v8::Local<v8::Object> object, | ||
| 1572 | - void* wrappable); | ||
| 1555 | + // This is kept as a compatibility layer for addons to wrap cppgc-managed | ||
| 1556 | + // objects on Node.js versions without v8::Object::Wrap(). Addons created to | ||
| 1557 | + // work with only Node.js versions with v8::Object::Wrap() should use that | ||
| 1558 | + // instead. | ||
| 1559 | + NODE_DEPRECATED("Use v8::Object::Wrap()", | ||
| 1560 | + NODE_EXTERN void SetCppgcReference(v8::Isolate* isolate, | ||
| 1561 | + v8::Local<v8::Object> object, | ||
| 1562 | + void* wrappable)); | ||
| 1573 | 1563 | ||
| 1574 | 1564 | } // namespace node | |
| 1575 | 1565 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -1,8 +1,10 @@ | |||
| 1 | + #include <assert.h> | ||
| 1 | 2 | #include <cppgc/allocation.h> | |
| 2 | 3 | #include <cppgc/garbage-collected.h> | |
| 3 | 4 | #include <cppgc/heap.h> | |
| 4 | 5 | #include <node.h> | |
| 5 | 6 | #include <v8-cppgc.h> | |
| 7 | + #include <v8-sandbox.h> | ||
| 6 | 8 | #include <v8.h> | |
| 7 | 9 | #include <algorithm> | |
| 8 | 10 | ||
@@ -15,21 +17,17 @@ class CppGCed : public cppgc::GarbageCollected<CppGCed> { | |||
| 15 | 17 | static void New(const v8::FunctionCallbackInfo<v8::Value>& args) { | |
| 16 | 18 | v8::Isolate* isolate = args.GetIsolate(); | |
| 17 | 19 | v8::Local<v8::Object> js_object = args.This(); | |
| 18 | - CppGCed* gc_object = cppgc::MakeGarbageCollected<CppGCed>( | ||
| 19 | - isolate->GetCppHeap()->GetAllocationHandle()); | ||
| 20 | + auto* heap = isolate->GetCppHeap(); | ||
| 21 | + assert(heap != nullptr); | ||
| 22 | + CppGCed* gc_object = | ||
| 23 | + cppgc::MakeGarbageCollected<CppGCed>(heap->GetAllocationHandle()); | ||
| 20 | 24 | node::SetCppgcReference(isolate, js_object, gc_object); | |
| 21 | 25 | args.GetReturnValue().Set(js_object); | |
| 22 | 26 | } | |
| 23 | 27 | ||
| 24 | 28 | static v8::Local<v8::Function> GetConstructor( | |
| 25 | 29 | v8::Local<v8::Context> context) { | |
| 26 | 30 | auto ft = v8::FunctionTemplate::New(context->GetIsolate(), New); | |
| 27 | - auto ot = ft->InstanceTemplate(); | ||
| 28 | - v8::WrapperDescriptor descriptor = | ||
| 29 | - context->GetIsolate()->GetCppHeap()->wrapper_descriptor(); | ||
| 30 | - uint16_t required_size = std::max(descriptor.wrappable_instance_index, | ||
| 31 | - descriptor.wrappable_type_index); | ||
| 32 | - ot->SetInternalFieldCount(required_size + 1); | ||
| 33 | 31 | return ft->GetFunction(context).ToLocalChecked(); | |
| 34 | 32 | } | |
| 35 | 33 | ||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -3,16 +3,12 @@ | |||
| 3 | 3 | #include <cppgc/heap.h> | |
| 4 | 4 | #include <node.h> | |
| 5 | 5 | #include <v8-cppgc.h> | |
| 6 | + #include <v8-sandbox.h> | ||
| 6 | 7 | #include <v8.h> | |
| 7 | 8 | #include "node_test_fixture.h" | |
| 8 | 9 | ||
| 9 | 10 | // This tests that Node.js can work with an existing CppHeap. | |
| 10 | 11 | ||
| 11 | - // Mimic the Blink layout. | ||
| 12 | - static int kWrappableTypeIndex = 0; | ||
| 13 | - static int kWrappableInstanceIndex = 1; | ||
| 14 | - static uint16_t kEmbedderID = 0x1; | ||
| 15 | - | ||
| 16 | 12 | // Mimic a class that does not know about Node.js. | |
| 17 | 13 | class CppGCed : public cppgc::GarbageCollected<CppGCed> { | |
| 18 | 14 | public: | |
@@ -23,21 +19,18 @@ class CppGCed : public cppgc::GarbageCollected<CppGCed> { | |||
| 23 | 19 | static void New(const v8::FunctionCallbackInfo<v8::Value>& args) { | |
| 24 | 20 | v8::Isolate* isolate = args.GetIsolate(); | |
| 25 | 21 | v8::Local<v8::Object> js_object = args.This(); | |
| 26 | - CppGCed* gc_object = cppgc::MakeGarbageCollected<CppGCed>( | ||
| 27 | - isolate->GetCppHeap()->GetAllocationHandle()); | ||
| 28 | - js_object->SetAlignedPointerInInternalField(kWrappableTypeIndex, | ||
| 29 | - &kEmbedderID); | ||
| 30 | - js_object->SetAlignedPointerInInternalField(kWrappableInstanceIndex, | ||
| 31 | - gc_object); | ||
| 22 | + auto* heap = isolate->GetCppHeap(); | ||
| 23 | + CHECK_NOT_NULL(heap); | ||
| 24 | + CppGCed* gc_object = | ||
| 25 | + cppgc::MakeGarbageCollected<CppGCed>(heap->GetAllocationHandle()); | ||
| 26 | + node::SetCppgcReference(isolate, js_object, gc_object); | ||
| 32 | 27 | kConstructCount++; | |
| 33 | 28 | args.GetReturnValue().Set(js_object); | |
| 34 | 29 | } | |
| 35 | 30 | ||
| 36 | 31 | static v8::Local<v8::Function> GetConstructor( | |
| 37 | 32 | v8::Local<v8::Context> context) { | |
| 38 | 33 | auto ft = v8::FunctionTemplate::New(context->GetIsolate(), New); | |
| 39 | - auto ot = ft->InstanceTemplate(); | ||
| 40 | - ot->SetInternalFieldCount(2); | ||
| 41 | 34 | return ft->GetFunction(context).ToLocalChecked(); | |
| 42 | 35 | } | |
| 43 | 36 | ||
@@ -58,12 +51,12 @@ TEST_F(NodeZeroIsolateTestFixture, ExistingCppHeapTest) { | |||
| 58 | 51 | ||
| 59 | 52 | // Create and attach the CppHeap before we set up the IsolateData so that | |
| 60 | 53 | // it recognizes the existing heap. | |
| 61 | - std::unique_ptr<v8::CppHeap> cpp_heap = v8::CppHeap::Create( | ||
| 62 | - platform.get(), | ||
| 63 | - v8::CppHeapCreateParams( | ||
| 64 | - {}, | ||
| 65 | - v8::WrapperDescriptor( | ||
| 66 | - kWrappableTypeIndex, kWrappableInstanceIndex, kEmbedderID))); | ||
| 54 | + std::unique_ptr<v8::CppHeap> cpp_heap = | ||
| 55 | + v8::CppHeap::Create(platform.get(), v8::CppHeapCreateParams{{}}); | ||
| 56 | + | ||
| 57 | + // TODO(joyeecheung): pass it into v8::Isolate::CreateParams and let V8 | ||
| 58 | + // own it when we can keep the isolate registered/task runner discoverable | ||
| 59 | + // during isolate disposal. | ||
| 67 | 60 | isolate->AttachCppHeap(cpp_heap.get()); | |
| 68 | 61 | ||
| 69 | 62 | // Try creating Context + IsolateData + Environment. | |
| Back | FazBrowse Home | New Git URL |
0 commit comments