Crash when browse this Site over top menu [@ nsGlobalWindow::RunTimeout]

VERIFIED FIXED

Status

()

Core
DOM: Core & HTML
--
critical
VERIFIED FIXED
12 years ago
11 years ago

People

(Reporter: 1106-29, Assigned: smaug)

Tracking

(4 keywords)

Trunk
crash, fixed1.8.1, regression, verified1.8.0.5
Points:
---
Bug Flags:
blocking1.8.0.5 +

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(5 attachments, 4 obsolete attachments)

(Reporter)

Description

12 years ago
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8) Gecko/20051111 Firefox/1.5

Browser Crash when you browse the site over the top menu
or when you go to other site in this tab.
i test on 1.5 see Build Identifier
and last 1.6a2 from 20.12.05 with clear profile


Reproducible: Always

Steps to Reproduce:
1.www.scriptinternals.de
2.over top menu goto download SystemScripter 6.0
3.wait or to next page or new site in same tap
(Reporter)

Updated

12 years ago
Version: unspecified → 1.5 Branch
(Reporter)

Updated

12 years ago
Confirmed with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a1) Gecko/20051220 Firefox/1.6a1 ID:2005122005

TB13161792E (trunk)
TB13161826W (branch)
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: talkbackid
Keywords: crash
1.9a1_2005081307 (no crash) - 1.9a1_2005081323 (crash)

http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=PhoenixTinderbox&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-08-13+06%3A00%3A00&maxdate=2005-08-13+23%3A00%3A00&cvsroot=%2Fcvsroot

Updated

12 years ago
Version: 1.5 Branch → Trunk
Without flash no crash.
Likely a regression from bug 297561.
Assignee: nobody → win32
Component: General → Widget: Win32
Product: Firefox → Core
QA Contact: general → ian
Keywords: regression
I don't understand step 3 of the steps to reproduce, could somebody clarify?
Flags: blocking1.8.1?
Version: Trunk → 1.8 Branch
On top of http://www.scriptinternals.de/new/ger/home/default.htm there's a dropdown menu (Download). In that menu go to System Scripter 0.6. The next page crashes.
Ah, ok, seems like you need to move the mouse before the new page loads. I got it to crash about half of the time when using these steps:

1) Go to http://www.scriptinternals.de/new/ger/home/default.htm
2) Click "SystemScripter 6.0" in the "Download" drop down menu at the top
3) Before the new page loads, start moving the mouse across the flash banner
Here it crashes immediately after leaving the flash animation on top of the second page. If I stay with my mouse inside the flash animation it doesn't crash. 
Flags: blocking1.8.1?

Comment 9

12 years ago
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9a1) Gecko/20051220 Firefox/1.6a1

