What are the threading considerations for EmbedderHeapTracer?

[Kenton Varda]

Hi v8-users,

I'm trying to understand how to correctly instrument my wrappers for multi-threaded garbage collection, and it seems I'm doing something wrong. Currently, my code seems to work only in --predictable mode (single-threaded), but I'd like to take advantage of GC in a background thread.

When in multi-threaded mode, I observe that occasionally, when V8 traces into a particular wrapper, the weak handles held by that wrapper have already been collected. The tracing is happening, but V8 seemingly prematurely collects some handles before I get a chance to mark them.

Here are the things I've done:

Per-object:

• When I create a new wrapper object, I give it two internal fields set to the native object pointer and a tracing callback function.
• The tracing callback, when run, invokes RegisterExternalReference() on each persistent handle reachable through the native object.
• Once the internal pointers are set, I invoke SetWeak() (with no arguments) on all handles reachable through the native object.

Globally:

• I call AddGCPrologueCallback() to register a callback for v8::kGCTypeScavenge.
  
• In the scavenge callback, I call VisitWeakHandles() with a visitor that calls MarkActive() on every handle.
• I call SetEmbedderHeapTracer() to register my own heap tracer implementation.
• In the heap tracer's RegisterV8References() method, I invoke each tracing callback using the pair of internal pointers (which calls RegisterExternalReference() on reachable weak handles, as described above).
• Currently, I don't do anything in AdvanceTracing() -- I complete all tracing before RegisterV8References() returns. (It appears this is what Blink does in its unified_heap_controller.cc, so I followed the example.) However, if I instead only save a copy of the pointer list in RegisterV8References() and then do the tracing later in AdvanceTracing(), then I observe problems with prematurely-collected handles more often. I'd guess this is because the underlying problem is a race condition, in which another thread is collecting those handles before I get a chance to trace them.
• I don't do anything in any of the other EmbedderHeapTracer callbacks.

Anything I'm missing here?

I haven't been able to find any documentation on how to use these interfaces. Please let me know if there are docs I missed.

Thanks,

-Kenton

[Daryl Haresign]

What kind of handles are you using? We have seen similar issues (v8 reporting objects that have already been collected) when using kFinalizer weak persistent handles. For us we were not making use of the kFinalizer feature (never resurrecting weak handles) so we simply switched to kParameter to avoid the issue.

The embedder heap tracer is only ever called on the main thread so there are no threading concerns.

[Kenton Varda]

Hi Daryl,

On Sun, Sep 23, 2018 at 4:15 PM 'Daryl Haresign' via v8-users <v8-u...@googlegroups.com> wrote:

What kind of handles are you using? We have seen similar issues (v8 reporting objects that have already been collected) when using kFinalizer weak persistent handles. For us we were not making use of the kFinalizer feature (never resurrecting weak handles) so we simply switched to kParameter to avoid the issue.

I'm not using finalizers.

I read your previous e-mails and I think your situation is different from mine. In your situation, V8 is tracing an object that was already collected. In my situation, V8 is tracing a perfectly live object, but it is prematurely collecting objects reachable *through* the live object. The whole point of tracing is to find out what handles are transitively reachable, but V8 seems occasionally to collect those handles before it has actually done the requisite tracing.

 

The embedder heap tracer is only ever called on the main thread so there are no threading concerns.

In that case it must be parallel scavenges collecting my handles concurrently with the heap tracer running on the main thread. But, I have a scavenge callback as I described which should mark all weak handles as active. Is there something else needed?

-Kenton

[Michael Lippautz]

On Sun, Sep 23, 2018 at 7:54 PM 'Kenton Varda' via v8-users <v8-u...@googlegroups.com> wrote:

Hi v8-users,

I'm trying to understand how to correctly instrument my wrappers for multi-threaded garbage collection, and it seems I'm doing something wrong. Currently, my code seems to work only in --predictable mode (single-threaded), but I'd like to take advantage of GC in a background thread.

When in multi-threaded mode, I observe that occasionally, when V8 traces into a particular wrapper, the weak handles held by that wrapper have already been collected. The tracing is happening, but V8 seemingly prematurely collects some handles before I get a chance to mark them.

