V8 minor GC may mis-collect wrappers?

[Kentaro Hara]

Hi

(If you're not interested in the details of the wrapper tracing, stop reading now :-)

I noticed that there may be some cases where a V8 minor GC mis-collect wrappers (if I'm not mis-understanding something).

Imagine the following setup:

// X.idl

interface X { attribute Y yyy; }

// X.h

class X {

  DEFINE_TRACE_WRAPPERS() { visitor->TraceWrappers(yyy_) };

  TraceWrapperMember<Y> yyy_;

};

// Y.idl

[DependentLifetime]  // This annotates that Y is traced by TraceWrappers.

interface Y { }

// Window.idl

interface Window { attribute X xxx; }

In this example, the expected behavior is that Y's wrapper is kept alive as long as X's wrapper is alive.

However, there can be a scenario where Y's wrapper is mis-collected while X's wrapper is alive:

0) Assume that X's wrapper is unmodified (i.e., doesn't have any expando) but Y's wrapper is modified. For example, call window.xxx.yyy.foo = 11111.

1) A minor GC is triggered. The minor GC collects X's wrapper because it is unmodified. The minor GC doesn't collect Y's wrapper because it is annotated as [DependentLifetime]. (i.e., [DependentLifetime] is an IDL extended attribute that indicates that V8 cannot collect the wrapper without calling the wrapper tracing. Since the minor GC does not call the wrapper tracing, it simply gives up collecting wrappers annotated with [DependentLifetime].)

2) A major GC is triggered. The major GC calls the wrapper tracing. However, since X's wrapper is already gone, Y's wrapper is not traced. Hence Y's wrapper gets collected.

3) When you call window.xxx, X's wrapper is recreated. This is fine because the old X's wrapper was unmodified and thus there is no way for the script to notice that X's wrapper is recreated.

4) When you call window.xxx.yyy.foo, this causes a problem. Since Y's wrapper is already gone, window.xxx.yyy recreates Y's wrapper. Then window.xxx.yyy.foo returns undefined instead of 11111...

I think there are two approaches to avoid the problem:

a) Add [DependentLifetime] to objects that are calling TraceWrappers. In the above example, add [DependentLifetime] to X.

b) Make the minor GC call the wrapper tracing. (Then we can drop [DependentLifetime] entirely.)

I don't think a) is an option because we will end up with adding [DependentLifetime] to almost all DOM objects (we're planning to trace all DOM attributes in the future). On the other hand, I'm not sure if b) is acceptable from the performance perspective.

Do you have any idea?

(Note: I don't think this is a new problem introduced by the wrapper tracing. We had the same problem in the [SetWrapperReferenceFrom/To] world -- a very long-standing bug.)

--

Kentaro Hara, Tokyo, Japan

[Yuki Shiino]

Once the wrapper tracing is used everywhere entirely in Blink, I guess that the problem would be gone?

In your scenario, I think that we should have:

class DOMWindow {

  DEFINE_TRACE_WRAPPERS() {

    visitor->TraceWrappers(xxx_);

  }

};

class X {

  DEFINE_TRACE_WRAPPERS() {

    visitor->TraceWrappers(yyy_);

  }

};

then, yyy_'s wrapper is reachable.

My understanding is that, unless we have a complete graph, GC won't work correctly.

Cheers,

Yuki Shiino

--
You received this message because you are subscribed to the Google Groups "platform-architecture-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jynrG3tf7OSL8C0UJL%2B_hYa2-BoHyNUO8Do5orxUrN95A%40mail.gmail.com.

[Yuki Shiino]

Hum, "complete graph" is a math term, and I didn't mean it.  I meant that our graph of wrapper tracing is in the middle of construction and imperfect, so there could be a lot of missing links.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

[Kentaro Hara]

On Wed, May 10, 2017 at 9:48 PM, Yuki Shiino <yukis...@chromium.org> wrote:

Once the wrapper tracing is used everywhere entirely in Blink, I guess that the problem would be gone?

It will somewhat improve the situation but the problem won't be gone.

In your scenario, I think that we should have:

class DOMWindow {

  DEFINE_TRACE_WRAPPERS() {

    visitor->TraceWrappers(xxx_);

  }

};

class X {

  DEFINE_TRACE_WRAPPERS() {

    visitor->TraceWrappers(yyy_);

  }

};

then, yyy_'s wrapper is reachable.

If DOMWindow's wrapper is unmodified, the minor GC will collect the DOMWindow's wrapper. Then the next major GC will collect XXX's wrapper and YYY's wrapper. Then expando on YYY's wrapper will be lost.

In the first place, the argument that the problem is gone if the wrapper tracing is used everywhere in Blink is saying that the minor GC cannot collect any wrapper. Which should be problematic :)

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsubsc...@chromium.org.

