206337 - Trunk M17rc1 crash on exit [@ nsDownloadManager::Observe]

Status: RESOLVED FIXED

(SeaMonkey :: Download & File Handling, defect)

Description

[Jan Carpenter]

trunk topcrash [@ nsDownloadManager::Observe]

nsDownloadManager::Observe   48 
Crash data range: 2003-05-09 to 2003-05-19
Build ID range: 2003050908 to 2003051808
 
     Count   Platform List 
     27   Windows NT 5.1 build 2600
     21   Windows NT 5.0 build 2195
     11   Windows 98 4.10 build 67766222
     1   Windows NT 5.2 build 3790
     1   Windows 98 4.10 build 67766446

Stack Trace: 

	 nsDownloadManager::Observe
[c:/builds/seamonkey/mozilla/xpfe/components/download-manager/src/nsDownloadManager.cpp
 line 913]
	 nsObserverService::NotifyObservers
[c:/builds/seamonkey/mozilla/xpcom/ds/nsObserverService.cpp  line 212]
	 nsProfile::ShutDownCurrentProfile
[c:/builds/seamonkey/mozilla/profile/src/nsProfile.cpp  line 1346]
	 DoOnShutdown	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line
777]
	 main	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1655]
	 WinMain	[c:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1672]
	 WinMainCRTStartup()
	 KERNEL32.dll + 0x2847c (0x77ea847c)
 
 	Source File :
c:/builds/seamonkey/mozilla/xpfe/components/download-manager/src/nsDownloadManager.cpp
line : 913
     (20206838)	Comments: closing last mozilla windows...
     (20182012)	Comments: closing mozilla
     (20143011)	URL: www.google.com
     (19985481)	URL: www.google.com
     (19985481)	Comments: ok  i just sent one of these. it seems to be crashing
EVERY time i close the browser. my last report mentioned PDF and ezcd creator if
you need to search for it. this is getting VERY annoying.
     (19985433)	URL: www.google.com
     (19985433)	Comments: had opened a PDF linked to from google. when i closed
acrobat reader (opened in a separate window  not in browser) all was fine. i
then closed the browser window and got the crash  probably while it was
reloading itself to the systray. this is a fresh XP
     (19985433)	Comments:  install  i've only patched windows and installed
mozilla and ezcd creator 5.x.x that came with my plextor dvd+rw  acrobat reader
 logitech mouseware  and java
     (19980402)	Comments: crash while closing mozilla
     (19975358)	Comments: can't remember what was open  computer was locked for
about 5 hours
     (19963945)	URL: http://www.vtk.rug.ac.be/kancomite
     (19958616)	URL: http://www.vtk.rug.ac.be/kancomite
     (19957230)	Comments: closing mozilla...

Comment 1

[Jay Patel [:jay]]

This continues to be a topcrasher for Mozilla 1.4 RC1.  There are also recent
crashes with Trunk and M140 Branch builds.  Here is the latest from Talkback for
M140RC1:

Count   Offset    Real Signature
[ 14   nsDownloadManager::Observe ee318bdd - nsDownloadManager::Observe ]
[ 6   nsDownloadManager::Observe dc724c05 - nsDownloadManager::Observe ]
[ 5   nsDownloadManager::Observe db67695b - nsDownloadManager::Observe ]
[ 5   nsDownloadManager::Observe be5208a6 - nsDownloadManager::Observe ]
[ 5   nsDownloadManager::Observe 5fc208d5 - nsDownloadManager::Observe ]
[ 2   nsDownloadManager::Observe e17a3db8 - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe ffe942a6 - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe b5cfd71a - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe aadec7ef - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe 7fe98028 - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe 79f96350 - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe 6c3f8b17 - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe 4a9d34e0 - nsDownloadManager::Observe ]
[ 1   nsDownloadManager::Observe 49b99ad8 - nsDownloadManager::Observe ]
 
     Crash date range: 2003-05-31 to 2003-06-09
     Min/Max Seconds since last crash: 6 - 608161
     Min/Max Runtime: 1021 - 716864
 
     Count   Platform List 
     15   Windows NT 5.0 build 2195
     13   Windows NT 5.1 build 2600
     9   Windows 98 4.10 build 67766222
     6   Windows 98 4.10 build 67766446
     2   Windows 98 4.90 build 73010104
 
     Count   Build Id List 
     45   2003052908
 
     No of Unique Users        20
 
 Stack trace(Frame) 

	 nsDownloadManager::Observe
