935903 - GC: SharedScriptData is not unmarked if a collection is reset

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jon Coppeard (:jonco)]

Attachment: unmark-shared-script-data

SharedScriptData has a 'marked' flag which is set indirectly from JSScript::markChildren() and cleared from js::SweepScriptData().  These objects are only marked in a full GC.  Unfortunately SweepScriptData() is not called if the collection is reset, or if what started as a full GC becomes a non-full GC.  So this could lead to SharedScriptData being retained longer than the scripts they reference.

I don't know if this is responsible for bug 931251, but it should be fixed.

The simplest solution is to unmark the SharedScriptData objects at the start of a full GC.

Comment 1

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

Comment on attachment 828565 [details] [diff] [review]
unmark-shared-script-data

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

Nice catch.

Comment 2

[Jon Coppeard (:jonco)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/264184ace1e1

Comment 3

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/264184ace1e1

Comment 4

[Till Schneidereit [:till]]

Comment on attachment 828565 [details] [diff] [review]
unmark-shared-script-data

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 679940
User impact if declined: unclear, this is a long-shot attempt to fix the unreproducible top-crash in bug 931251. See bug 931251, comment 10 for some details.
Testing completed (on m-c, etc.): yes
Risk to taking this patch (and alternatives if risky): extremely low. The patch fixes a bug and doesn't otherwise change any behavior. Applies cleanly and builds the js shell on beta and aurora.
String or IDL/UUID changes made by this patch: n/a

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/12b4725d98b1
https://hg.mozilla.org/releases/mozilla-beta/rev/7fed679f0661

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/7fed679f0661

Comment 7

[u279076]

I don't think this needs QA verification. If anyone thinks that's a mistake please remove the [qa-] whiteboard tag and add the verifyme keyword.