fyi: The cross-component tracing used for EmbedderHeapTracer always goes through the main thread, i.e., none of its methods are called concurrently.

 

Here are the things I've done:

Per-object:

• When I create a new wrapper object, I give it two internal fields set to the native object pointer and a tracing callback function.
• The tracing callback, when run, invokes RegisterExternalReference() on each persistent handle reachable through the native object.
• Once the internal pointers are set, I invoke SetWeak() (with no arguments) on all handles reachable through the native object.

Globally:

• I call AddGCPrologueCallback() to register a callback for v8::kGCTypeScavenge.
  
• In the scavenge callback, I call VisitWeakHandles() with a visitor that calls MarkActive() on every handle.

Looks like Isolate::VisitWeakHandles only iterates over weak handles with a non-zero class id. Any chance that this one is still 0?

 

• I call SetEmbedderHeapTracer() to register my own heap tracer implementation.
• In the heap tracer's RegisterV8References() method, I invoke each tracing callback using the pair of internal pointers (which calls RegisterExternalReference() on reachable weak handles, as described above).
• Currently, I don't do anything in AdvanceTracing() -- I complete all tracing before RegisterV8References() returns. (It appears this is what Blink does in its unified_heap_controller.cc, so I followed the example.) However, if I instead only save a copy of the pointer list in RegisterV8References() and then do the tracing later in AdvanceTracing(), then I observe problems with prematurely-collected handles more often. I'd guess this is because the underlying problem is a race condition, in which another thread is collecting those handles before I get a chance to trace them.

The symptom described here hints to the Scavenger collecting objects. From the above description it looks like you wanted to preserve those objects on Scavenge. 

Some ideas:

- Maybe class id is 0 (see above)?

- Maybe the handle are not SetWeak immediately but only after some time where a Scavenge could've happened?

 

• I don't do anything in any of the other EmbedderHeapTracer callbacks.

Anything I'm missing here?

I haven't been able to find any documentation on how to use these interfaces. Please let me know if there are docs I missed.

There's no additional documentation and you already found the existing implementations for wrapper tracing and unified heap garbage collections.

The Scavenger quirks are definitely something we should document though. 

Cheers, -Michael

[Kenton Varda]

Hi Michael,

Thanks for commenting!

On Tue, Sep 25, 2018 at 6:37 AM Michael Lippautz <mlip...@chromium.org> wrote:

Looks like Isolate::VisitWeakHandles only iterates over weak handles with a non-zero class id. Any chance that this one is still 0?

No, I set class ID to 1 just before calling SetWeak() -- for no other purpose than this requirement. :)

 

The symptom described here hints to the Scavenger collecting objects. From the above description it looks like you wanted to preserve those objects on Scavenge. 

Some ideas:

- Maybe class id is 0 (see above)?

- Maybe the handle are not SetWeak immediately but only after some time where a Scavenge could've happened?

But before calling SetWeak(), the handle would be strong, and therefore not collected, right? Do I need to call MarkActive() at the same time as SetWeak() to make sure any currently-running scavenges don't collect it?

-Kenton

 

 

• I don't do anything in any of the other EmbedderHeapTracer callbacks.

Anything I'm missing here?

I haven't been able to find any documentation on how to use these interfaces. Please let me know if there are docs I missed.

There's no additional documentation and you already found the existing implementations for wrapper tracing and unified heap garbage collections.

The Scavenger quirks are definitely something we should document though. 

Cheers, -Michael

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to a topic in the Google Groups "v8-users" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/v8-users/e8LsFC-LNGE/unsubscribe.
To unsubscribe from this group and all its topics, send an email to v8-users+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Michael Lippautz]

Two more ideas:

- V8 is using incremental marking for such wrappers which means that the C++ graph should either (a) be immutable during marking or (b) should keep the marking state consistent with e.g. write barriers.

- Specific issue: When creating an API object while marking is running, V8 may allocate the object as already marked (called black allocation). This means that RegisterV8References() will not be called as the objects are not discovered by the marker. In Blink, we thus issue a write barrier for such objects when establishing a new link.

We've some embedders that don't want to implement the incremental version and they thus set the flag incremental_marking_wrappers to false. You could try disabling that flag but also dropping predictable.