[d:/builds/seamonkey/mozilla/xpfe/components/download-manager/src/nsDownloadManager.cpp
 line 917] 
	 nsObserverService::NotifyObservers
[d:/builds/seamonkey/mozilla/xpcom/ds/nsObserverService.cpp  line 212] 
	 nsProfile::ShutDownCurrentProfile
[d:/builds/seamonkey/mozilla/profile/src/nsProfile.cpp  line 1346] 
	 DoOnShutdown	[d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line
777] 
	 main	[d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1655] 
	 WinMain	[d:/builds/seamonkey/mozilla/xpfe/bootstrap/nsAppRunner.cpp  line 1672] 
	 WinMainCRTStartup()  
	 KERNEL32.dll + 0x2847c (0x77ea847c)   
 
     (20817000)	Comments: closing last mozilla window...
     (20816278)	Comments: Sitting at the front page of slashdot.org. I hit alt
then released as to read a story. I then pressed alt+f4 to close it. It seemed
to close just fine then the feedback agent appeared.
     (20798349)	Comments: Had read my email  sent a post  sorted inbox items and
went to File  Exit to end program.
     (20719866)	Comments: Had quit mozilla then right clicked on task bar icon
to quit totally  then mozilla crashed
     (20669639)	Comments: cloed down browser 
     (20655221)	Comments: closed the browser
     (20637123)	URL: on exiting the taskbar icon thing.
     (20637110)	URL: on reloading the taskbar icon thing.
     (20636610)	URL: on reloading the taskbar icon thing.
     (20618674)	URL: exiting taskbar.

Comment 2

[Matthias Versen [:Matti]]

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

Comment 3

[Matthias Versen [:Matti]]

Chris: can you ask a someone (with a "Can't save files." comment) from the
reports if renaming "downloads.rdf" in the profile helps ?

Comment 4

[Jay Patel [:jay]]

Updating summary with M17a since this is a topcrasher for Mozilla 1.7 alpha.

matti:  I've identified a user that has been crashing consistently and will ask
him to try your suggestion.  

Comment 5

[Jay Patel [:jay]]

No luck with responses from users.  I just emailed another user to see if he/she
can help us.

Comment 6

[Jay Patel [:jay]]

Matti:  Do you think it's a corrupt downloads.rdf file that might be causing
this?  If so, is there a way to edit the file to see if we can reproduce the
crash ourselves?

Comment 7

