Skip to content

Commit f970087

Browse 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 <[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: #49419 Refs: v8/v8@0aa622e Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Yagiz Nizipli <[email protected]>
1 parent a3020dd commit f970087

File tree

8 files changed

+103
-36
lines changed

8 files changed

+103
-36
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.15',
39+
'v8_embedder_string': '-node.16',
4040

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

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

Lines 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

‎deps/v8/samples/process.cc

Lines 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
}

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

Lines 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;

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

Lines 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)) {

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

Lines 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

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

Lines 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

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

Lines 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)