310456 - Crash [@ js_MarkScript] when visiting Gmail, visiting another site and then going back

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Martijn Wargers (dead)]

This regressed between 2005-09-21 and 2005-09-22 and it is only a trunk crash:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-09-21+06%3A00%3A00&maxdate=2005-09-22+09%3A00%3A00&cvsroot=%2Fcvsroot

To reproduce:
- visit Gmail (you have to have an account)
- Visit another site
- Go back

From talkback ID TB9817090Q: 
js_MarkScript 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsscript.c, line 1344]
fun_mark  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsfun.c,
line 1357]
js_Mark  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsobj.c,
line 4119]
MarkGCThing 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsgc.c, line 1146]
js_MarkGCThing 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsgc.c, line 1446]
js_MarkAtomState 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsatom.c, line 467]
js_GC  [c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsgc.c,
line 1705]
js_NewGCThing 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsgc.c, line 636]
AllocSlots 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 1811]
js_NewObject 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsobj.c, line 1929]
js_NewFunction 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsfun.c, line 1964]
FunctionDef 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsparse.c, line 858]
FunctionStmt 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsparse.c, line 1021]
Statements 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsparse.c, line 1053]
Statement 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsparse.c, line 1704]
Statements 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsparse.c, line 1053]
js_CompileTokenStream 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsparse.c, line 468]
CompileTokenStream 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3432]
JS_CompileUCScriptForPrincipals 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3513]
JS_EvaluateUCScriptForPrincipals 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/js/src/jsapi.c, line 3948]
nsJSContext::EvaluateString 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp,
line 1067]
nsScriptLoader::EvaluateScript 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsScriptLoader.cpp,
line 748]
nsScriptLoader::ProcessRequest 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsScriptLoader.cpp,
line 642]
nsScriptLoader::ProcessScriptElement 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/base/src/nsScriptLoader.cpp,
line 583]
nsHTMLScriptElement::MaybeProcessScript 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLScriptElement.cpp,
line 673]
nsHTMLScriptElement::SetHtmlFor 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/content/src/nsHTMLScriptElement.cpp,
line 537]
SinkContext::CloseContainer 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 1403]
HTMLContentSink::CloseContainer 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/content/html/document/src/nsHTMLContentSink.cpp,
line 2985]
CNavDTD::HandleToken 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 838]
CNavDTD::BuildModel 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/CNavDTD.cpp,
line 459]
nsParser::BuildModel 
[c:/builds/tinderbox/Fx-Trunk/WINNT_5.2_Depend/mozilla/parser/htmlparser/src/nsParser.cpp,
line 2010]

I have bfcache enable (haven't checked yet without bfcache).

Comment 1

[Hermann Schwab]

Mozilla/5.0 (Windows; U; Win98; en-US; rv:1.9a1) Gecko/20050927 SeaMonkey/1.1a
TB9874524K

Comment 2

[Hermann Schwab]

*** Bug 310493 has been marked as a duplicate of this bug. ***

Comment 3

[Hermann Schwab]

there is still a 'nice to have' Patch waiting according to Bug 305181 comment 25.
Bug 305181 [FIX]Crash when navigating between Gmail and another URL [@
nsXPConnect::ReleaseJSContext]

Comment 4

[Boris Zbarsky [:bzbarsky]]

The remaining patch in bug 305181 isn't really a crash fix of any sort...

Given the regression range, this looks to be a regression from bug 308085...

Comment 5

[Boris Zbarsky [:bzbarsky]]

Then again, bug 308085 landed on branch too.  But it's the onle JS engine change
on trunk in that range...

Comment 6

[Ria Klaassen (not reading all bugmail)]

Weird, I can reproduce the crash in this build: 1.9a1_2005092912, but not in
this build: 1.9a1_2005092922.
Something cured it?

Comment 7

[Boris Zbarsky [:bzbarsky]]

That's the range in which mrbkap backed out one of the patches for bug 307317...

Comment 8

[Martijn Wargers (dead)]

Works for me too now, on current trunk. Marking WFM then?

Comment 9

[Boris Zbarsky [:bzbarsky]]

I'd wait and see what happens in bug 307317

Comment 10

[Blake Kaplan (:mrbkap) (inactive)]

This is a regression from bug 308085. Fix in a jiffy.

Comment 11

[Blake Kaplan (:mrbkap) (inactive)]

Attachment: Null-check the script

This is very similar to bug 309695... The fix in bug 307317 must be causing us
to mark the function, even after we've refused to create the script (or before
the script is created, I'm not sure).

Comment 12

[Blake Kaplan (:mrbkap) (inactive)]

Actually, the stack says: "Before the script is created", no invalid javascript
needed here.

Comment 13

[Mike Shaver (:shaver emeritus)]

Comment on attachment 198033 [details] [diff] [review]
Null-check the script

r=shaver

Comment 14

[Martijn Wargers (dead)]

Does the "Null-check the script" patch also take care of bug 310399?

Comment 15

[Blake Kaplan (:mrbkap) (inactive)]

That stack is pretty weird (looks like an optimizer got to it :-(), so once I
check this fix in, people should test to see if fixes the problem, but it
certainly is possible (since we are marking in that stack).

Comment 16

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into trunk.

Comment 17

[Blake Kaplan (:mrbkap) (inactive)]

Comment on attachment 198033 [details] [diff] [review]
Null-check the script

This is a very safe null-defense patch that fixes a regression from bug 308085
which was also checked in on the branch.

Comment 18

[Martijn Wargers (dead)]

(In reply to comment #15)
> That stack is pretty weird (looks like an optimizer got to it :-(), so once I
> check this fix in, people should test to see if fixes the problem, but it
> certainly is possible (since we are marking in that stack).
The problem is that it already doesn't crash anymore (just like this bug), so no
idea if the patch would fix that bug.

Comment 19

[Blake Kaplan (:mrbkap) (inactive)]

Fix checked into MOZILLA_1_8_BRANCH.

Comment 20

[Blake Kaplan (:mrbkap) (inactive)]

(In reply to comment #18)
> The problem is that it already doesn't crash anymore (just like this bug), so no
> idea if the patch would fix that bug.

Upon further reflection, it looks like this patch should fix that bug as well.

Comment 21

[Eiichi]

*** Bug 310399 has been marked as a duplicate of this bug. ***