548215 - Don't support finalization by the embedding before JSGC_FINALIZE_END state

Status: RESOLVED INCOMPLETE

(Core :: JavaScript Engine, enhancement, P3)

Description

[Andreas Gal :gal]

For API compatibility we supported this using a pretty sketchy hack in jsgc.cpp. This is in the way of weak pointer support (i.e. ephemeron tables). I will update mdc when this patch lands and notify the mailing list. I doubt anyone else really uses these hooks though.

Comment 1

[Andreas Gal :gal]

Attachment: patch

Comment 2

[Andreas Gal :gal]

Attachment: patch

Update comment.

Comment 3

[Igor Bukanov]

The patch should also restrict the usage of JS_IsAboutToFinalized (the public API) to make sure that it is only called after the marking phase is completed. That would require to update the xpconnect code.

Comment 4

[Andreas Gal :gal]

#3 is a good idea. I can add a debug assert for that.

Comment 5

[Andreas Gal :gal]

Attachment: assert that IsAboutToBeFinalized API is only called after marking

Comment 6

[Andreas Gal :gal]

Attachment: patch

Comment 7

[Igor Bukanov]

Comment on attachment 428788 [details] [diff] [review]
patch

>     /*
>+     * Allow the embedding to do some additional marking.
>+     */
>+    if (rt->gcCallback)
>+        (void) rt->gcCallback(cx, JSGC_MARK);
>+
>+    /*
>      * Mark children of things that caused too deep recursion during the above
>      * tracing.
>      */
>     MarkDelayedChildren(&trc);
> 

This still is not right. MarkDelayedChildren should be called before JSGC_MARK so the cycle collector callback can see all the marked objects. 

A solution for that could be to drop JSGC_MARK, make MarkDelayedChildren a friend function and call it before JSGC_MARK_END and explicitly in the cycle collector.

Comment 8

[Andreas Gal :gal]

I don't think this patch is needed any more for my work, so I will let go of this.

I still think the API should be cleaned up.

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Moving to p3 because no activity for at least 1 year(s).
See https://github.com/mozilla/bug-handling/blob/master/policy/triage-bugzilla.md#how-do-you-triage for more information