To post to this group, send email to platform-architecture-dev@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jynrG3tf7OSL8C0UJL%2B_hYa2-BoHyNUO8Do5orxUrN95A%40mail.gmail.com.

[Kentaro Hara]

I guess that the following code could cause the problem. (I haven't tested it though.)

list = node.getElementsByTagName("div");

list.foo = 1111;

node = null;

list = null;

minorGC();  // This collects the node wrapper because it is unmodified.

majorGC();  // This collects the node list wrapper because it is not traced (because the node wrapper is already gone).

node.getElementsByTagName("div").foo;  // Returns undefined instead of 1111...

[Yuki Shiino]

I talked with haraken@ offline, and I think that we agree some points.

a) With the current implementation, the global proxy is one of the root objects for V8 GC.

The global proxy makes the global object alive, and the global object (= window) makes the DOMWindow alive.  So we can assume that DOMWindows are always alive (as long as it's still used).

b) Like node lists or something that we'd like to make alive, we should have a chain of references from one of the root objects to the objects that need to be alive.  As long as we have a chain of wrapper tracing references, this works.

c) For empty wrapper objects that we can re-create from Blink's DOM objects, it's okay that minor V8 GC collects them.

d) Currently, we don't have necessary and sufficient reference-chains of wrapper tracing in our code base, there seem several missing links.  We're not sure if it's realistically easy to make it available.  But technically, wrapper tracing can solve the problem.

e) The Unified GC (V8 GC + Blink GC) might solve the problem more easily than the wrapper tracing.

This is summary from my point of view.

Cheers,

Yuki Shiino

[Kentaro Hara]

Yeah.

- Currently there are cases where wrappers are mis-collected.

- I was thinking that this is because the minor GC is not calling the wrapper tracing but the observation was wrong. The problem won't be gone even if we make the minor GC call the wrapper tracing.

- To fix the problem we need to a) add more necessary references with the wrapper tracing or b) implement the unified GC and keep alive all wrappers reachable from Oilpan.

[Michael Lippautz]

FYI: I didn't have time yet to follow up on this one and read up on the discussion. I hope I will get to this one soon.

I am actually currently working on V8's minor GC and I am also looking into how we handle global handles (also including unmodified wrappers).

-Michael

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architect...@chromium.org.
To post to this group, send email to platform-arc...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jynrG3tf7OSL8C0UJL%2B_hYa2-BoHyNUO8Do5orxUrN95A%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

--

Kentaro Hara, Tokyo, Japan

[Kentaro Hara]

This discussion is very involved and would be hard to catch up on this thread. Feel free to ping me via IM.

To unsubscribe from this group and stop receiving emails from it, send an email to platform-architecture-dev+unsub...@chromium.org.
To post to this group, send email to platform-architecture-dev@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/platform-architecture-dev/CABg10jynrG3tf7OSL8C0UJL%2B_hYa2-BoHyNUO8Do5orxUrN95A%40mail.gmail.com.

--

Kentaro Hara, Tokyo, Japan

--

Kentaro Hara, Tokyo, Japan

--

Kentaro Hara, Tokyo, Japan