Created attachment 526627 [details] shell testcase, unpack, chdir and run "js -m -n -a -f shell.js -f main.js" Here's another GC related problem, maybe it's even related to the previous (yet unfixed) problem. The test case is again quite complicated, unpack, chdir and run "js -m -n -a -f shell.js -f main.js" to reproduce (tested on TI ac0989a03bf1, 32 bit).
The problem here is that after calling NewScriptFromCG during compileScript, we don't root it and call defineGlobals, which can make new shapes and trigger a GC. That GC can't (I think) collect the new shape since no script object has been made for it yet, but the script isn't traced and the GC can collect the bindings which have been removed from the cg and added to the new script. This problem also seems to be on TM. The only reason it doesn't show up there seems to be that the JM branch allocates shapes slightly differently (different empty shapes for each script) and there is a small window where the GC can happen for the bug to occur. Even with gczeal(2), GCs only occur when arenas get filled up and not on every allocation. Would it be possible to have a gczeal(3) or something that GCs on every allocation? (is that how gczeal used to work?)
If gczeal(2) does not execute a GC at every possible point of time where it could also happen in normal execution mode, then I second the need for gczeal(3). It would probably make it much easier to find certain GC bugs.
Created attachment 533881 [details] [diff] [review] patch Patch against JM (similar in TM except the ensureShape call is not there). I think this is causing orange on the JM tinderbox now, and should go in before the fixes to gczeal (bug 650978). How should this land? I think this hazard has been in since November.
Assignee: general → bhackett1024
Attachment #533881 - Flags: review?(dmandelin)
Comment on attachment 533881 [details] [diff] [review] patch It would be nice to replace "goto bad" with RAII but that's optional. I think billm is the one you want to coordinate landing with.
Attachment #533881 - Flags: review?(dmandelin) → review+
With bug 650978 fixed this should have started causing problems in TM (were it a TM hazard) but didn't. Bill looked into this more closely (thanks!) and it turns out the bindings at risk are always empty (always global scripts), so the shape is the compartment's empty call shape which the conservative GC seems to always be able to find a reference to (luckily, as we don't take care to keep it rooted). This affects TI because it uses a different empty shape for each script, which does not get accidentally rooted. Pushed the fix to TM, but I don't think this needs to go into aurora or beta. http://hg.mozilla.org/tracemonkey/rev/14f91384ce26
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Whiteboard: [sg:high?] → [sg:high?] [fixed-in-tracemonkey]
This assertion was seen once in crash automation on Windows XP Beta on http://tintuconline.com.vn/vn/index.html I haven't tried to reproduce locally.
Looks fixed on m-c. http://hg.mozilla.org/mozilla-central/rev/14f91384ce26 Re: comment 7, this is a generic assertion indicating heap corruption detected by the GC, so could be caused by any number of factors.
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago → 8 years ago
Resolution: --- → FIXED
status1.9.2: --- → unaffected
status-firefox7: --- → fixed
status-firefox8: --- → fixed
status-firefox9: --- → fixed
Target Milestone: --- → mozilla7
Would QA be able to verify this using one of the debug builds from FTP? If so, some instruction would be appreciated. Thanks.
Whiteboard: [sg:high?] [fixed-in-tracemonkey] → [sg:high?] [fixed-in-tracemonkey][qa?]
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #9) > Would QA be able to verify this using one of the debug builds from FTP? If > so, some instruction would be appreciated. Thanks. If the location you are downloading from offers a jsshell-*.zip file (like the Tinderbox debug builds do), then you can use the js shell in there (the js binary) to run the test as described in comment 0. If you don't have the js shell but want to build it, then you can do so using the branch you want to test on. If you want me to help you with that and/or give you build instructions/an easy build script, let me know! :)
(In reply to Christian Holler (:decoder) from comment #10) > If the location you are downloading from offers a jsshell-*.zip file (like > the Tinderbox debug builds do), then you can use the js shell in there. This would be simplest I think. Could you point me to some test builds to use for verification?
(In reply to Anthony Hughes, Mozilla QA (irc: ashughes) from comment #11) > This would be simplest I think. Could you point me to some test builds to > use for verification? Sure. For example for x86-64 aurora, you can use the jsshell-*.zip in https://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-aurora-linux64-debug/1323342110/ Let me know if you need anything else :)
Thanks Christian. Marking [qa+] for verification with the build (and similar) as per comment 12.
Whiteboard: [sg:high?] [fixed-in-tracemonkey][qa?] → [sg:high?] [fixed-in-tracemonkey][qa+]
You need to log in before you can comment on or make changes to this bug.