Closed Bug 448104 Opened 16 years ago Closed 16 years ago

Firefox crashing on shutdown - memory could not be read [@ nsAppShell:EventWindow]

Categories

(Core :: Widget: Win32, defect, P2)

Product:
Core
Component:
Widget: Win32
Platform:
x86
Windows XP
Type:
defect

Tracking

()

Status:
VERIFIED FIXED

People

(Reporter: wgianopoulos, Assigned: robarnold)

References

Details

(4 keywords)

Crash Data

Attachments

(3 files, 1 obsolete file)

Bookmarks file that causes the issue
9.45 KB, text/html
Details
windbg crash stack at shutdown
2.73 KB, text/plain
Details
v1.1
745 bytes, patch
Details | Diff | Splinter Review 
Several seconds after I close Firefox, it crashes with a nsAppSell:EventWindow saying:

The instruction at "0x5ad17531" referenced memory at "0x00000014". The memory could not be "read".

This happens even in safe-mode.  It does not fail with a new profile, but does fail with that new profile after I import my bookmarks.

This does NOT occur with the 2008071703 nightly build.  It fails with 2008071803.

I will try to isolate exactly what in my bookmarks file is triggering this and attach more information.

I also have not raised the priority to critical as I have not seen any mention of any other nightly testers running into this.
Under Windows/XP, start Firefox with a fresh profile.
Under Organize Bookmarks, import this attachment.
Firefox will now produce this error whenever you close the Application.

This issue is not reproducible under Linux.

