1329247 - Rooted may be stored on zone list that doesn't match its contents

Status: RESOLVED WORKSFORME

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

Since bug 1266887 we link Rooteds on the stack into a linked list owned by the context's zone.  The aim was to move away from having a single list for the runtime and to eventually allow marking only roots for collected zones.

However there's no check that the contents of the Rooted are actually in this zone and there are a few places where we end up with a Rooted on one zone's list containing something that's in another zone.

At the moment we mark Rooteds for all zones in a runtime.  However if the context had entered a compartment owned by a different runtime (e.g. the self hosting compartment) then we would end up not marking the root (as well as this being racing when adding/remove Rooteds from the list).

I don't know if this happens in practice but it seems best to disallow it.  

Adding checks for this is not trival though - Rooted can be templated on many types and is exported outside of the engine where we don't have concrete type definitions, so we can't easily get the zone for the contents but must call into the engine to do this.

Also, ensuring we enter the correct compartment before creating a Rooted can be a pain.  For example when creating a new global we don't know the compartment in advance so have to store it unrooted first, then enter the compartment and root it.  And functions that deal with things from different compartments (e.g. JSRuntime::cloneSelfHostedFunctionScript) end up with several AutoEnterCompartments interleaved between the Rooted definitions.

Maybe we should add a new Rooted constructor that explicitly takes a zone or dynamically works out where to put it.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1324512-assertions WIP

I wrote some assertions for this but there's a ton of places that need to be fixed.

One big one is our wrap API that takes a mutable handle containing an object in one zone and sets it to a wrapper that may be in a different zone.

Comment 2

[Jan de Mooij [:jandem]]

Just curious, how much are the per-Zone lists buying us in practice? It sounds like a neat idea but difficult to do correctly.

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Jan de Mooij [:jandem] from comment #2)
Well they're not buying us too much right now.  In future we could use these to skip marking roots for zones we aren't collecting, which would be nice.  I think this will be more important when we have a multithreaded setup.

Comment 4

[Jon Coppeard (:jonco)]

Since Brian's changes in bug 1325050 we store the Rooteds in a list on the contact again, so this problem no longer happens.