Last Comment Bug 337716 - Crash [@ js_GetSlotThreadSafe nsGetInterface::operator JS_GetParent] with testcase, using iframe/timers/alert
: Crash [@ js_GetSlotThreadSafe nsGetInterface::operator JS_GetParent] with tes...
Status: VERIFIED FIXED
[sg:critical?] testcase reused for bu...
: crash, regression, testcase, verified1.8.0.10, verified1.8.1.2
Product: Core
Classification: Components
Component: DOM (show other bugs)
: Trunk
: All All
: -- critical (vote)
: ---
Assigned To: Johnny Stenback (:jst, jst@mozilla.com)
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Mentors:
Depends on: 52209 364657 367123
Blocks: 302889
  Show dependency treegraph
 
Reported: 2006-05-12 08:28 PDT by Martijn Wargers [:mwargers] (not working for Mozilla)
Modified: 2008-04-03 15:49 PDT (History)
6 users (show)
jaymoz: blocking1.8.1.2+
jaymoz: blocking1.8.0.10+
jwalden+bmo: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Fix per mrbkap's comment. (732 bytes, patch)
2007-01-04 16:06 PST, Johnny Stenback (:jst, jst@mozilla.com)
mrbkap: review+
jonas: superreview+
jaymoz: approval1.8.1.2+
jaymoz: approval1.8.0.10+
Details | Diff | Splinter Review
mac crash report (9.85 KB, text/plain)
2007-01-12 10:01 PST, alice nodelman [:alice] [:anode]
no flags Details

Description Martijn Wargers [:mwargers] (not working for Mozilla) 2006-05-12 08:28:28 PDT
See upcoming testcase, that I'll attach.
You need to let it run at least 5 seconds or so, to let it build up multiple alert dialogs.
After waiting, click them away.
At this point, I crash often (50% of the time).

I get backtraces with different signatures, so that's why I'm filing it as security sensitive.

Some talkback ID's width different signatures:
TB18597168W
0xb82c0278
js_GetSlotThreadSafe   JS_GetParent   nsDOMClassInfo::WrapNative   nsWindowSH::SetProperty   XPC_WN_Helper_SetProperty   js_SetProperty   JS_SetUCProperty   nsWindowSH::SetProperty   XPC_WN_Helper_SetProperty   js_SetProperty   js_Interpret   0x0188ba40

TB18597034E
0x024a6ff4
nsGetInterface::operator()   nsCOMPtr_base::assign_from_helper   nsCOMPtr<nsIScriptGlobalObject>::nsCOMPtr<nsIScriptGlobalObject>   nsLocationSH::PreCreate   XPCWrappedNative::GetNewOrUsed   XPCConvert::NativeInterface2JSObject   nsXPConnect::WrapNative   nsDOMClassInfo::WrapNative   nsWindowSH::SetProperty   XPC_WN_Helper_SetProperty   js_SetProperty   JS_SetUCProperty   nsWindowSH::SetProperty   XPC_WN_Helper_SetProperty   js_SetProperty   js_Interpret   0x0188c9e8

TB18588913W
JS_GetParent   nsDOMClassInfo::WrapNative   nsWindowSH::SetProperty   XPC_WN_Helper_SetProperty   js_SetProperty   JS_SetUCProperty   nsWindowSH::SetProperty   XPC_WN_Helper_SetProperty   js_SetProperty   js_Interpret   0x02080760

This seems to have regressed between 2005-08-01 and 2005-08-02:
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-08-01+06&maxdate=2005-08-02+08&cvsroot=%2Fcvsroot
I suspect bug 302889 might be the culprit.
Comment 1 Martijn Wargers [:mwargers] (not working for Mozilla) 2006-05-12 08:34:24 PDT
Created attachment 221816 [details]
testcase
Comment 2 Blake Kaplan (:mrbkap) 2006-05-12 16:28:52 PDT
I even asked jst who marks the inner window's JS object yesterday ;-).