[Christian :Biesinger (don't email me, ping me on IRC)]

from a mail conversation between bz and me about this bug:

maybe some people write-protected their downloads.rdf, or created a folder with
that name, and that caused this crash?

Christian Biesinger wrote:

>> See http://talkback-public.mozilla.org/reports/M17b/smart-analysis.all and
note crash #13.  Any idea what's up?
>
>
> Well, that line is:
>     if (NS_FAILED(rv)) return rv;


Talkback is a lying little bitch about line numbers.

> OK, more likely is the line before:
>     rv = mDataSource->GetSources(gNC_DownloadState, intLiteral, PR_TRUE,
getter_AddRefs(downloads));
>
> helpful would be the contents of local variables...


Yeah... See comments in .seamonkey about privacy issues.  :(

> Could
>   rv = gRDFService->GetDataSourceBlocking(downloadsDB.get(),
getter_AddRefs(mDataSource));
>   if (NS_FAILED(rv)) return rv;
> possibly fail? (in ::Init)


I don't see why not (eg failure to create downloads.rdf on disk).

> Some lines before that, there's
>   obsService->AddObserver(this, "profile-before-change", PR_FALSE);
>   obsService->AddObserver(this, "profile-approve-change", PR_FALSE);
> PR_FALSE means, I suppose, "keep strong reference".
>
> that'd keep the object alive and explain the crash...


Indeed.  We should probably move those to the very end of the method, since we
don't want a ref to us sticking about if we failed to init.

> Failure of:
>   rv = GetProfileDownloadsFileURL(downloadsDB);
> might also explain it, but I find that somewhat unlikely - it's basically
GetSpecialDirectory followed by GetUrlSpecFromFile.


GetSpecialDirectory returns a non-directory in this case, eh?  Lovely... just
lovely.

> guess that analysis should be put into some bug.


Yeah.  Wanna file?  ;)

> the question is of course why GetDataSourceBlocking would fail; and if that
analysis is correct.


Indeed.... I'm not sure how to test it offhand, but as I pointed out above it
_can_ fail and we should deal with it failing.

> does trunk talkback show the crash? assuming it works...


It does not show the crash, and it does work.  But it has 114 incidents from 95
users recorded.  1.7b branch has 1585 crashes from 1136 users recorded; of these
crashes 12 are in this method.  So by simple ratios we should expect about 4/5
of a report of a crash in this method on trunk.  Given that, 0 is not too
surprising.  ;)

-Boris

Comment 8

[Christian :Biesinger (don't email me, ping me on IRC)]

Attachment: patch

Comment 9

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 146699 [details] [diff] [review]
patch

sr=bzbarsky if you add a comment explaining that those two calls MUST be the
last things in Init() and why.

Comment 10

[Christian :Biesinger (don't email me, ping me on IRC)]

ok, I'll add the following comment: 
// The following two AddObserver calls must be the last lines in this function,
// because otherwise, this function may fail (and thus, this object would be not
// completely initialized), but the observerservice would still keep a reference
// to us and notify us about shutdown, which may cause crashes.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Sounds good.

Comment 12

[[e-mail]]

Comment on attachment 146699 [details] [diff] [review]
patch

Why not just comment that you can't usefully observe profile changes until your
members are initialized, or something?

Comment 13

[Christian :Biesinger (don't email me, ping me on IRC)]

hm... I do want to express that failure from Init should destroy the object and
these observers are preventing it

Comment 14

[Christian :Biesinger (don't email me, ping me on IRC)]

Comment on attachment 146699 [details] [diff] [review]
patch

checked in on trunk.

this probably fixes an 1.7b topcrash, and would be good to have for 1.7. risk
is low.

Comment 15

[Boris Zbarsky [:bzbarsky]]

Filed bug 241611 on equivalent Firefox code.

Comment 16

[Jay Patel [:jay]]

M17beta -> M17rc1 for tracking.  This continues to be a topcrash for Mozilla 1.7
RC1.  Any chance we can get this checked in on the 1.7 branch?  If it's a low
risk patch, it would be nice to see if this crash goes away in RC2 (we don't
have enough Talkback data for the MozillaTrunk to verify this fix or a solid
reproducible test case yet).

Comment 17

[Boris Zbarsky [:bzbarsky]]

Jay, we can check it into branch as soon as it gets approval.  Kinda need that,
y'know.

Comment 18

[chris hofmann]

Comment on attachment 146699 [details] [diff] [review]
patch

a=chofmann for 1.7

Comment 19

[Christian :Biesinger (don't email me, ping me on IRC)]

checked in on 1.7 branch

Checking in nsDownloadManager.cpp;
/cvsroot/mozilla/xpfe/components/download-manager/src/nsDownloadManager.cpp,v 
<--  nsDownloadManager.cpp
new revision: 1.91.2.1; previous revision: 1.91
done

I'm leaving this open for now, to see whether this actually fixed the crash

Comment 20

[Oliver Klee]

Any news on whether this actually fixed the crash?

Comment 21

[Jay Patel [:jay]]

If anyone was able to reproduce this before the fix, please try to crash with a
recent Trunk nightly and let us know how it went.

I don't see any crashes in the latest MozillaTrunk Talkback data, but until
Mozilla 1.7 rc2 goes out, we won't know how it looks on the branch.  So far, so
good.

Comment 22

[Jay Patel [:jay]]

Marking fixed since the patch is in.  We can verify it once we get more Talkback
data for rc2.  But if anyone is able to reproduce with a recent build, please
reopen.

Comment 23

[David Epstein]

Verified on the branch, Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US;
rv:1.8a2) Gecko/20040701. not crashing on exit. verified patch checkin on
Mozilla 1.7 branch tree.