On Tue, Sep 25, 2018 at 6:36 PM 'Kenton Varda' via v8-users <v8-u...@googlegroups.com> wrote:

Hi Michael,

Thanks for commenting!

On Tue, Sep 25, 2018 at 6:37 AM Michael Lippautz <mlip...@chromium.org> wrote:

Looks like Isolate::VisitWeakHandles only iterates over weak handles with a non-zero class id. Any chance that this one is still 0?

No, I set class ID to 1 just before calling SetWeak() -- for no other purpose than this requirement. :)

 

Ack.

 

The symptom described here hints to the Scavenger collecting objects. From the above description it looks like you wanted to preserve those objects on Scavenge. 

Some ideas:

- Maybe class id is 0 (see above)?

- Maybe the handle are not SetWeak immediately but only after some time where a Scavenge could've happened?

But before calling SetWeak(), the handle would be strong, and therefore not collected, right? Do I need to call MarkActive() at the same time as SetWeak() to make sure any currently-running scavenges don't collect it?

Yip, didn't think about this one. That all should work.

 

[Kenton Varda]

I figured this out.

First, some misunderstandings:

1) --predictable was a red herring. It fixed the specific reproduction case I had (seemingly by accident), but did not actually fix the general problem (and had some negative side-effects).

2) The term "write barrier" was confusing me because I'm not well-read in GC literature and I was thinking of a write barrier between threads. So I was assuming you were telling me that I was missing some thread synchronization, but given that only scavenges are multi-threaded and they don't trace through native objects, I didn't get it. But now I've figured out that this is actually a garbage collection term that is not necessarily related to threading.

3) I didn't fully realize that GC being incremental means that V8 interleaves calls to the tracer with regular code execution, meaning that new objects could be created while a trace is in progress. This has almost all the same implications as being multi-threaded, without the actual threading.

So, here's specifically what I did (in the hopes that it helps someone in the future):

* When allocating a new native object, I check if a trace pass is in progress (i.e. TracePrologue() has been called but TraceEpilogue() has not). If so, I add the object to the list of objects that needs to be traced. This handles the specific case that you mentioned, where the object is allocated already-marked, and hence won't otherwise be traced as part of the current tracing pass. It looks like I can't actually do the trace synchronously when the object is created -- V8 doesn't expect RegisterExternalReference() to be called at this time and crashes. Instead, by adding it to the list, my tracer's IsTracingDone() now returns false, which leads V8 to call AdvanceTracing() at some future point, which is where I can then trace the object.

* It's possible that a newly-created object will be destroyed before tracing progresses, if all handles go out-of-scope. Hence, the object's weak callback must also check if a trace is in progress, and must then remove the object from the list of objects to be traced. Otherwise, AdvanceTracing() will end up tracing a dangling pointer.

* If a native object can be modified after creation to add new weak handles, then of course we run into a similar problem: The parent object may already have been traced during the current cycle, and so the child needs to be traced immediately. At present this case does not actually come up in the objects we've instrumented, but it will at some point. I guess I will either need to re-queue the parent object to be traced again, or I'll need to specifically record that the new handle needs to have RegisterExternalRefreence() called when tracing next advances.

In any case, my GC issues seem now to be fixed without any special flags. Hooray!

-Kenton

--

[Michael Lippautz]

Glad that your setup works for you now. Is your project open source? In any case, let us know if you encounter any further issues. 

I also have a few followup questions below to make sure nothing is wrong and potentially improve V8's handling there.

On Tue, Oct 2, 2018 at 6:34 PM 'Kenton Varda' via v8-users <v8-u...@googlegroups.com> wrote:

I figured this out.

First, some misunderstandings:

1) --predictable was a red herring. It fixed the specific reproduction case I had (seemingly by accident), but did not actually fix the general problem (and had some negative side-effects).

That's what I thought, yeah.

 

2) The term "write barrier" was confusing me because I'm not well-read in GC literature and I was thinking of a write barrier between threads. So I was assuming you were telling me that I was missing some thread synchronization, but given that only scavenges are multi-threaded and they don't trace through native objects, I didn't get it. But now I've figured out that this is actually a garbage collection term that is not necessarily related to threading.

Yes, I should've been more specific. Glad you figured it out.

 

3) I didn't fully realize that GC being incremental means that V8 interleaves calls to the tracer with regular code execution, meaning that new objects could be created while a trace is in progress. This has almost all the same implications as being multi-threaded, without the actual threading.

It's slightly simpler as it avoids concurrent access on objects and all the challenges that come with that.

 

So, here's specifically what I did (in the hopes that it helps someone in the future):

* When allocating a new native object, I check if a trace pass is in progress (i.e. TracePrologue() has been called but TraceEpilogue() has not). If so, I add the object to the list of objects that needs to be traced. This handles the specific case that you mentioned, where the object is allocated already-marked, and hence won't otherwise be traced as part of the current tracing pass. It looks like I can't actually do the trace synchronously when the object is created -- V8 doesn't expect RegisterExternalReference() to be called at this time and crashes. Instead, by adding it to the list, my tracer's IsTracingDone() now returns false, which leads V8 to call AdvanceTracing() at some future point, which is where I can then trace the object.

Delaying is usually better as depending on scheduling and idleness it has the potential to run in a less-busy phase of execution. That depends on the kind of platform you are running on though.

On a first look, the synchronous call to RegisterExternalReference should also work though. Any chance you can pass along what's happening via bug or email? 

* It's possible that a newly-created object will be destroyed before tracing progresses, if all handles go out-of-scope. Hence, the object's weak callback must also check if a trace is in progress, and must then remove the object from the list of objects to be traced. Otherwise, AdvanceTracing() will end up tracing a dangling pointer.

How can it go away? IIRC, then you set up the Scavenger correctly so that it would be strong and the currently running Mark-Compact GC should not be able to finish without tracing it.

 

* If a native object can be modified after creation to add new weak handles, then of course we run into a similar problem: The parent object may already have been traced during the current cycle, and so the child needs to be traced immediately. At present this case does not actually come up in the objects we've instrumented, but it will at some point. I guess I will either need to re-queue the parent object to be traced again, or I'll need to specifically record that the new handle needs to have RegisterExternalRefreence() called when tracing next advances.

The is the problem of graph mutation during incremental (or concurrent) garbage collections. As already said, we rely on write barriers to re-visit such objects. You can follow the Blink implementation here. It uses a wrapping type that intercepts each write as manually instrumenting code with write barriers is way too error prone for us.

 

Cheers, -Michael

[Kenton Varda]

On Tue, Oct 2, 2018 at 11:10 AM Michael Lippautz <mlip...@chromium.org> wrote:

Glad that your setup works for you now. Is your project open source? In any case, let us know if you encounter any further issues. 

This is all for the Cloudflare Workers runtime. https://developers.cloudflare.com/workers/about/

At present it is not open source. But, I'd very much like to release our C++<->V8 glue layer at some point soon. We've used a lot of template/macro magic to make it pretty painless to export a C++ interface to JavaScript.

* When allocating a new native object, I check if a trace pass is in progress (i.e. TracePrologue() has been called but TraceEpilogue() has not). If so, I add the object to the list of objects that needs to be traced. This handles the specific case that you mentioned, where the object is allocated already-marked, and hence won't otherwise be traced as part of the current tracing pass. It looks like I can't actually do the trace synchronously when the object is created -- V8 doesn't expect RegisterExternalReference() to be called at this time and crashes. Instead, by adding it to the list, my tracer's IsTracingDone() now returns false, which leads V8 to call AdvanceTracing() at some future point, which is where I can then trace the object.

Delaying is usually better as depending on scheduling and idleness it has the potential to run in a less-busy phase of execution. That depends on the kind of platform you are running on though.

On a first look, the synchronous call to RegisterExternalReference should also work though. Any chance you can pass along what's happening via bug or email? 

If I call RegisterExternalReference() without checking if a trace is in progress, I pretty immediately get this unhelpful error dump:

    #

    # Fatal error in , line 0

    # unreachable code

    #

    #

    #

    #FailureMessage Object: 0x7f638f70b980

Followed by a segfault with this stack trace:

    :? v8::internal::ConcurrentMarking::Run(int, v8::internal::ConcurrentMarking::TaskState*)

    ??:? v8::platform::WorkerThread::Run()

    platform-posix.cc:? v8::base::ThreadEntry(void*)

    glibc-2.24/nptl/pthread_create.c:333 start_thread

    glibc-2.24/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:97 clone