The answer to my question is that the inner window's JS object is rooted through the outer window's mInnerWindowHolder. This is usually enough keep the inner window alive, but nsWindowSH::SetProperty first calls SetHref() (which gives the outer window a new inner window, unrooting the inner window's global JS object) and then tries to use the inner window's global JS object to wrap the location object. Martijn's alerts allow a GC to nest in between and we access the inner window's collected JS object and crash.

Unfortunately, when I swapped the two calls (so that I wrapped the location object before the call to SetHref) I hit a JS assertion.
Comment 3 Johnny Stenback (:jst, jst@mozilla.com) 2006-12-18 14:43:30 PST
Taking bug.
Comment 4 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-04 16:03:23 PST
Blake's attempt to swap the order of changing the location and wrapping the location object does fix this problem and is IMO the right fix here. And now that the assertion is fixed (bug 364657), swapping the two calls is safe. Patch coming up.
Comment 5 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-04 16:06:07 PST
Created attachment 250526 [details] [diff] [review]
Fix per mrbkap's comment.
Comment 6 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-05 14:29:38 PST
Fixed on the trunk.
Comment 7 Jay Patel [:jay] 2007-01-08 15:45:42 PST
Comment on attachment 250526 [details] [diff] [review]
Fix per mrbkap's comment.

Approved for both branches branches, a=jay for drivers.
Comment 8 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-10 17:39:40 PST
Fixed on the branches.
Comment 9 alice nodelman [:alice] [:anode] 2007-01-11 19:30:46 PST
Testing with 2.0.0.2pre and the provided testcase still caused a crash.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.1.2pre) Gecko/2007011103 BonEcho/2.0.0.2pre

Should the testcase no longer crash at all, or am I just looking for a 'secure' crash?

Comment 10 Johnny Stenback (:jst, jst@mozilla.com) 2007-01-11 22:26:34 PST
Hmm, the testcase should not crash at all. I wonder if there's other bugs that are causing a crash with the same testcase, maybe mac only? I've ran the testcase a *lot* with my fix and haven't seen a crash. Please re-test with another build and another platform too if you can and report back (and reopen the bug if it's still crashing for you). Oh, and talkback data would be useful too.
Comment 11 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-01-12 03:04:19 PST
This is fixed for me, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8.1.2pre) Gecko/20070111 BonEcho/2.0.0.2pre
and current trunk build.
I'm on windows, so perhaps a Mac specific crash?
Comment 12 alice nodelman [:alice] [:anode] 2007-01-12 10:01:19 PST
Created attachment 251291 [details]
mac crash report

Reproduced the crash on mac for 2.0.0.2pre.  It was triggered after alternating clicking on the 'okay' button on the alert and the 'back' arrow on the browser - it was the combination of these two actions that caused the crash.
Comment 13 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-01-12 10:14:48 PST
Ok, indeed, with the steps to reproduce in comment 12, I can too reproduce this crash on the latest branch build, talkback ID: TB28292905Y
Probably a new bug should be filed on that issue, so this bug can stay fixed then.
Comment 14 alice nodelman [:alice] [:anode] 2007-01-12 11:20:58 PST
Now I have another issue.  The testcase causes 1.5.0.10pre to hang.  After running the testcase, having the alert raised, waiting 5 seconds and then clicking the alert the browser becomes unresponsive.  Clicking on buttons has no effect - though I can command-Q to quit.

Mozilla/5.0 (Macintosh; U; Intel Mac OS X; en-US; rv:1.8.0.10pre) Gecko/20070111 Firefox/1.5.0.10pre
Comment 15 Martijn Wargers [:mwargers] (not working for Mozilla) 2007-01-16 07:03:12 PST
Ok, I have filed bug 367123 for the crash mentioned in commment 9 and further.
That crash is a different issue, because it has a different way of reproducing and has a different stacktrace.
The original bug is fixed on trunk and branches, this was verified by me and Alice.

Note You need to log in before you can comment on or make changes to this bug.