Closed Bug 713140 Opened 8 years ago Closed 8 years ago

Browser Crash [@ nsDownloadManager::RemoveDownloadsForURI(nsIURI*) ]

Categories

(Toolkit :: Downloads API, defect, critical)

defect
Not set
critical

Tracking

()

VERIFIED FIXED
Tracking Status
firefox11 + verified
firefox12 --- verified

People

(Reporter: marcia, Unassigned)

References

Details

(Keywords: crash, regression, topcrash, Whiteboard: [qa!])

Crash Data

Seen while looking at trunk crash stats. Some Reports show up in 11.0a1, but if looks as if crashes increased in this signature on the trunk starting with the 20111220112824 build. So far all reports are Windows.

https://crash-stats.mozilla.com/report/list?signature=nsDownloadManager::RemoveDownloadsForURI%28nsIURI*%29

Possible pushlog regression range based on crash stats: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=d75ebb37080e&tochange=2afd7ae68e8b

https://crash-stats.mozilla.com/report/index/690629c4-f550-489d-b16a-7f50d2111222

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsDownloadManager::RemoveDownloadsForURI 	toolkit/components/downloads/nsDownloadManager.cpp:254
1 	mozutils.dll 	je_malloc 	memory/jemalloc/jemalloc.c:6306
2 	xul.dll 	NS_TableDrivenQI 	obj-firefox/xpcom/build/nsISupportsImpl.cpp:49
3 	mozutils.dll 	choose_arena 	memory/jemalloc/jemalloc.c:2972
4 	xul.dll 	nsMaybeWeakPtr_base::GetValueAs 	toolkit/components/places/nsMaybeWeakPtr.cpp:47
5 	xul.dll 	nsMaybeWeakPtr<nsINavHistoryObserver>::GetValue 	toolkit/components/places/nsMaybeWeakPtr.h:75
6 	xul.dll 	nsMaybeWeakPtr<nsINavHistoryObserver>::operator nsCOMPtr<nsINavHistoryObserver> const 	toolkit/components/places/nsMaybeWeakPtr.h:70
7 	xul.dll 	nsNavHistory::NotifyOnPageExpired 	toolkit/components/places/nsNavHistory.cpp:3950
8 	xul.dll 	XPTC__InvokebyIndex 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke_asm_x86_64.asm:90
9 	xul.dll 	XPTC__InvokebyIndex 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke_asm_x86_64.asm:129
10 	xul.dll 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1559
Linux: bp-0ceca950-ad58-4067-aebb-7bcc62111223
OSX: bp-f9ff5596-6935-4eaf-bed7-cd38e2111224
Crash Signature: [@ nsDownloadManager::RemoveDownloadsForURI(nsIURI*) ] → [@ nsDownloadManager::RemoveDownloadsForURI(nsIURI*) ] [@ nsDownloadManager::RemoveDownloadsForURI ]
OS: Windows 7 → All
Hardware: x86 → All
Seen mostly when users are upgrading or shutting down firefox normally
Just experienced this on Mac OSX after clicking the update "apply" button. Some minutes before that I cleaned up my download history.
Tim, can you reproduce this reliably?
It's #3 top crasher in 11.0a2 and #4 in 12.0a1.
Keywords: topcrash
Version: Trunk → 11 Branch
SeaMonkey 2.9a1 linux x86_64 (20120104003016) at closedown: bp-b330c4b6-81fa-4f2f-91eb-1bb782120105

sysout/syserr log ends as follows:
Document https://bugzilla.mozilla.org/show_bug.cgi?id=713280#c18 loaded successfully
*** LOG addons.xpi: Calling bootstrap method shutdown on restartless.restart@erikvold.com version 8
*** LOG addons.xpi: Calling bootstrap method shutdown on aboutstartup@glandium.org version 0.1.5
*** LOG addons.xpi: Calling bootstrap method shutdown on dominant.color@agadak.net version 2
*** LOG addons.xpi: Calling bootstrap method shutdown on tabstats@glandium.org version 0.0.2
*** LOG addons.xpi: Calling bootstrap method shutdown on customization@adblockplus.org version 1.0
*** LOG addons.xpi: Calling bootstrap method shutdown on abpwatcher@adblockplus.org version 1.2.1a.161
cz: Shutting down ChatZilla.

which makes me think that the addons manager was in the process of closing down but hadn't yet reached the end of it.

