[api] Fix handle leak when getting Context embedder data

addaleax · Commit Bot · commit e5dbc95cc0bf · 2019-10-28T12:10:56.000Z
The `Context::SlowGetAlignedPointerFromEmbedderData()` method returns a pointer, so the fact that it allocates handles is not obvious to the caller. Since this is the slow path anyway, simply add a handle scope inside of it. The tests are also modified to perform the same check for the `Object` equivalent of this method. Change-Id: I5f03c9a7b70b3a17315609df021606a53c9feb2d Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1879902 Reviewed-by: Yang Guo <yangguo@chromium.org> Commit-Queue: Yang Guo <yangguo@chromium.org> Cr-Commit-Position: refs/heads/master@{#64583}
diff --git a/src/api/api.cc b/src/api/api.cc
@@ -1312,6 +1312,7 @@ void Context::SetEmbedderData(int index, v8::Local<Value> value) {
 
 void* Context::SlowGetAlignedPointerFromEmbedderData(int index) {
   const char* location = "v8::Context::GetAlignedPointerFromEmbedderData()";
+  HandleScope handle_scope(GetIsolate());
   i::Handle<i::EmbedderDataArray> data =
       EmbedderDataFor(this, index, false, location);
   if (data.is_null()) return nullptr;
diff --git a/test/cctest/test-api.cc b/test/cctest/test-api.cc
@@ -2956,8 +2956,11 @@ THREADED_TEST(SetAlignedPointerInInternalFields) {
 
   obj->SetAlignedPointerInInternalFields(2, indices, values);
   CcTest::CollectAllGarbage();
-  CHECK_EQ(heap_allocated_1, obj->GetAlignedPointerFromInternalField(0));
-  CHECK_EQ(heap_allocated_2, obj->GetAlignedPointerFromInternalField(1));
+  {
+    v8::SealHandleScope no_handle_leak(isolate);
+    CHECK_EQ(heap_allocated_1, obj->GetAlignedPointerFromInternalField(0));
+    CHECK_EQ(heap_allocated_2, obj->GetAlignedPointerFromInternalField(1));
+  }
 
   indices[0] = 1;
   indices[1] = 0;
@@ -3010,6 +3013,7 @@ THREADED_TEST(EmbedderDataAlignedPointers) {
   }
   CcTest::CollectAllGarbage();
   for (int i = 0; i < 100; i++) {
+    v8::SealHandleScope no_handle_leak(env->GetIsolate());
     CHECK_EQ(AlignedTestPointer(i), env->GetAlignedPointerFromEmbedderData(i));
   }
 }

Original file line number	Diff line number	Diff line change
`@@ -2956,8 +2956,11 @@ THREADED_TEST(SetAlignedPointerInInternalFields) {`
`2956`	`2956`
`2957`	`2957`	`obj->SetAlignedPointerInInternalFields(2, indices, values);`
`2958`	`2958`	`CcTest::CollectAllGarbage();`
`2959`		`- CHECK_EQ(heap_allocated_1, obj->GetAlignedPointerFromInternalField(0));`
`2960`		`- CHECK_EQ(heap_allocated_2, obj->GetAlignedPointerFromInternalField(1));`
	`2959`	`+ {`
	`2960`	`+ v8::SealHandleScope no_handle_leak(isolate);`
	`2961`	`+ CHECK_EQ(heap_allocated_1, obj->GetAlignedPointerFromInternalField(0));`
	`2962`	`+ CHECK_EQ(heap_allocated_2, obj->GetAlignedPointerFromInternalField(1));`
	`2963`	`+ }`
`2961`	`2964`
`2962`	`2965`	`indices[0] = 1;`
`2963`	`2966`	`indices[1] = 0;`
`@@ -3010,6 +3013,7 @@ THREADED_TEST(EmbedderDataAlignedPointers) {`
`3010`	`3013`	`}`
`3011`	`3014`	`CcTest::CollectAllGarbage();`
`3012`	`3015`	`for (int i = 0; i < 100; i++) {`
	`3016`	`+ v8::SealHandleScope no_handle_leak(env->GetIsolate());`
`3013`	`3017`	`CHECK_EQ(AlignedTestPointer(i), env->GetAlignedPointerFromEmbedderData(i));`
`3014`	`3018`	`}`
`3015`	`3019`	`}`