1001355 - IsAboutToBeFinalized overload for VMFunction pointers is backwards

Status: RESOLVED FIXED

(Core :: JavaScript: GC, defect)

Description

[Jon Coppeard (:jonco)]

I just spotted that IsAboutToBeFinalized(const js::jit::VMFunction **vmfunc) always returns true, when in fact it should always return false.

This was because I renamed the function from IsMarked() in bug 790338 but didn't invert the result.  The same thing happened for the IonCode overload but was fixed in bug 816493.

This means that the functionWrappers_ map in JitRuntime will always be cleared on GC.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1001355-is-about-to-be-finalized

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

This code seems pretty over-generalized. We appear to have only one use of WeakCache, and in that case the key is a VMFunction*. Can we just assume, in WeakCache, that the key will never be finalized? Then we can remove the weird overload of IsAboutToFinalized for VMFunction. It seems pretty error-prone to have that around since it doesn't do what you expect at all.

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Bill McCloskey (:billm) from comment #2)
Well the reason I saw this was because Till was talking about adding a new use of WeakCache (from JSScript to LazyScript, or the other way around I don't remember), which would require this to work with keys that can be finalized.

Another option might be to add IsAboutToBeFinalized overload on void* to catch all non-GC things.

Comment 4

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 8412683 [details] [diff] [review]
bug1001355-is-about-to-be-finalized

Well, all right.

Comment 5

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/4b91913320ef

Comment 6

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/4b91913320ef