#6  0xb5332a04 in nsGlobalWindow::RunTimeout (this=0x8960628, aTimeout=0x8ce2fc8) at /moz/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp:6306
#7  0xb5332c90 in nsGlobalWindow::TimerCallback (aTimer=0x8ce7b60, aClosure=0x8ce2fc8) at /moz/trunk/mozilla/dom/src/base/nsGlobalWindow.cpp:6602
#8  0xb7d8ca3b in nsTimerImpl::Fire (this=0x8ce7b60) at /moz/trunk/mozilla/xpcom/threads/nsTimerImpl.cpp:400
#9  0xb7d8cccd in handleTimerEvent (aEvent=0xb1700588) at /moz/trunk/mozilla/xpcom/threads/nsTimerImpl.cpp:467
#10 0xb7d847ae in PL_HandleEvent (self=0xb1700588) at /moz/trunk/mozilla/xpcom/threads/plevent.c:688
#11 0xb7d8463a in PL_ProcessPendingEvents (self=0x80e1d90) at /moz/trunk/mozilla/xpcom/threads/plevent.c:623
#12 0xb7d876bd in nsEventQueueImpl::ProcessPendingEvents (this=0x80e1d48) at /moz/trunk/mozilla/xpcom/threads/nsEventQueue.cpp:417
#13 0xb5d00230 in event_processor_callback (source=0x82ee438, condition=G_IO_IN, data=0x80e1d48) at /moz/trunk/mozilla/widget/src/gtk2/nsAppShell.cpp:67
#14 0xb770531c in g_vasprintf () from /usr/lib/libglib-2.0.so.0
#15 0xb76de4ee in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#16 0xb76e14f6 in g_main_context_check () from /usr/lib/libglib-2.0.so.0
#17 0xb76e19d8 in g_main_context_iteration () from /usr/lib/libglib-2.0.so.0
#18 0xb7ae26a8 in gtk_main_iteration () from /usr/lib/libgtk-x11-2.0.so.0
#19 0xb1ae1a72 in gtkTimerCallback () from /home/aguthrie/.mozilla/plugins/libflashplayer.so
#20 0xb76e0006 in g_main_context_wakeup () from /usr/lib/libglib-2.0.so.0
#21 0xb76de4ee in g_main_context_dispatch () from /usr/lib/libglib-2.0.so.0
#22 0xb76e14f6 in g_main_context_check () from /usr/lib/libglib-2.0.so.0
#23 0xb76e17e3 in g_main_loop_run () from /usr/lib/libglib-2.0.so.0
#24 0xb7ae2e65 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#25 0xb5d00c26 in nsAppShell::Run (this=0x816f400) at /moz/trunk/mozilla/widget/src/gtk2/nsAppShell.cpp:139
#26 0xb708086f in nsAppStartup::Run (this=0x816f3b8) at /moz/trunk/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:161
#27 0xb7ed80ed in XRE_main (argc=2, argv=0xbfd2aa04, aAppData=0x8049820) at /moz/trunk/mozilla/toolkit/xre/nsAppRunner.cpp:2298
#28 0x08048676 in main (argc=2, argv=0xbfd2aa04) at /moz/trunk/mozilla/browser/app/nsBrowserApp.cpp:61
Assignee: win32 → general
Component: Widget: Win32 → DOM: Level 0
Keywords: talkbackid
OS: Windows XP → All
Hardware: PC → All
Summary: Crash when browse this Site over top menu → Crash when browse this Site over top menu [@ nsGlobalWindow::RunTimeout]

Comment 10

12 years ago

*** This bug has been marked as a duplicate of 315286 ***
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → DUPLICATE
Yeah, I still crash with current trunk build on this one.
Talkback ID: TB17867994E
nsGlobalWindow::ClearTimeoutOrInterval   nsGlobalWindow::ClearTimeoutOrInterval   nsGlobalWindow::ClearInterval   XPCWrappedNative::CallMethod  
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---

Updated

12 years ago
Version: 1.8 Branch → Trunk

Updated

12 years ago
Flags: blocking1.9a1?
(Assignee)

Comment 12

11 years ago
Created attachment 223727 [details] [diff] [review]
a patch for 1.8

