Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit f970087

Browse filesBrowse files
joyeecheungjoyeecheung
authored
deps: V8: backport 93b1a74cbc9b
Original commit message: Reland "[api] allow v8::Data as internal field" This is a reland of commit 0aa622e12893e9921c01a34ce9507b544e599c4a The original patch tried to run a test that calls exit() in the fatal error handler in parallel, which would not work. This marked the test with TEST() to avoid running it in parallel. Original change's description: > [api] allow v8::Data as internal field > > Previously only v8::Value can be stored as internal fields. > In some cases, however, it's necessary for the embedder to > tie the lifetime of a v8::Data with the lifetime of a > JS object, and that v8::Data may not be a v8::Value, as > it can be something returned from the V8 API. One way to > keep the v8::Data alive may be to use a v8::Persistent<v8::Data> > but that can easily lead to leaks. > > This patch changes v8::Object::GetInternalField() and > v8::Object::SetInernalField() to accept v8::Data instead of just > v8::Value, so that v8::Data can kept alive by a JS object in > a way that the GC can be aware of to address this problem. > This is a breaking change for embedders > using v8::Object::GetInternalField() as it changes the return > type. Since most v8::Value subtypes only support direct casts > from v8::Value but not v8::Data, calls like > > object->GetInternalField(index).As<v8::External>() > > needs to be updated to cast the value to v8::Value first: > > object->GetInternalField(index).As<v8::Value>().As<v8::External>() > > Bug: v8:14120 > Change-Id: I731c958d1756b9d5ee4a3e78813416cd60d1b7ca > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4707972 > Reviewed-by: Michael Lippautz <mlippautz@chromium.org> > Commit-Queue: Joyee Cheung <joyee@igalia.com> > Cr-Commit-Position: refs/heads/main@{#89718} Bug: v8:14120 Change-Id: I3e45d09b5c300d5eefc73e380ef21ac2bd61760c Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4834471 Commit-Queue: Joyee Cheung <joyee@igalia.com> Reviewed-by: Camillo Bruni <cbruni@chromium.org> Cr-Commit-Position: refs/heads/main@{#89824} Refs: v8/v8@93b1a74 PR-URL: #49419 Refs: v8/v8@0aa622e Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
1 parent a3020dd commit f970087
Copy full SHA for f970087

File tree

Expand file treeCollapse file tree

8 files changed

+103
-36
lines changed
Open diff view settings
Filter options
Expand file treeCollapse file tree

8 files changed

+103
-36
lines changed
Open diff view settings
Collapse file

‎common.gypi‎

Copy file name to clipboardExpand all lines: common.gypi
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@
3636

3737
# Reset this number to 0 on major V8 upgrades.
3838
# Increment by one for each non-official patch applied to deps/v8.
39-
'v8_embedder_string': '-node.15',
39+
'v8_embedder_string': '-node.16',
4040

4141
##### V8 defaults for Node.js #####
4242

Collapse file

‎deps/v8/include/v8-object.h‎

Copy file name to clipboardExpand all lines: deps/v8/include/v8-object.h
+17-8Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -474,11 +474,20 @@ class V8_EXPORT Object : public Value {
474474
return object->InternalFieldCount();
475475
}
476476

477-
/** Gets the value from an internal field. */
478-
V8_INLINE Local<Value> GetInternalField(int index);
477+
/**
478+
* Gets the data from an internal field.
479+
* To cast the return value into v8::Value subtypes, it needs to be
480+
* casted to a v8::Value first. For example, to cast it into v8::External:
481+
*
482+
* object->GetInternalField(index).As<v8::Value>().As<v8::External>();
483+
*
484+
* The embedder should make sure that the internal field being retrieved
485+
* using this method has already been set with SetInternalField() before.
486+
**/
487+
V8_INLINE Local<Data> GetInternalField(int index);
479488

480-
/** Sets the value in an internal field. */
481-
void SetInternalField(int index, Local<Value> value);
489+
/** Sets the data in an internal field. */
490+
void SetInternalField(int index, Local<Data> data);
482491

483492
/**
484493
* Gets a 2-byte-aligned native pointer from an internal field. This field
@@ -710,13 +719,13 @@ class V8_EXPORT Object : public Value {
710719
private:
711720
Object();
712721
static void CheckCast(Value* obj);
713-
Local<Value> SlowGetInternalField(int index);
722+
Local<Data> SlowGetInternalField(int index);
714723
void* SlowGetAlignedPointerFromInternalField(int index);
715724
};
716725

717726
// --- Implementation ---
718727

719-
Local<Value> Object::GetInternalField(int index) {
728+
Local<Data> Object::GetInternalField(int index) {
720729
#ifndef V8_ENABLE_CHECKS
721730
using A = internal::Address;
722731
using I = internal::Internals;
@@ -734,12 +743,12 @@ Local<Value> Object::GetInternalField(int index) {
734743
#endif
735744

736745
#ifdef V8_ENABLE_CONSERVATIVE_STACK_SCANNING
737-
return Local<Value>(reinterpret_cast<Value*>(value));
746+
return Local<Data>(reinterpret_cast<Data*>(value));
738747
#else
739748
internal::Isolate* isolate =
740749
internal::IsolateFromNeverReadOnlySpaceObject(obj);
741750
A* result = HandleScope::CreateHandle(isolate, value);
742-
return Local<Value>(reinterpret_cast<Value*>(result));
751+
return Local<Data>(reinterpret_cast<Data*>(result));
743752
#endif
744753
}
745754
#endif
Collapse file

‎deps/v8/samples/process.cc‎

Copy file name to clipboardExpand all lines: deps/v8/samples/process.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -388,7 +388,7 @@ Local<Object> JsHttpRequestProcessor::WrapMap(map<string, string>* obj) {
388388
// Utility function that extracts the C++ map pointer from a wrapper
389389
// object.
390390
map<string, string>* JsHttpRequestProcessor::UnwrapMap(Local<Object> obj) {
391-
Local<External> field = obj->GetInternalField(0).As<External>();
391+
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
392392
void* ptr = field->Value();
393393
return static_cast<map<string, string>*>(ptr);
394394
}
@@ -504,7 +504,7 @@ Local<Object> JsHttpRequestProcessor::WrapRequest(HttpRequest* request) {
504504
* wrapper object.
505505
*/
506506
HttpRequest* JsHttpRequestProcessor::UnwrapRequest(Local<Object> obj) {
507-
Local<External> field = obj->GetInternalField(0).As<External>();
507+
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
508508
void* ptr = field->Value();
509509
return static_cast<HttpRequest*>(ptr);
510510
}
Collapse file

‎deps/v8/src/api/api.cc‎

Copy file name to clipboardExpand all lines: deps/v8/src/api/api.cc
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6342,16 +6342,16 @@ static bool InternalFieldOK(i::Handle<i::JSReceiver> obj, int index,
63426342
location, "Internal field out of bounds");
63436343
}
63446344