Note: you will need to use a Firefox build from before the checkin for bug 99457 because of bug 447884.
Raising severity now that a testcase is available.
Severity: major → critical
Flags: blocking-firefox3.1?
Keywords: testcase
I should add that the bookmark in question uses micro-summaries.
I assume it's this bug.
Possibly from bug 444679 or bug 444013 since it looks theme related.
Backing out bug 444679 did not help.   Bug 444013 does not back out cleanly, but looks like the culprit form the crash stack.
Blocks: 444013
Component: Places → Widget: Win32
Flags: blocking-firefox3.1?
Product: Firefox → Core
QA Contact: places → win32
Flags: blocking1.9.1?
Attachment #331417 - Attachment description: mookmarks file that causes the issue → Bookmarks file that causes the issue
I'm not sure how bug 444013 causes this crash; it looks like a uxtheme bug.
I'm also experiencing the same error message. On shutdown error message appears. No other signs, only add-ons are Stumble, and Chatzilla

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/2008072703 Minefield/3.1a2pre
Summary: Firefox crashing on shutdown - memory could not be read → Firefox crashing on shutdown - memory could not be [@ nsAppShell:EventWindow]
(In reply to comment #7)
> I'm not sure how bug 444013 causes this crash; it looks like a uxtheme bug.
> 

I have 2 machines which I used the uxtheme patch on.  It works fine on one and not the other.
An FYI my crash (Dup Bug 448648) happened with a clean profile in shiretoko
I have verified bug 444013 as being the cause of this regression.

I did this by doing an hg cone specifying changeset e05e892261a8 (the changed pushed immediately preceding this one in the tree).  That build did NOT exhibit the issue.  I then added changeset 68ddc8f23721 (the checking for this bug) and that build DOES exhibit the issue.
Summary: Firefox crashing on shutdown - memory could not be [@ nsAppShell:EventWindow] → Firefox crashing on shutdown - memory could not be rear [@ nsAppShell:EventWindow]
Summary: Firefox crashing on shutdown - memory could not be rear [@ nsAppShell:EventWindow] → Firefox crashing on shutdown - memory could not be read [@ nsAppShell:EventWindow]
This bit of the stack trace didn't make sense:
> 0012fd34 101b661b xul!nsUXThemeData::Teardown(void)+0x5 [e:\builds\moz2_slave\mozilla-central-win32-nightly\build\widget\src\windows\nsuxthemedata.cpp @ 78]
> 0012fd38 10247a8e xul!DllMain(struct HINSTANCE__ * hModule = 0x10247b6f, unsigned long reason = 0x10000000, void * lpReserved = 0x0012fd9c)+0x22 [e:\builds\moz2_slave\mozilla-central-win32-nightly\build\toolkit\library\nsdllmain.cpp @ 64]
until I realized that the compiler was inlining nsToolkit::Shutdown.
I cannot reproduce this crash on Vista with the bookmarks file. Can anyone find a testcase that does?
Keywords: qawanted
Can you try to reproduce on a XP box?  I haven't seen anyone confirm that they are seeing this on vista.
I can try it on my XP machine tomorrow. But it's surprising that this issue exists only on XP (and that it's triggered by bookmarks).
I'll ask in the daily build thread on mozillazine.org to see if anyone with Vista experiences this crash.
(In reply to comment #17)
> I'll ask in the daily build thread on mozillazine.org to see if anyone with
> Vista experiences this crash.

"Can't reproduce or experienced this on 32 bit Vista. Maybe it's to do with a patched uxtheme.dll."

"Same here, no crashes on close using Vista HP SP1."

"Im on Vista Business SP1 and not had this error on shutdown, used to have it often when browsing Facebook and not had the error in the last week or so on nightly builds."

The last one sounds like a different crash though.  And just to note, I do not have a patched uxtheme.dll.  I haven't made any modifications to the XP theme and used the default Luna theme.

I have done more testing.

My original report was using the plain old vanilla uxtheme.dll which came with Windows XP SP2 (6.0.2900.2180)

I subsequently tried adding the hotfix for KB908536 which updated uxtheme.dll to version 6.0.2900.2845.  This is the newest version of uxtheme.dll that I could find on the Microsoft support site that they support under Windows XP SP2.  This update did not help at all with this issue.

However, doing more experimentation, I have found that this issue only seems to occur if I am using the Windows XP default (LUNA) theme.  Switching to classic theme avoids the issue altogether.

So in order to reproduce and debug this you must be running Windows XP and using the Luna theme.

I just tested with Windows XP SP3 which comes with uxtheme.dll version 6.0.2900.5512 and the issue is still present there.

So, even it this is uxthene.dll  bug, it would seem unwise to release a product that does not work under Windows XP with the default theme, so some workaround is obviously required.
Bill, do yours tabs open back up when you open Firefox after this crash?  I can't remember if mine do or not but I think they don't.  If not, we should add dataloss to the keywords. 
(In reply to comment #21)
> Bill, do yours tabs open back up when you open Firefox after this crash?  I
> can't remember if mine do or not but I think they don't.  If not, we should add
> dataloss to the keywords. 
> 
I just tried and my tabs restored just fine.

What does not work well is software update.  You need to tell it not to restart, exit the browser, wait for the error dialog and dismiss it.  Then start Firefox again for the software update to complete reliably.  The issue with letting it do the restart, is that unless you are quick enough dismissing the dialog box, the update fails saying there is another copy of Firefox running.
Let me clarify on the tabs restoring.

It will correctly restore tabs in the case where you have preferences set to use show windows and tabs from last time.

If the browser crashes for something other than this bug the tabs are correctly restored.

The tabs will not be restored to the last browser state because of this crash unless you have the preference set to show windows and tabs from last time, but that is OK and is not dataloss because in that case you were trying to do a normal browser shutdown so the tabs should not be restored just because somewhere in the course of the normal shutdown a crash occurs.
Rob, can we just not call CloseThemeData, and just leak the theme handles?  Would only be an issue if someone was switching themes like crazy, I would think.
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
(In reply to comment #24)
> Rob, can we just not call CloseThemeData, and just leak the theme handles? 
> Would only be an issue if someone was switching themes like crazy, I would
> think.
> 

I verified that removing the call to closeTheme in nsUXThemeData::Invalidate avoids the issue.  Sounds like you had already figured that out though.
Could the issue here be that nsUXThemeData::Teardown() is being called on browser shutdown before the microsummary and livebookmarks update services are being stopped?
(In reply to comment #25)
> (In reply to comment #24)
> > Rob, can we just not call CloseThemeData, and just leak the theme handles? 
> > Would only be an issue if someone was switching themes like crazy, I would
> > think.
> > 
> 
> I verified that removing the call to closeTheme in nsUXThemeData::Invalidate
> avoids the issue.  Sounds like you had already figured that out though.
> 

I just verified that removing the call to nsUXThemeData::Teardown from nsToolkit::Shutdown also avoids this crash.

This would seem to be a much better workaround, as it would only leak on shutdown and not on theme change.

Ok, that sounds like the best solution for now. I don't think we tore down on shutdown before the nsUXThemeData change so we're no worse off.
Status: NEW → ASSIGNED
Assignee: nobody → tellrob
Status: ASSIGNED → NEW
Attached patch proposed fix (obsolete) — — Splinter Review 

        Attachment #333362 -
        Flags: review?(vladimir)

    
    

    
  
Comment on attachment 333362 [details] [diff] [review]
proposed fix

In spirit, yes, but can you comment it out instead and put a comment explaining why we're not calling Teardown, referencing this bug?

        Attachment #333362 -
        Flags: review?(vladimir) → review+

    
    

    
  

      
      
      
      

        Attached patch
          
          
        v1.1
        — 
          — Splinter Review
      

    


  
Ok, commented out and put in reference to this bug

        Attachment #333362 -
        Attachment is obsolete: true

    
    

    
  
Pushed to mozilla-central:
http://hg.mozilla.org/mozilla-central/index.cgi/rev/47fd555eff5c
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED

    
  
Keywords: qawanted

    
    

    
  
Bill can probably verify as well, but I don't see a crash using Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080815064214 Minefield/3.1a2pre and using the STR in Comment 0.

    
    

    
  
Verified on vista using same build as Marcia.

    
    

    
  
Verified as fixing the original problem I reported under Windows/XP with Luna theme with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080815064214 Firefox/3.0.1.
Status: RESOLVED → VERIFIED

    
    

    
  
(In reply to comment #35)
> Verified as fixing the original problem I reported under Windows/XP with Luna
> theme with Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre)
> Gecko/20080815064214 Firefox/3.0.1.
> 

Ooops.  Should have read:

Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1a2pre) Gecko/20080815064214 Minefield/3.1a2pre

    
    

    
  
Has this been checked into the firefox trunk yet?  It has been marked fixed for several days, but on Firefox Build forum, as of August 17, this is still llisted as a Bug and has not yet been moved to the Fixed section.  Is this because it is still awaiting check ins?

    
    

    
  
As comment 32 states, it was checked in on the 14th and verified fixed by a few people.  Peter just didn't remove it from the red list yet.

    
  
Keywords: verified1.9.1
Keywords: fixed1.9.1
Crash Signature: [@ nsAppShell:EventWindow]

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: