1254376 - [jsdbg2] GlobalObject's debugger vector is sort of weak, so it needs read barriers

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jim Blandy :jimb]

Access to a GlobalObject's observing Debuggers should use a read barrier.

A GlobalObject's vector of the js::Debuggers of which it is a debuggee is not traced; rather, its contents are edited by code in js/src/vm/Debugger.cpp that copes with Debugger objects and GlobalObjects being GC'd. However, this vector is used to notify observing Debuggers of activity that takes place in the global's scope. As a consequence, we can end up invoking hooks on Debuggers that the incremental GC has no way of knowing are live.

Bug 1252453 is an example of the kind of problem this engenders.

The classic problem with finalizers is that it's hard to tell what order to run them in, because the collector doesn't understand the dependencies objects have on each other. There's a bunch of hair in Debugger.cpp, handling two things at once and running steps from the sweep phase and other such merriment which is clearly trying to cope with these problems. I'm sure there's a more principled way to address this.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1254376-debugger-barrier

What do you think about relaxing the type constraints on barriers to allow us to use them with non-GC pointers?

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 8731664 [details] [diff] [review]
bug1254376-debugger-barrier

Review of attachment 8731664 [details] [diff] [review]:
-----------------------------------------------------------------

In this case I think it makes sense. My only worry is that someone will do Heap<LargeContainer> and then we'll be stuck trying to figure out why the GC is janking. I guess the lack of barrier methods on our container classes prevents this to a certain degree.

I'm not against this, given the benefits, but we should be sure to update the documentation to make it clearer where barriers should and should not be used.

Comment 3

[Jon Coppeard (:jonco)]

Attachment: bug1254376-debugger-barrier v2

Comment 4

[Jim Blandy :jimb]

Comment on attachment 8731804 [details] [diff] [review]
bug1254376-debugger-barrier v2

Review of attachment 8731804 [details] [diff] [review]:
-----------------------------------------------------------------

This passes all tests? Delightful.

Comment 5

[Jim Blandy :jimb]

Thanks very much for the patch, Jon!

Comment 6

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a803bb1e41a2

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a803bb1e41a2