As Waldo points out, Handle::repoint is a footgun, as it allows us to create Handles that refer to things that have a *shorter* lifetime than the handle, which we couldn't otherwise: Handle h1 = something; { Rooted r(cx, blech); Handle h2 = r; h1.repoint(h2); } /* h1 is garbage */ There are only a few uses of Handle::repoint: CompileOptions, C1Spewer, and IonSpewer. All of those can be changed to use different things, perhaps the AutoRooter type proposed in bug 892643.

Is this still valid with the depending bugs fixed?

As long as there exist uses of Handle<T>::repoint, then this bug is valid. It looks like C1Spewer and IonSpewer are the ones that still exist.

I though it would be simple to replace the handles that we call repoint() on with PersistentRooteds, which can simply be assigned to. However I ended up having to refactor IonSpewer so that it's stored as a pointer in the JitRuntime so that it can be initialised after we're created a JSRuntime. This has the additional advantage of not using a global to store this state, which presumable didn't work if there were multiple runtimes.

And now we can just kill Handle::repoint()

Comment on attachment 8424894 [details] [diff] [review] 1 - refactor-ion-spewer Review of attachment 8424894 [details] [diff] [review]: ----------------------------------------------------------------- IonSpewer::function is unused, looks like. I just removed it and I can compile a debug shell; can you do that instead?

(In reply to Jon Coppeard (:jonco) from comment #3) > This has the additional advantage of not using a global to store this state, > which presumable didn't work if there were multiple runtimes. Good point. The spewer is mostly used in the shell though where it's not a problem, and IIUC even with the spewer-per-runtime we'll still fopen the same file for writing from multiple threads (not sure what will happen there). As the spewer is just a debug-only thing, I think it's fine to leave it as a singleton for now until people actually run into problems with it.

Comment on attachment 8424896 [details] [diff] [review] 2 - remove-repoint Review of attachment 8424896 [details] [diff] [review]: ----------------------------------------------------------------- \o/ r=me

Thanks, I should have spotted that! This is much simpler.

Comment on attachment 8425345 [details] [diff] [review] 1 - refector-ion-spewer v2 Review of attachment 8425345 [details] [diff] [review]: ----------------------------------------------------------------- Thanks!

https://hg.mozilla.org/integration/mozilla-inbound/rev/179d30a83cb5 https://hg.mozilla.org/integration/mozilla-inbound/rev/687938abd5de

https://hg.mozilla.org/mozilla-central/rev/179d30a83cb5 https://hg.mozilla.org/mozilla-central/rev/687938abd5de