This is a possible patch (for 1.8 for now since the dhtml menu on the page 
doesn't work too well in 1.9 - flash is painted above it).
The change to t->Release(nsnull) in nsGlobalWindow::SuspendTimeouts should fix 
nsGlobalWindow::ClearAllTimeouts crashes, at least those ones I saw during debugging this.
mTimeoutsSuspended makes sure that timeouts aren't used in child docshells of
a frozen window.
Asking review to get comments whether this is good enough, or should IsFrozen/Freeze() work in a bit different way, somehow recursively.
Attachment #223727 - Flags: review?(mrbkap)
(Assignee)

Comment 13

11 years ago
I can see this crash also in 1.8.0, and even without Flash.
Just before the crash there is
###!!! ASSERTION: How'd our timer end up null if we're not frozen?: 'IsFrozen()', file /home/smaug/mozilla/mozilla_cvs/1.8.0_branch/mozilla/dom/src/base/nsGlobalWindow.cpp, line 6465

Patch fixes the crash, but as I asked, would it be better to make Freeze() to
work in a bit different way?
Moving the Release in SuspendTimeouts is a good fix and I'm sorry that I was so wrong in the first place.

I think that the right fix for the bug is to call Freeze() before suspending the timeouts, though. This avoids the extra member. I've looked, and there isn't anything that should be depending on the inner window's mFrozen member.

I'll attach a patch once my build finishes and I can test it.
Created attachment 224621 [details] [diff] [review]
Alternative patch
Assignee: general → mrbkap
Attachment #223727 - Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #224621 - Flags: superreview?(bryner)
Attachment #224621 - Flags: review?(bugmail)
Attachment #223727 - Flags: review?(mrbkap)
Comment on attachment 224621 [details] [diff] [review]
Alternative patch

Do you think you could expand the comment next to Freeze a bit, so that if I'm reading this code I'll have an idea what sort of things _could_ happen to the inner window, and why we don't want them to?

Looks good otherwise.
Attachment #224621 - Flags: superreview?(bryner) → superreview+
(In reply to comment #12)
> mTimeoutsSuspended makes sure that timeouts aren't used in child docshells of
> a frozen window.

My patch addresses this, and I believe that it is possible, but could you attach a stack or explain how a timeout managed to fire between the entrance to nsGlobalWindow::SaveWindowState and the call to nsGlobalWindow::SuspendTimeouts?
(Assignee)

Comment 18

11 years ago
(In reply to comment #17)
> My patch addresses this, and I believe that it is possible, but could you
> attach a stack or explain how a timeout managed to fire between the entrance to
> nsGlobalWindow::SaveWindowState and the call to
> nsGlobalWindow::SuspendTimeouts?
> 

It is not at that time, I think.
attachment 224621 [details] [diff] [review] does *not* fix the crash, at least not in 1.8
(Can't test on trunk since it is not working at all in 64bit Linux, Bug 340452)

This is one stack I got (with attachment 224621 [details] [diff] [review])
#0  0x00000038c520cf6d in raise () from /lib64/libpthread.so.0
#1  0x000000000041892b in nsProfileLock::FatalSignalHandler (signo=11) at nsProfileLock.cpp:206
#2  <signal handler called>
#3  0x00002aaab137062c in nsGlobalWindow::RunTimeout (this=0x1f9b960, aTimeout=0x124c680)
    at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/dom/src/base/nsGlobalWindow.cpp:6460
#4  0x00002aaab1370b9f in nsGlobalWindow::TimerCallback (aTimer=Variable "aTimer" is not available.
)
    at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/dom/src/base/nsGlobalWindow.cpp:6914
#5  0x00002aaaaae15f1c in nsTimerImpl::Fire (this=0x61f350) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/xpcom/threads/nsTimerImpl.cpp:394
#6  0x00002aaaaae16138 in handleTimerEvent (event=0x2aaab3a01e80) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/xpcom/threads/nsTimerImpl.cpp:459
#7  0x00002aaaaae0fe49 in PL_HandleEvent (self=0x2aaab3a01e80) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/xpcom/threads/plevent.c:688
#8  0x00002aaaaae101af in PL_ProcessPendingEvents (self=0x5bdfa0) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/xpcom/threads/plevent.c:623
#9  0x00002aaaaae127af in nsEventQueueImpl::ProcessPendingEvents (this=0x5bdda0)
    at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/xpcom/threads/nsEventQueue.cpp:417
#10 0x00002aaaaf8b3812 in event_processor_callback (source=0x7fffff895460, condition=0, data=0x4159dbf04813a)
    at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/widget/src/gtk2/nsAppShell.cpp:67
#11 0x00000038c6a2703a in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
#12 0x00000038c6a2a1b5 in g_main_context_check () from /usr/lib64/libglib-2.0.so.0
#13 0x00000038c6a2a4dd in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
#14 0x00000038c8a1ed03 in gtk_main () from /usr/lib64/libgtk-x11-2.0.so.0
#15 0x00002aaaaf8b3dd2 in nsAppShell::Run (this=0x69b860) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/widget/src/gtk2/nsAppShell.cpp:139
#16 0x00002aaaafc37caa in nsAppStartup::Run (this=0x69b810)
    at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/toolkit/components/startup/src/nsAppStartup.cpp:150
#17 0x0000000000409575 in XRE_main (argc=Variable "argc" is not available.
) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/toolkit/xre/nsAppRunner.cpp:2376
#18 0x00000000004041a8 in main (argc=Variable "argc" is not available.
) at /home/smaug/mozilla/mozilla_cvs/18_branch/mozilla/browser/app/nsBrowserApp.cpp:61
(Assignee)

Comment 19

11 years ago
Created attachment 224680 [details]
a test case

This doesn't crash, but shows that timers aren't always suspended in frames.
(Assignee)

Comment 20

11 years ago
(In reply to comment #19)
> Created an attachment (id=224680) [edit]
> a test case
> 
> This doesn't crash, but shows that timers aren't always suspended in frames.
> 
You may have to shift-reload before seeing the problem.
And I got trunk working, and there I see a crash, but apparently it is
not the same timer related bug.

(Assignee)

Comment 21

11 years ago
Trunk crash is a regression from Bug 336674
Comment on attachment 223727 [details] [diff] [review]
a patch for 1.8

Sigh, this is easiest, I suppose.
Attachment #223727 - Attachment is obsolete: false
Attachment #223727 - Flags: review+

Updated

11 years ago
Attachment #224621 - Attachment is obsolete: true
Attachment #224621 - Flags: review?(bugmail)
(Assignee)

Comment 23

11 years ago
Comment on attachment 223727 [details] [diff] [review]
a patch for 1.8

And what would you think, Brian?
Attachment #223727 - Flags: superreview?(bryner)
Comment on attachment 223727 [details] [diff] [review]
a patch for 1.8

>--- dom/src/base/nsGlobalWindow.cpp	19 May 2006 18:23:11 -0000	1.761.2.37
>+++ dom/src/base/nsGlobalWindow.cpp	29 May 2006 21:13:51 -0000
>@@ -6321,17 +6322,17 @@ nsGlobalWindow::SetTimeoutOrInterval(PRB
> 
>     rv = sSecMan->CheckSameOriginPrincipal(subjectPrincipal, ourPrincipal);
>     timeout->mPrincipal = NS_SUCCEEDED(rv) ? subjectPrincipal : ourPrincipal;
>     rv = NS_OK;
>   }
> 
>   PRTime delta = (PRTime)interval * PR_USEC_PER_MSEC;
> 
>-  if (!IsFrozen()) {
>+  if (!mTimeoutsSuspended) {
>     // If we're not currently frozen, then we set timeout->mWhen to be the
>     // actual firing time of the timer (i.e., now + delta). We also actually
>     // create a timer and fire it off.

The comment here no longer matches the |if| check.

Just to make sure I understand (I probably knew the answer at one time), we suspend timeouts on child windows, but we don't call Freeze() on them, and that's why this is necessary?
(Assignee)

Comment 25

11 years ago
> The comment here no longer matches the |if| check.

The comments should be then "If timeouts are not suspended, then..."
> 
> Just to make sure I understand (I probably knew the answer at one time), we
> suspend timeouts on child windows, but we don't call Freeze() on them, and
> that's why this is necessary?

Yes. The question I had/have is that should we freeze also child windows?
Although, with this patch maybe that is not needed. 

(In reply to comment #25)
> Yes. The question I had/have is that should we freeze also child windows?
> Although, with this patch maybe that is not needed. 

I've been considering this too. We could call Freeze on the sub-windows in SuspendTimeouts, and Thaw them in ResumeTimeouts.
Created attachment 224757 [details] [diff] [review]
Something like this

Smaug does this patch help?
Attachment #224757 - Flags: review?

Updated

11 years ago
Attachment #224757 - Flags: review? → review?(Olli.Pettay)
(Assignee)

Comment 28

11 years ago
Created attachment 224836 [details]
test case for online/offline events in iframes
(Assignee)

Comment 29

11 years ago
Created attachment 224838 [details] [diff] [review]
Freeze/thaw on inner window and fixes to delayed dom events

This is based on attachment 224757 [details] [diff] [review], but calling Freeze/Thaw on inner window and
also fixing problems with offline/online and storage events when windows
are restored from bfcache.
Attachment #223727 - Attachment is obsolete: true
Attachment #224757 - Attachment is obsolete: true
Attachment #224838 - Flags: superreview?(bryner)
Attachment #224838 - Flags: review?(mrbkap)
Attachment #224757 - Flags: review?(Olli.Pettay)
Attachment #223727 - Flags: superreview?(bryner)
Comment on attachment 224838 [details] [diff] [review]
Freeze/thaw on inner window and fixes to delayed dom events

Oops. I obviously need to test my patches more! This looks great.
Attachment #224838 - Flags: review?(mrbkap) → review+
(Assignee)

Updated

11 years ago
Flags: blocking1.8.1?
(Assignee)

Updated

11 years ago
Attachment #224838 - Flags: approval-branch-1.8.1?(bryner)
Attachment #224838 - Flags: superreview?(bryner)
Attachment #224838 - Flags: superreview+
Attachment #224838 - Flags: approval-branch-1.8.1?(bryner)
Attachment #224838 - Flags: approval-branch-1.8.1+
(Assignee)

Comment 31

11 years ago
Comment on attachment 224838 [details] [diff] [review]
Freeze/thaw on inner window and fixes to delayed dom events

oops, this doesn't apply to 1.8.1, but checked in to trunk. Will do the 1.8.1 version of the patch.
Attachment #224838 - Flags: approval-branch-1.8.1+
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 12 years ago11 years ago
Resolution: --- → FIXED
(Assignee)

Comment 32

11 years ago
Created attachment 225802 [details] [diff] [review]
for 1.8.1

Otherwise the same patch, but no need to handle online/offline events (which are trunk only) and FireDelayedDOMEvents doesn't exists in branch, so I had to add it 
to nsPIDOMWindow_MOZILLA_1_8_BRANCH
Assignee: mrbkap → Olli.Pettay
Status: RESOLVED → ASSIGNED
Attachment #225802 - Flags: superreview?(bryner)
Attachment #225802 - Flags: review?(bryner)
Attachment #225802 - Flags: approval-branch-1.8.1?(bryner)
Resolution: FIXED → ---
(Assignee)

Comment 33

11 years ago
Comment on attachment 225802 [details] [diff] [review]
for 1.8.1

argh, too late here. wrong one
Attachment #225802 - Attachment is obsolete: true
Attachment #225802 - Flags: superreview?(bryner)
Attachment #225802 - Flags: review?(bryner)
Attachment #225802 - Flags: approval-branch-1.8.1?(bryner)
(Assignee)

Comment 34

11 years ago
Created attachment 225806 [details] [diff] [review]
this one is for 1.8.1
Attachment #225806 - Flags: superreview?(bryner)
Attachment #225806 - Flags: review?(bryner)
Attachment #225806 - Flags: approval-branch-1.8.1?(bryner)
Comment on attachment 225806 [details] [diff] [review]
this one is for 1.8.1

>--- dom/src/base/nsGlobalWindow.cpp	15 Jun 2006 03:15:47 -0000	1.761.2.41
>+++ dom/src/base/nsGlobalWindow.cpp	16 Jun 2006 00:39:47 -0000
>@@ -7155,16 +7208,23 @@ nsGlobalWindow::SaveWindowState(nsISuppo
>   if (!mContext || !mJSObject) {
>     // The window may be getting torn down; don't bother saving state.
>     return NS_OK;
>   }
> 
>   nsGlobalWindow *inner = GetCurrentInnerWindowInternal();
>   NS_ASSERTION(inner, "No inner window to save");
> 
>+  // Don't do anything else to this inner window! After this point, all
>+  // calls to SetTimeoutOrInterval will create entries in the timeout
>+  // list that will only run after this window has come out of the bfcache.
>+  // Also, while we're frozen, we won't dispatch online/offline events
>+  // to the page.

Get rid of the reference to online/offline events since those aren't on the branch anyway

Looks ok otherwise
Attachment #225806 - Flags: superreview?(bryner)
Attachment #225806 - Flags: superreview+
Attachment #225806 - Flags: review?(bryner)
Attachment #225806 - Flags: review+
Attachment #225806 - Flags: approval-branch-1.8.1?(bryner)
Attachment #225806 - Flags: approval-branch-1.8.1+
Could this have caused a Tp regression on btek?
(Assignee)

Comment 37

11 years ago
I backed out the patch, but couldn't really see any difference in tp, 
though the noise is quite bad.
Keywords: fixed1.8.1
(Assignee)

Comment 38

11 years ago
So now the patches are in trunk and branch
(Assignee)

Updated

11 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago11 years ago
Flags: blocking1.9a1?
Flags: blocking1.8.1?
Resolution: --- → FIXED
Flags: blocking1.8.0.5+
Comment on attachment 225806 [details] [diff] [review]
this one is for 1.8.1

approved for 1.8.0 branch, a=dveditz
Attachment #225806 - Flags: approval1.8.0.5+
Blocks: 342507
*** Bug 342507 has been marked as a duplicate of this bug. ***
Olli: can you land this on the 1.8.0 branch as well? Thanks.
(Assignee)

Comment 42

11 years ago
Created attachment 226916 [details] [diff] [review]
patch for 1.8.0 (without storage event fixes)
(Assignee)

Updated

11 years ago
Keywords: fixed1.8.0.5

Comment 43

11 years ago
Verified FIXED using Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.8.0.5) Gecko/20060626 Firefox/1.5.0.5.

No crash anymore when using Gavin's steps to reproduce in comment 7.
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.0.5 → verified1.8.0.5
Crash Signature: [@ nsGlobalWindow::RunTimeout]
You need to log in before you can comment on or make changes to this bug.