It looks like if make sure only to call it if a trace is in progress, then it works fine.

* It's possible that a newly-created object will be destroyed before tracing progresses, if all handles go out-of-scope. Hence, the object's weak callback must also check if a trace is in progress, and must then remove the object from the list of objects to be traced. Otherwise, AdvanceTracing() will end up tracing a dangling pointer.

How can it go away? IIRC, then you set up the Scavenger correctly so that it would be strong and the currently running Mark-Compact GC should not be able to finish without tracing it.

I may have misspoke slightly here: it appears the effect I was seeing is *not* necessarily related to newly-created objects. Let me restate.

It appears that it's possible that an object registered with EmbedderHeapTracer::RegisterV8References() can actually be collected (i.e. its weak callback is called) between then and EmbedderHeapTracer::AdvanceTracing(). So, in RegisterV8References() I have to make sure to put all the pointers in a map. When an object is destroyed while tracing is active, I have to go remove it from the tracer's map. In AdvanceTracing() I can then trace whatever is still in the map at that point.

I'm not sure what mechanism causes the object to be destroyed immediately after it has been registered for tracing. But, AFAICT, the object is legitimately no longer reachable -- I'm not seeing any objects being destroyed that were still in-use.

-Kenton

[Michael Lippautz]

On Tue, Oct 2, 2018 at 10:12 PM 'Kenton Varda' via v8-users <v8-u...@googlegroups.com> wrote:

On Tue, Oct 2, 2018 at 11:10 AM Michael Lippautz <mlip...@chromium.org> wrote:

Glad that your setup works for you now. Is your project open source? In any case, let us know if you encounter any further issues. 

This is all for the Cloudflare Workers runtime. https://developers.cloudflare.com/workers/about/

At present it is not open source. But, I'd very much like to release our C++<->V8 glue layer at some point soon. We've used a lot of template/macro magic to make it pretty painless to export a C++ interface to JavaScript.

Cool :)

 

* When allocating a new native object, I check if a trace pass is in progress (i.e. TracePrologue() has been called but TraceEpilogue() has not). If so, I add the object to the list of objects that needs to be traced. This handles the specific case that you mentioned, where the object is allocated already-marked, and hence won't otherwise be traced as part of the current tracing pass. It looks like I can't actually do the trace synchronously when the object is created -- V8 doesn't expect RegisterExternalReference() to be called at this time and crashes. Instead, by adding it to the list, my tracer's IsTracingDone() now returns false, which leads V8 to call AdvanceTracing() at some future point, which is where I can then trace the object.

Delaying is usually better as depending on scheduling and idleness it has the potential to run in a less-busy phase of execution. That depends on the kind of platform you are running on though.

On a first look, the synchronous call to RegisterExternalReference should also work though. Any chance you can pass along what's happening via bug or email? 

If I call RegisterExternalReference() without checking if a trace is in progress, I pretty immediately get this unhelpful error dump:

    #

    # Fatal error in , line 0

    # unreachable code

    #

    #

    #

    #FailureMessage Object: 0x7f638f70b980

Followed by a segfault with this stack trace:

    :? v8::internal::ConcurrentMarking::Run(int, v8::internal::ConcurrentMarking::TaskState*)

    ??:? v8::platform::WorkerThread::Run()

    platform-posix.cc:? v8::base::ThreadEntry(void*)

    glibc-2.24/nptl/pthread_create.c:333 start_thread

    glibc-2.24/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:97 clone

It looks like if make sure only to call it if a trace is in progress, then it works fine.

Yes, that's correct. RegisterExternalReference() requires tracing to be in progress. However, you should be able to call it from anywhere on the main thread, e.g., also when V8 registers a reference through  RegisterV8References. What I meant is that there's no need to *only* process the objects during AdvanceTracing.

 

* It's possible that a newly-created object will be destroyed before tracing progresses, if all handles go out-of-scope. Hence, the object's weak callback must also check if a trace is in progress, and must then remove the object from the list of objects to be traced. Otherwise, AdvanceTracing() will end up tracing a dangling pointer.

