New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 885125 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 2
Type: Bug

Blocking:
issue 843903



Sign in to add a comment

Handle non-API types in cross-component tracing

Project Member Reported by mlippautz@chromium.org, Sep 18

Issue description

JSArrayBuffer is always allocated with embedder field count of two [1] so that it can be attached to a ScriptWrappable whenever needed [2].

V8's marker expects objects that use wrapper tracing to be of type JSApiObject [3] and ignores any other types that also have the back references set.

It manifests in the following way using HeapUnifiedGarbageCollection:
1. JSArrayBuffer is attached as a non-mainworld wrapper to a SW
2. SW is not held alive from the main world anymore.
3. JSArrayBuffer is still alive in the non-main world
4. The handle in the v8::GlobalValueMap is kept alive (it's a phantom handle that points to a live object).
5. The Blink-side object is collected as it was not traced through from V8.
6. The next non-unified GC will try to use V8's handles to find the SW objects and crash because the object was reclaimed.

Since the other direction (Blink->V8) is fine and DOMArrayBuffer is a leaf object for wrapper tracing there should be no consequences on master.

I am unsure of the scenario where something would be alive in a non-mainworld but not in the mainworld but it looks like that can hapen.

[1] https://cs.chromium.org/chromium/src/v8/include/v8.h?sq=package:chromium&g=0&l=4437
[2] https://cs.chromium.org/chromium/src/out/Debug/gen/third_party/blink/renderer/bindings/core/v8/v8_array_buffer.cc?sq=package:chromium&g=0&l=67
[3] https://cs.chromium.org/chromium/src/v8/src/heap/mark-compact-inl.h?q=VisitJSApiObject&sq=package:chromium&g=0&l=72
 
The same thing is true for ArrayBufferView which includes all typed arrays.

https://cs.chromium.org/chromium/src/v8/include/v8.h?sq=package:chromium&g=0&l=4633
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 19

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/fe566be004fd4ad351db15e84ea346887c423033

commit fe566be004fd4ad351db15e84ea346887c423033
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Wed Sep 19 14:14:06 2018

[heap] Concurrently process wrapper objects

Concurrently process objects and only read embedder fields on the main
thread.

Also prepares the concurrent marking infrastructure to plug this
processing into different types.

Bug:  chromium:885125 , chromium:843903
Change-Id: I23b7f778c16cff118dec93e11e2bbd02aaf11a78
Reviewed-on: https://chromium-review.googlesource.com/1231175
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56043}
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/concurrent-marking.cc
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/concurrent-marking.h
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/heap.cc
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/incremental-marking.cc
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/incremental-marking.h
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/mark-compact-inl.h
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/mark-compact.cc
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/src/heap/mark-compact.h
[modify] https://crrev.com/fe566be004fd4ad351db15e84ea346887c423033/test/cctest/heap/test-concurrent-marking.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/c7cd3cc6b183df93f7ba3b88b28ddf0d73c1f953

commit c7cd3cc6b183df93f7ba3b88b28ddf0d73c1f953
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Thu Sep 20 09:04:01 2018

[heap] Introduce sub-visitors for JSArrayBufferView

Introduces visitor methods for
- JSDataView
- JSTypedArray

Bug:  chromium:885125 , chromium:843903
Change-Id: I812eaf0619034641c6998f9d164bee84bc4c6ca2
Reviewed-on: https://chromium-review.googlesource.com/1235693
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56061}
[modify] https://crrev.com/c7cd3cc6b183df93f7ba3b88b28ddf0d73c1f953/src/heap/concurrent-marking.cc
[modify] https://crrev.com/c7cd3cc6b183df93f7ba3b88b28ddf0d73c1f953/src/heap/objects-visiting.h
[modify] https://crrev.com/c7cd3cc6b183df93f7ba3b88b28ddf0d73c1f953/src/objects.cc
[modify] https://crrev.com/c7cd3cc6b183df93f7ba3b88b28ddf0d73c1f953/src/objects/map.h

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 20

The following revision refers to this bug:
  https://chromium.googlesource.com/v8/v8.git/+/4d9f09b513c45d292af618d5a57607c41f821f9e

commit 4d9f09b513c45d292af618d5a57607c41f821f9e
Author: Michael Lippautz <mlippautz@chromium.org>
Date: Thu Sep 20 14:39:22 2018

[heap] Add support for non-API wrapper types

Adds support for tracing wrappers of the following types:
- JSArrayBuffer
- JSDataView
- JSTypedArray

Unlike API objects, these objects are equipped with embedder fields at compile
time and can thus be attached to Blink objects at any time.

Bug:  chromium:885125 , chromium:843903
Change-Id: If2dab4831f42a4edc0748b7071d451fe1953f076
Reviewed-on: https://chromium-review.googlesource.com/1234418
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/master@{#56092}
[modify] https://crrev.com/4d9f09b513c45d292af618d5a57607c41f821f9e/src/heap/concurrent-marking.cc
[modify] https://crrev.com/4d9f09b513c45d292af618d5a57607c41f821f9e/src/heap/mark-compact-inl.h
[modify] https://crrev.com/4d9f09b513c45d292af618d5a57607c41f821f9e/src/heap/mark-compact.h
[modify] https://crrev.com/4d9f09b513c45d292af618d5a57607c41f821f9e/src/objects.cc
[modify] https://crrev.com/4d9f09b513c45d292af618d5a57607c41f821f9e/src/objects.h

Status: Fixed (was: Assigned)

Sign in to add a comment