Skip to content

Commit

Permalink
deps: V8: cherry-pick e5dbc95
Browse files Browse the repository at this point in the history
Original commit message:

    [api] Fix handle leak when getting Context embedder data

    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 <[email protected]>
    Commit-Queue: Yang Guo <[email protected]>
    Cr-Commit-Position: refs/heads/master@{#64583}

Refs: v8/v8@e5dbc95
Fixes: #30127
PR-URL: #30130
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
  • Loading branch information
Gabriel Schulhof authored and targos committed Nov 5, 2019
1 parent 5469811 commit 104bfb9
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 3 deletions.
2 changes: 1 addition & 1 deletion common.gypi
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@

# Reset this number to 0 on major V8 upgrades.
# Increment by one for each non-official patch applied to deps/v8.
'v8_embedder_string': '-node.17',
'v8_embedder_string': '-node.18',

##### V8 defaults for Node.js #####

Expand Down
1 change: 1 addition & 0 deletions deps/v8/src/api/api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1314,6 +1314,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;
Expand Down
8 changes: 6 additions & 2 deletions deps/v8/test/cctest/test-api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2955,8 +2955,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;
Expand Down Expand Up @@ -3009,6 +3012,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));
}
}
Expand Down

0 comments on commit 104bfb9

Please sign in to comment.