Skip to content

Commit 867586c

Browse files
targostargos
committed
deps: V8: cherry-pick 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 <[email protected]> > Commit-Queue: Joyee Cheung <[email protected]> > 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 <[email protected]> Reviewed-by: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/main@{#89824} Refs: v8/v8@93b1a74 PR-URL: #49639 Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Rafael Gonzaga <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]>
1 parent 4ad3479 commit 867586c

File tree

8 files changed

+104
-35
lines changed

8 files changed

+104
-35
lines changed

‎common.gypi

Lines 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.7',
39+
'v8_embedder_string': '-node.8',
4040

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

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

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -483,11 +483,20 @@ class V8_EXPORT Object : public Value {
483483
return object.template value<Object>()->InternalFieldCount();
484484
}
485485

486-
/** Gets the value from an internal field. */
487-
V8_INLINE Local<Value> GetInternalField(int index);
486+
/**
487+
* Gets the data from an internal field.
488+
* To cast the return value into v8::Value subtypes, it needs to be
489+
* casted to a v8::Value first. For example, to cast it into v8::External:
490+
*
491+
* object->GetInternalField(index).As<v8::Value>().As<v8::External>();
492+
*
493+
* The embedder should make sure that the internal field being retrieved
494+
* using this method has already been set with SetInternalField() before.
495+
**/
496+
V8_INLINE Local<Data> GetInternalField(int index);
488497

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

492501
/**
493502
* Gets a 2-byte-aligned native pointer from an internal field. This field
@@ -725,13 +734,13 @@ class V8_EXPORT Object : public Value {
725734
private:
726735
Object();
727736
static void CheckCast(Value* obj);
728-
Local<Value> SlowGetInternalField(int index);
737+
Local<Data> SlowGetInternalField(int index);
729738
void* SlowGetAlignedPointerFromInternalField(int index);
730739
};
731740

732741
// --- Implementation ---
733742

734-
Local<Value> Object::GetInternalField(int index) {
743+
Local<Data> Object::GetInternalField(int index) {
735744
#ifndef V8_ENABLE_CHECKS
736745
using A = internal::Address;
737746
using I = internal::Internals;
@@ -750,7 +759,7 @@ Local<Value> Object::GetInternalField(int index) {
750759

751760
auto isolate = reinterpret_cast<v8::Isolate*>(
752761
internal::IsolateFromNeverReadOnlySpaceObject(obj));
753-
return Local<Value>::New(isolate, value);
762+
return Local<Data>::New(isolate, value);
754763
}
755764
#endif
756765
return SlowGetInternalField(index);

‎deps/v8/samples/process.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -386,7 +386,7 @@ Local<Object> JsHttpRequestProcessor::WrapMap(map<string, string>* obj) {
386386
// Utility function that extracts the C++ map pointer from a wrapper
387387
// object.
388388
map<string, string>* JsHttpRequestProcessor::UnwrapMap(Local<Object> obj) {
389-
Local<External> field = obj->GetInternalField(0).As<External>();
389+
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
390390
void* ptr = field->Value();
391391
return static_cast<map<string, string>*>(ptr);
392392
}
@@ -502,7 +502,7 @@ Local<Object> JsHttpRequestProcessor::WrapRequest(HttpRequest* request) {
502502
* wrapper object.
503503
*/
504504
HttpRequest* JsHttpRequestProcessor::UnwrapRequest(Local<Object> obj) {
505-
Local<External> field = obj->GetInternalField(0).As<External>();
505+
Local<External> field = obj->GetInternalField(0).As<Value>().As<External>();
506506
void* ptr = field->Value();
507507
return static_cast<HttpRequest*>(ptr);
508508
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6246,16 +6246,16 @@ static bool InternalFieldOK(i::Handle<i::JSReceiver> obj, int index,
62466246
location, "Internal field out of bounds");
62476247
}
62486248

6249-
Local<Value> v8::Object::SlowGetInternalField(int index) {
6249+
Local<Data> v8::Object::SlowGetInternalField(int index) {
62506250
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
62516251
const char* location = "v8::Object::GetInternalField()";
62526252
if (!InternalFieldOK(obj, index, location)) return Local<Value>();
62536253
i::Handle<i::Object> value(i::JSObject::cast(*obj)->GetEmbedderField(index),
62546254
obj->GetIsolate());
6255-
return Utils::ToLocal(value);
6255+
return ToApiHandle<Data>(value);
62566256
}
62576257

6258-
void v8::Object::SetInternalField(int index, v8::Local<Value> value) {
6258+
void v8::Object::SetInternalField(int index, v8::Local<Data> value) {
62596259
i::Handle<i::JSReceiver> obj = Utils::OpenHandle(this);
62606260
const char* location = "v8::Object::SetInternalField()";
62616261
if (!InternalFieldOK(obj, index, location)) return;

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

Lines changed: 57 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2884,6 +2884,45 @@ THREADED_TEST(FunctionPrototype) {
28842884
CHECK_EQ(v8_run_int32value(script), 321);
28852885
}
28862886

2887+
bool internal_field_check_called = false;
2888+
void OnInternalFieldCheck(const char* location, const char* message) {
2889+
internal_field_check_called = true;
2890+
exit(strcmp(location, "v8::Value::Cast") +
2891+
strcmp(message, "Data is not a Value"));
2892+
}
2893+
2894+
// The fatal error handler would call exit() so this should not be run in
2895+
// parallel.
2896+
TEST(InternalDataFields) {
2897+
LocalContext env;
2898+
v8::Isolate* isolate = env->GetIsolate();
2899+
v8::HandleScope scope(isolate);
2900+
2901+
Local<v8::FunctionTemplate> templ = v8::FunctionTemplate::New(isolate);
2902+
Local<v8::ObjectTemplate> instance_templ = templ->InstanceTemplate();
2903+
instance_templ->SetInternalFieldCount(1);
2904+
Local<v8::Object> obj = templ->GetFunction(env.local())
2905+
.ToLocalChecked()
2906+
->NewInstance(env.local())
2907+
.ToLocalChecked();
2908+
CHECK_EQ(1, obj->InternalFieldCount());
2909+
Local<v8::Data> data = obj->GetInternalField(0);
2910+
CHECK(data->IsValue() && data.As<v8::Value>()->IsUndefined());
2911+
Local<v8::Private> sym = v8::Private::New(isolate, v8_str("Foo"));
2912+
obj->SetInternalField(0, sym);
2913+
Local<v8::Data> field = obj->GetInternalField(0);
2914+
CHECK(!field->IsValue());
2915+
CHECK(field->IsPrivate());
2916+
CHECK_EQ(sym, field);
2917+
2918+
#ifdef V8_ENABLE_CHECKS
2919+
isolate->SetFatalErrorHandler(OnInternalFieldCheck);
2920+
USE(obj->GetInternalField(0).As<v8::Value>());
2921+
// If it's never called this would fail.
2922+
CHECK(internal_field_check_called);
2923+
#endif
2924+
}
2925+
28872926
THREADED_TEST(InternalFields) {
28882927
LocalContext env;
28892928
v8::Isolate* isolate = env->GetIsolate();
@@ -2897,9 +2936,12 @@ THREADED_TEST(InternalFields) {
28972936
->NewInstance(env.local())
28982937
.ToLocalChecked();
28992938
CHECK_EQ(1, obj->InternalFieldCount());
2900-
CHECK(obj->GetInternalField(0)->IsUndefined());
2939+
CHECK(obj->GetInternalField(0).As<v8::Value>()->IsUndefined());
29012940
obj->SetInternalField(0, v8_num(17));
2902-
CHECK_EQ(17, obj->GetInternalField(0)->Int32Value(env.local()).FromJust());
2941+
CHECK_EQ(17, obj->GetInternalField(0)
2942+
.As<v8::Value>()
2943+
->Int32Value(env.local())
2944+
.FromJust());
29032945
}
29042946

29052947
TEST(InternalFieldsSubclassing) {
@@ -2925,14 +2967,16 @@ TEST(InternalFieldsSubclassing) {
29252967
CHECK_EQ(0, i_obj->map()->GetInObjectProperties());
29262968
// Check writing and reading internal fields.
29272969
for (int j = 0; j < nof_embedder_fields; j++) {
2928-
CHECK(obj->GetInternalField(j)->IsUndefined());
2970+
CHECK(obj->GetInternalField(j).As<v8::Value>()->IsUndefined());
29292971
int value = 17 + j;
29302972
obj->SetInternalField(j, v8_num(value));
29312973
}
29322974
for (int j = 0; j < nof_embedder_fields; j++) {
29332975
int value = 17 + j;
2934-
CHECK_EQ(value,
2935-
obj->GetInternalField(j)->Int32Value(env.local()).FromJust());
2976+
CHECK_EQ(value, obj->GetInternalField(j)
2977+
.As<v8::Value>()
2978+
->Int32Value(env.local())
2979+
.FromJust());
29362980
}
29372981
CHECK(env->Global()
29382982
->Set(env.local(), v8_str("BaseClass"), constructor)
@@ -3032,9 +3076,12 @@ THREADED_TEST(GlobalObjectInternalFields) {
30323076
v8::Local<v8::Object> global_proxy = env->Global();
30333077
v8::Local<v8::Object> global = global_proxy->GetPrototype().As<v8::Object>();
30343078
CHECK_EQ(1, global->InternalFieldCount());
3035-
CHECK(global->GetInternalField(0)->IsUndefined());
3079+
CHECK(global->GetInternalField(0).As<v8::Value>()->IsUndefined());
30363080
global->SetInternalField(0, v8_num(17));
3037-
CHECK_EQ(17, global->GetInternalField(0)->Int32Value(env.local()).FromJust());
3081+
CHECK_EQ(17, global->GetInternalField(0)
3082+
.As<v8::Value>()
3083+
->Int32Value(env.local())
3084+
.FromJust());
30383085
}
30393086

30403087

@@ -7789,7 +7836,7 @@ void InternalFieldCallback(bool global_gc) {
77897836
.ToLocalChecked();
77907837
handle.Reset(isolate, obj);
77917838
CHECK_EQ(2, obj->InternalFieldCount());
7792-
CHECK(obj->GetInternalField(0)->IsUndefined());
7839+
CHECK(obj->GetInternalField(0).As<v8::Value>()->IsUndefined());
77937840
t1 = new Trivial(42);
77947841
t2 = new Trivial2(103, 9);
77957842

@@ -29699,7 +29746,8 @@ class HiddenDataDelegate : public v8::Context::DeepFreezeDelegate {
2969929746
std::vector<v8::Local<v8::Object>>& children_out) override {
2970029747
int fields = obj->InternalFieldCount();
2970129748
for (int idx = 0; idx < fields; idx++) {
29702-
v8::Local<v8::Value> child_value = obj->GetInternalField(idx);
29749+
v8::Local<v8::Value> child_value =
29750+
obj->GetInternalField(idx).As<v8::Value>();
2970329751
if (child_value->IsExternal()) {
2970429752
if (!FreezeExternal(v8::Local<v8::External>::Cast(child_value),
2970529753
children_out)) {

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

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,9 +46,11 @@ template <typename T>
4646
static void CheckInternalFieldsAreZero(v8::Local<T> value) {
4747
CHECK_EQ(T::kInternalFieldCount, value->InternalFieldCount());
4848
for (int i = 0; i < value->InternalFieldCount(); i++) {
49-
CHECK_EQ(0, value->GetInternalField(i)
50-
->Int32Value(CcTest::isolate()->GetCurrentContext())
51-
.FromJust());
49+
v8::Local<v8::Value> field =
50+
value->GetInternalField(i).template As<v8::Value>();
51+
CHECK_EQ(
52+
0,
53+
field->Int32Value(CcTest::isolate()->GetCurrentContext()).FromJust());
5254
}
5355
}
5456

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3667,23 +3667,27 @@ UNINITIALIZED_TEST(SnapshotCreatorTemplates) {
36673667
.ToLocalChecked()
36683668
->ToObject(context)
36693669
.ToLocalChecked();
3670-
v8::Local<v8::Object> b =
3671-
a->GetInternalField(0)->ToObject(context).ToLocalChecked();
3672-
v8::Local<v8::Object> c =
3673-
b->GetInternalField(0)->ToObject(context).ToLocalChecked();
3670+
v8::Local<v8::Object> b = a->GetInternalField(0)
3671+
.As<v8::Value>()
3672+
->ToObject(context)
3673+
.ToLocalChecked();
3674+
v8::Local<v8::Object> c = b->GetInternalField(0)
3675+
.As<v8::Value>()
3676+
->ToObject(context)
3677+
.ToLocalChecked();
36743678

36753679
InternalFieldData* a1 = reinterpret_cast<InternalFieldData*>(
36763680
a->GetAlignedPointerFromInternalField(1));
3677-
v8::Local<v8::Value> a2 = a->GetInternalField(2);
3681+
v8::Local<v8::Value> a2 = a->GetInternalField(2).As<v8::Value>();
36783682

36793683
InternalFieldData* b1 = reinterpret_cast<InternalFieldData*>(
36803684
b->GetAlignedPointerFromInternalField(1));
3681-
v8::Local<v8::Value> b2 = b->GetInternalField(2);
3685+
v8::Local<v8::Value> b2 = b->GetInternalField(2).As<v8::Value>();
36823686

3683-
v8::Local<v8::Value> c0 = c->GetInternalField(0);
3687+
v8::Local<v8::Value> c0 = c->GetInternalField(0).As<v8::Value>();
36843688
InternalFieldData* c1 = reinterpret_cast<InternalFieldData*>(
36853689
c->GetAlignedPointerFromInternalField(1));
3686-
v8::Local<v8::Value> c2 = c->GetInternalField(2);
3690+
v8::Local<v8::Value> c2 = c->GetInternalField(2).As<v8::Value>();
36873691

36883692
CHECK(c0->IsUndefined());
36893693

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,13 +63,15 @@ class ValueSerializerTest : public TestWithIsolate {
6363
StringFromUtf8("value"),
6464
[](Local<String> property, const PropertyCallbackInfo<Value>& info) {
6565
CHECK(i::ValidateCallbackInfo(info));
66-
info.GetReturnValue().Set(info.Holder()->GetInternalField(0));
66+
info.GetReturnValue().Set(
67+
info.Holder()->GetInternalField(0).As<v8::Value>());
6768
});
6869
function_template->InstanceTemplate()->SetAccessor(
6970
StringFromUtf8("value2"),
7071
[](Local<String> property, const PropertyCallbackInfo<Value>& info) {
7172
CHECK(i::ValidateCallbackInfo(info));
72-
info.GetReturnValue().Set(info.Holder()->GetInternalField(1));
73+
info.GetReturnValue().Set(
74+
info.Holder()->GetInternalField(1).As<v8::Value>());
7375
});
7476
for (Local<Context> context :
7577
{serialization_context_, deserialization_context_}) {
@@ -2884,6 +2886,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint32) {
28842886
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
28852887
uint32_t value = 0;
28862888
EXPECT_TRUE(object->GetInternalField(0)
2889+
.As<v8::Value>()
28872890
->Uint32Value(serialization_context())
28882891
.To(&value));
28892892
WriteExampleHostObjectTag();
@@ -2915,9 +2918,11 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripUint64) {
29152918
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
29162919
uint32_t value = 0, value2 = 0;
29172920
EXPECT_TRUE(object->GetInternalField(0)
2921+
.As<v8::Value>()
29182922
->Uint32Value(serialization_context())
29192923
.To(&value));
29202924
EXPECT_TRUE(object->GetInternalField(1)
2925+
.As<v8::Value>()
29212926
->Uint32Value(serialization_context())
29222927
.To(&value2));
29232928
WriteExampleHostObjectTag();
@@ -2955,6 +2960,7 @@ TEST_F(ValueSerializerTestWithHostObject, RoundTripDouble) {
29552960
.WillRepeatedly(Invoke([this](Isolate*, Local<Object> object) {
29562961
double value = 0;
29572962
EXPECT_TRUE(object->GetInternalField(0)
2963+
.As<v8::Value>()
29582964
->NumberValue(serialization_context())
29592965
.To(&value));
29602966
WriteExampleHostObjectTag();

0 commit comments

Comments
 (0)