FWIW I have the Jökulsárlón download manager installed (version 0.4).
Summary: Firefox Crash [@ nsDownloadManager::RemoveDownloadsForURI(nsIURI*) ] → Browser Crash [@ nsDownloadManager::RemoveDownloadsForURI(nsIURI*) ]
Version: 11 Branch → Trunk
I also just saw this when I hit "apply update"; I *hadn't* just cleared my download history, though. A subsequent update didn't crash unfortunately :(
(In reply to Joe Drew (:JOEDREW!) from comment #7)
> [...] A subsequent update didn't crash unfortunately :(

Well, after comment #6 my subsequent closedowns did *not* crash, and I don't call that "unfortunate". :-)
(In reply to Sheila Mooney from comment #4)
> Tim, can you reproduce this reliably?

No, I tried several times with my possible STR, but no success.
I rather think it's an unwanted effect of bug 711536 and probably bug 713172 will fix this, though we should also find the crash point.
Blocks: 711536
(In reply to Scoobidiver from comment #10)
> I suspect the following changeset in bug 711799:
> http://hg.mozilla.org/mozilla-central/diff/fccaadf8161c/toolkit/components/
> places/nsNavHistoryResult.h

I don't see a relation with the stack, so I don't think it's related.
Looks like the download manager code uses cached statements and connection without null checking them. So yes bug 713172 will fix this, then imo we should use a StatementCache in the component, rather than pure cached statement members.
https://tbpl.mozilla.org/php/getParsedLog.php?id=8485460&tree=Firefox
Rev3 MacOSX Leopard 10.5.8 mozilla-central opt test mochitests-1/5 on 2012-01-11 11:18:49 PST for https://hg.mozilla.org/mozilla-central/rev/17a76e33b6fe (which is above https://hg.mozilla.org/mozilla-central/rev/a1a6f5452614)

PROCESS-CRASH | Shutdown | application crashed (minidump found)
Crash dump filename: /var/folders/Xr/Xr--yJnSEY0U11ET5NZuMU+++TM/-Tmp-/tmpDOMnxI/minidumps/B8145506-ADD1-4E8B-81D9-383DEAE4ADA8.dmp
Operating system: Mac OS X
                  10.5.8 9L31a
CPU: x86
      family 6 model 23 stepping 10
     2 CPUs

Crash reason:  EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE
Crash address: 0x0

Thread 0 (crashed)
 0  XUL!nsDownloadManager::RemoveDownloadsForURI [nsDownloadManager.cpp:17a76e33b6fe : 254 + 0x0]
    eip = 0x03efcdc8   esp = 0xbfffc720   ebp = 0x00000001   ebx = 0x03efcd3f
    esi = 0x143b4800   edi = 0x00000000   eax = 0x00000000   ecx = 0x14e21820
    edx = 0x00000000   efl = 0x00210246
    Found by: given as instruction pointer in context
 1  XUL!nsNavHistory::NotifyOnPageExpired [nsNavHistory.cpp:17a76e33b6fe : 3949 + 0x25]
    eip = 0x04027690   esp = 0xbfffc810   ebp = 0x00000001   ebx = 0x0402758c
    esi = 0x00000001   edi = 0x074cdb30
    Found by: call frame info
 2  XUL + 0x102d46f
    eip = 0x042a8470   esp = 0xbfffc890   ebp = 0xbfffc8c8   ebx = 0x03d9705f
    esi = 0xbfffcaf0   edi = 0xbfffcab0
    Found by: call frame info
 3  XUL!XPCWrappedNative::CallMethod [XPCWrappedNative.cpp:17a76e33b6fe : 2902 + 0x29]
    eip = 0x03d98c41   esp = 0xbfffc8d0   ebp = 0x00000005
    Found by: previous frame's frame pointer
(In reply to Marco Bonardo [:mak] from comment #13)
> So yes bug 713172 will fix this
Confirmed. The latest crashes occurs in 12.0a1/20120112 just before bug 713172 landed.

> we should use a StatementCache in the component, rather than pure cached 
> statement members.
I let this bug open as long as there's no bug filed for that.
with the backout done in bug 713172 (https://bugzilla.mozilla.org/show_bug.cgi?id=713172#c34) this should slowly disappear from the stats. Please confirm.
(In reply to Marco Bonardo [:mak] from comment #16)
> with the backout done in bug 713172
> (https://bugzilla.mozilla.org/show_bug.cgi?id=713172#c34) this should slowly
> disappear from the stats. Please confirm.

That's great news. We'll track and wait to see what happens.
(In reply to Marco Bonardo [:mak] from comment #16)
> with the backout done in bug 713172
> (https://bugzilla.mozilla.org/show_bug.cgi?id=713172#c34) this should slowly
> disappear from the stats. Please confirm.
Confirmed. The latest crash occurred in 11.0a2/20120120:
https://crash-stats.mozilla.com/report/list?signature=nsDownloadManager%3A%3ARemoveDownloadsForURI
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
This might be hard to verify if we can't reproduce this originally, qa+ for now.
Whiteboard: [qa+]
No crashes with this signature on 11 Beta, no crashes since the backout (20120120).
https://crash-stats.mozilla.com/report/list?signature=nsDownloadManager::RemoveDownloadsForURI%28nsIURI*%29

Unless there are some other specific STR, this can be set to verified.
Marking this verified based on no reports in crashstats. If someone can reproduce this please reopen.
Status: RESOLVED → VERIFIED
Whiteboard: [qa+] → [qa!]
You need to log in before you can comment on or make changes to this bug.