How can it go away? IIRC, then you set up the Scavenger correctly so that it would be strong and the currently running Mark-Compact GC should not be able to finish without tracing it.

I may have misspoke slightly here: it appears the effect I was seeing is *not* necessarily related to newly-created objects. Let me restate.

It appears that it's possible that an object registered with EmbedderHeapTracer::RegisterV8References() can actually be collected (i.e. its weak callback is called) between then and EmbedderHeapTracer::AdvanceTracing(). So, in RegisterV8References() I have to make sure to put all the pointers in a map. When an object is destroyed while tracing is active, I have to go remove it from the tracer's map. In AdvanceTracing() I can then trace whatever is still in the map at that point.

I'm not sure what mechanism causes the object to be destroyed immediately after it has been registered for tracing. But, AFAICT, the object is legitimately no longer reachable -- I'm not seeing any objects being destroyed that were still in-use.

That should not be possible when everything is properly set up. Any object can only go away

- on Scavenge, which should be prohibited by marking it as active in the prologue callbacks

- on Mark-Compact, which should not be possible as tracing is still in progress.

The only thing I can think of where this could happen is when V8 calls AdvanceTracing where it *must* trace all objects that are cached (indicated by completing actions in the old version, or with infinite as timeout in the new version) and the embedder does not follow this policy. The Mark-Compact collector implements various stages of weakness processing and an object could be collected if AdvanceTracing(infinity) does not fully process all objects on the embedder side. Then the next AdvanceTracing(infinity) call could see dead objects. 

Any chance that you are not completely draining all objects when it's required?

Cheers, -Michael

[Kenton Varda]

On Wed, Oct 3, 2018 at 1:49 AM Michael Lippautz <mlip...@chromium.org> wrote:

It appears that it's possible that an object registered with EmbedderHeapTracer::RegisterV8References() can actually be collected (i.e. its weak callback is called) between then and EmbedderHeapTracer::AdvanceTracing(). So, in RegisterV8References() I have to make sure to put all the pointers in a map. When an object is destroyed while tracing is active, I have to go remove it from the tracer's map. In AdvanceTracing() I can then trace whatever is still in the map at that point.

I'm not sure what mechanism causes the object to be destroyed immediately after it has been registered for tracing. But, AFAICT, the object is legitimately no longer reachable -- I'm not seeing any objects being destroyed that were still in-use.

That should not be possible when everything is properly set up. Any object can only go away

- on Scavenge, which should be prohibited by marking it as active in the prologue callbacks

- on Mark-Compact, which should not be possible as tracing is still in progress.

This seems to have to do with MarkIndependent().

When I create a wrapper, I create one weak handle that is held by the wrapper itself and exists solely to detect when the wrapper should be destroyed (via the weak callback). On this handle I call MarkIndependent(). I don't quite remember why -- I probably cargo-culted it from somewhere.

It appears that handles marked independent are not visited by VisitWeakHandles(). Hence, the scavenge prologue callback does not mark the handle active. If this is the only handle pointing to the object, then, it will be scavenged. But independent handles apparently are still traced, hence it can be passed to RegisterV8References() before it is scavenged.

I think this behavior is actually overall a good thing for me. If the only handle left pointing at an object is this fake handle that exists only to invoke the destructor, then that object might as well be collected immediately. Waiting through the next full trace cycle before collecting it is a waste.

However, I notice that MarkIndependent() is deprecated, with the message: "Objects are always considered independent." The message suggests that MarkIndependent() has no effect, but that seems inaccurate. Is there a preferred alternative to get the behavior I want here?

The only thing I can think of where this could happen is when V8 calls AdvanceTracing where it *must* trace all objects that are cached (indicated by completing actions in the old version, or with infinite as timeout in the new version) and the embedder does not follow this policy. The Mark-Compact collector implements various stages of weakness processing and an object could be collected if AdvanceTracing(infinity) does not fully process all objects on the embedder side. Then the next AdvanceTracing(infinity) call could see dead objects. 

Any chance that you are not completely draining all objects when it's required?

Currently, my AdvanceTracing() callback always drains the whole queue. I haven't tried to implement deadlines.

-Kenton