6345-
Local<Value> v8::Object::SlowGetInternalField(int index) {
6345+
Local<Data> v8::Object::SlowGetInternalField(int index) {
63466346
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
63476347
const char* location = "v8::Object::GetInternalField()";
63486348
if (!InternalFieldOK(obj, index, location)) return Local<Value>();
63496349
i::Handle<i::Object> value(i::JSObject::cast(*obj).GetEmbedderField(index),
63506350
obj->GetIsolate());
6351-
return Utils::ToLocal(value);
6351+
return ToApiHandle<Data>(value);
63526352
}
63536353

6354-
void v8::Object::SetInternalField(int index, v8::Local<Value> value) {
6354+
void v8::Object::SetInternalField(int index, v8::Local<Data> value) {
63556355
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
63566356
const char* location = "v8::Object::SetInternalField()";
63576357
if (!InternalFieldOK(obj, index, location)) return;
Collapse file

‎deps/v8/test/cctest/test-api.cc‎

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-api.cc
+55-9Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,6 +2874,43 @@ THREADED_TEST(FunctionPrototype) {
28742874
CHECK_EQ(v8_run_int32value(script), 321);
28752875
}
28762876

2877+
bool internal_field_check_called = false;
2878+
void OnInternalFieldCheck(const char* location, const char* message) {
2879+
internal_field_check_called = true;
2880+
exit(strcmp(location, "v8::Value::Cast") +
2881+
strcmp(message, "Data is not a Value"));
2882+
}
2883+
2884+
THREADED_TEST(InternalDataFields) {
2885+
LocalContext env;
2886+
v8::Isolate* isolate = env->GetIsolate();
2887+
v8::HandleScope scope(isolate);
2888+
2889+
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
2890+
Local<v8::ObjectTemplate> instance_templ = templ->InstanceTemplate();
2891+
instance_templ->SetInternalFieldCount(1);
2892+
Local<v8::Object> obj = templ->GetFunction(env.local())
2893+
.ToLocalChecked()
2894+
->NewInstance(env.local())
2895+
.ToLocalChecked();
2896+
CHECK_EQ(1, obj->InternalFieldCount());
2897+
Local<v8::Data> data = obj->GetInternalField(0);
2898+
CHECK(data->IsValue() && data.As<v8::Value>()->IsUndefined());
2899+
Local<v8::Private> sym = v8::Private::New(isolate, v8_str("Foo"));
2900+
obj->SetInternalField(0, sym);
2901+
Local<v8::Data> field = obj->GetInternalField(0);
2902+
CHECK(!field->IsValue());
2903+
CHECK(field->IsPrivate());
2904+
CHECK_EQ(sym, field);
2905+
2906+
#ifdef V8_ENABLE_CHECKS
2907+
isolate->SetFatalErrorHandler(OnInternalFieldCheck);
2908+
USE(obj->GetInternalField(0).As<v8::Value>());
2909+
// If it's never called this would fail.
2910+
CHECK(internal_field_check_called);
2911+
#endif
2912+
}
2913+
28772914
THREADED_TEST(InternalFields) {
28782915
LocalContext env;
28792916
v8::Isolate* isolate = env->GetIsolate();
@@ -2887,9 +2924,12 @@ THREADED_TEST(InternalFields) {
28872924
->NewInstance(env.local())
28882925
.ToLocalChecked();
28892926
CHECK_EQ(1, obj->InternalFieldCount());
2890-
CHECK(obj->GetInternalField(0)->IsUndefined());
2927+
CHECK(obj->GetInternalField(0).As<v8::Value>()->IsUndefined());
28912928
obj->SetInternalField(0, v8_num(17));
2892-
CHECK_EQ(17, obj->GetInternalField(0)->Int32Value(env.local()).FromJust());
2929+
CHECK_EQ(17, obj->GetInternalField(0)
2930+
.As<v8::Value>()
2931+
->Int32Value(env.local())
2932+
.FromJust());
28932933
}
28942934

28952935
TEST(InternalFieldsSubclassing) {
@@ -2915,14 +2955,16 @@ TEST(InternalFieldsSubclassing) {
29152955
CHECK_EQ(0, i_obj->map().GetInObjectProperties());
29162956
// Check writing and reading internal fields.
29172957
for (int j = 0; j < nof_embedder_fields; j++) {
2918-
CHECK(obj->GetInternalField(j)->IsUndefined());
2958+
CHECK(obj->GetInternalField(j).As<v8::Value>()->IsUndefined());
29192959
int value = 17 + j;
29202960
obj->SetInternalField(j, v8_num(value));
29212961
}
29222962
for (int j = 0; j < nof_embedder_fields; j++) {
29232963
int value = 17 + j;
2924-
CHECK_EQ(value,
2925-
obj->GetInternalField(j)->Int32Value(env.local()).FromJust());
2964+
CHECK_EQ(value, obj->GetInternalField(j)
2965+
.As<v8::Value>()
2966+
->Int32Value(env.local())
2967+
.FromJust());
29262968
}
29272969
CHECK(env->Global()
29282970
->Set(env.local(), v8_str("BaseClass"), constructor)
@@ -3022,9 +3064,12 @@ THREADED_TEST(GlobalObjectInternalFields) {
30223064
v8::Local<v8::Object> global_proxy = env->Global();
30233065
v8::Local<v8::Object> global = global_proxy->GetPrototype().As<v8::Object>();
30243066
CHECK_EQ(1, global->InternalFieldCount());
3025-
CHECK(global->GetInternalField(0)->IsUndefined());
3067+
CHECK(global->GetInternalField(0).As<v8::Value>()->IsUndefined());
30263068
global->SetInternalField(0, v8_num(17));
3027-
CHECK_EQ(17, global->GetInternalField(0)->Int32Value(env.local()).FromJust());
3069+
CHECK_EQ(17, global->GetInternalField(0)
3070+
.As<v8::Value>()
3071+
->Int32Value(env.local())
3072+
.FromJust());
30283073
}
30293074

30303075

@@ -7766,7 +7811,7 @@ void InternalFieldCallback(bool global_gc) {
77667811
.ToLocalChecked();
77677812
handle.Reset(isolate, obj);
77687813
CHECK_EQ(2, obj->InternalFieldCount());
7769-
CHECK(obj->GetInternalField(0)->IsUndefined());
7814+
CHECK(obj->GetInternalField(0).As<v8::Value>()->IsUndefined());
77707815
t1 = new Trivial(42);
77717816
t2 = new Trivial2(103, 9);
77727817

@@ -29858,7 +29903,8 @@ class HiddenDataDelegate : public v8::Context::DeepFreezeDelegate {
2985829903
std::vector<v8::Local<v8::Object>>& children_out) override {
2985929904
int fields = obj->InternalFieldCount();
2986029905
for (int idx = 0; idx < fields; idx++) {
29861-
v8::Local<v8::Value> child_value = obj->GetInternalField(idx);
29906+
v8::Local<v8::Value> child_value =
29907+
obj->GetInternalField(idx).As<v8::Value>();
2986229908
if (child_value->IsExternal()) {
2986329909
if (!FreezeExternal(v8::Local<v8::External>::Cast(child_value),
2986429910
children_out)) {
Collapse file

‎deps/v8/test/cctest/test-api.h‎

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-api.h
+5-3Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,9 +44,11 @@ template <typename T>
4444
static void CheckInternalFieldsAreZero(v8::Local<T> value) {
4545
CHECK_EQ(T::kInternalFieldCount, value->InternalFieldCount());
4646
for (int i = 0; i < value->InternalFieldCount(); i++) {
47-
CHECK_EQ(0, value->GetInternalField(i)
48-
->Int32Value(CcTest::isolate()->GetCurrentContext())
49-
.FromJust());
47+
v8::Local<v8::Value> field =
48+
value->GetInternalField(i).template As<v8::Value>();
49+
CHECK_EQ(
50+
0,
51+
field->Int32Value(CcTest::isolate()->GetCurrentContext()).FromJust());
5052
}
5153
}
5254

Collapse file

‎deps/v8/test/cctest/test-serialize.cc‎

Copy file name to clipboardExpand all lines: deps/v8/test/cctest/test-serialize.cc
+12-8Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3577,23 +3577,27 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
35773577
.ToLocalChecked()
35783578
->ToObject(context)
35793579
.ToLocalChecked();
3580-
v8::Local<v8::Object> b =
3581-
a->GetInternalField(0)->ToObject(context).ToLocalChecked();
3582-
v8::Local<v8::Object> c =
3583-
b->GetInternalField(0)->ToObject(context).ToLocalChecked();
3580+
v8::Local<v8::Object> b = a->GetInternalField(0)
3581+
.As<v8::Value>()
3582+
->ToObject(context)
3583+
.ToLocalChecked();
3584+
v8::Local<v8::Object> c = b->GetInternalField(0)
3585+
.As<v8::Value>()
3586+
->ToObject(context)
3587+
.ToLocalChecked();
35843588

35853589
InternalFieldData* a1 = reinterpret_cast<InternalFieldData*>(
35863590
a->GetAlignedPointerFromInternalField(1));
3587-
v8::Local<v8::Value> a2 = a->GetInternalField(2);
3591+
v8::Local<v8::Value> a2 = a->GetInternalField(2).As<v8::Value>();
35883592

35893593
InternalFieldData* b1 = reinterpret_cast<InternalFieldData*>(
35903594
b->GetAlignedPointerFromInternalField(1));
3591-
v8::Local<v8::Value> b2 = b->GetInternalField(2);
3595+
v8::Local<v8::Value> b2 = b->GetInternalField(2).As<v8::Value>();
35923596

3593-
v8::Local<v8::Value> c0 = c->GetInternalField(0);
3597+
v8::Local<v8::Value> c0 = c->GetInternalField(0).As<v8::Value>();
35943598
InternalFieldData* c1 = reinterpret_cast<InternalFieldData*>(
35953599
c->GetAlignedPointerFromInternalField(1));
3596-
v8::Local<v8::Value> c2 = c->GetInternalField(2);
3600+
v8::Local<v8::Value> c2 = c->GetInternalField(2).As<v8::Value>();
35973601

35983602
CHECK(c0->IsUndefined());
35993603

Collapse file

‎deps/v8/test/unittests/objects/value-serializer-unittest.cc‎

Copy file name to clipboardExpand all lines: deps/v8/test/unittests/objects/value-serializer-unittest.cc
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,12 +61,14 @@ class ValueSerializerTest : public TestWithIsolate {
6161
function_template->InstanceTemplate()->SetAccessor(
6262
StringFromUtf8("value"),
6363
[](Local<String> property, const PropertyCallbackInfo<Value>& args) {
64-
args.GetReturnValue().Set(args.Holder()->GetInternalField(0));
64+
args.GetReturnValue().Set(
65+
args.Holder()->GetInternalField(0).As<v8::Value>());
6566
});
6667
function_template->InstanceTemplate()->SetAccessor(
6768
StringFromUtf8("value2"),
6869
[](Local<String> property, const PropertyCallbackInfo<Value>& args) {
69-
args.GetReturnValue().Set(args.Holder()->GetInternalField(1));
70+
args.GetReturnValue().Set(
71+
args.Holder()->GetInternalField(1).As<v8::Value>());
7072
});
7173
for (Local<Context> context :
7274
{serialization_context_, deserialization_context_}) {
@@ -2897,6 +2899,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint32) {
28972899
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
28982900
uint32_t value = 0;
28992901
EXPECT_TRUE(object->GetInternalField(0)
2902+
.As<v8::Value>()
29002903
->Uint32Value(serialization_context())
29012904
.To(&value));
29022905
WriteExampleHostObjectTag();
@@ -2928,9 +2931,11 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint64) {
29282931
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
29292932
uint32_t value = 0, value2 = 0;
29302933
EXPECT_TRUE(object->GetInternalField(0)
2934+
.As<v8::Value>()
29312935
->Uint32Value(serialization_context())
29322936
.To(&value));
29332937
EXPECT_TRUE(object->GetInternalField(1)
2938+
.As<v8::Value>()
29342939
->Uint32Value(serialization_context())
29352940
.To(&value2));
29362941
WriteExampleHostObjectTag();
@@ -2968,6 +2973,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripDouble) {
29682973
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
29692974
double value = 0;
29702975
EXPECT_TRUE(object->GetInternalField(0)
2976+
.As<v8::Value>()
29712977
->NumberValue(serialization_context())
29722978
.To(&value));
29732979
WriteExampleHostObjectTag();

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.