409179 - Inconsistent autoresume state causes download manager instantiation to fail (can lead to crash)

Status: RESOLVED FIXED

(Toolkit :: Downloads API, defect, P2)

Description

[Robert Hollar]

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9b2) Gecko/2007121120 Firefox/3.0b2

Every time I try to download something Firefox jest crashes I've reinstalled it twice now nothing works. It happened after i switched around the download locations in the options pannel.

Reproducible: Always

Steps to Reproduce:
1.Change download location a few times
2.Click on any download
3.Watch as Firefox dies
4.Open Explorer and download with it
Actual Results:  
you get the file with Explorer and not Firefox and Firefox dies

Expected Results:  
downloaded the file <- I hope

No add-ons not themes
I'm using Xp pro sp2
Intel Core 2 Duo
Nvidia 8800Gts

Comment 1

[Steve England [:stevee]]

Do you submit crash data with breakpad after your crash?

"1.Change download location a few times"
- This is in Tools, Options, Main, Downloads?

"2.Click on any download"
- Any download from what site?

Comment 2

[Robert Hollar]

any site and yes in the tools options main and downloads ill screen capture a vid if you want

Comment 3

[Steve England [:stevee]]

Did you submit crash data with breakpad after your crash?

Comment 4

[Shawn Wilsher :sdwilsh]

Without a crash report, this is going to be hard to fix.  I'm not reproducing it...

Comment 5

[Robert Hollar]

BuildID: 2007121120
CrashTime: 1198195497
Email: 
InstallTime: 1198151926
Name: 
ProductName: Firefox
SecondsSinceLastCrash: 41867
URL: http://www.fpsbanana.com/skins/download/34780
UserID: 813e6f97-9c9b-445c-8c49-673fc769c5c2
Vendor: Mozilla
Version: 3.0b2

This report also contains information about the state of the application when it crashed.

Comment 6

[Robert Hollar]

it happens on any download form any website

Comment 7

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #5)
> BuildID: 2007121120
> CrashTime: 1198195497
> Email: 
> InstallTime: 1198151926
> Name: 
> ProductName: Firefox
> SecondsSinceLastCrash: 41867
> URL: http://www.fpsbanana.com/skins/download/34780
> UserID: 813e6f97-9c9b-445c-8c49-673fc769c5c2
> Vendor: Mozilla
> Version: 3.0b2

Can you please include the actual incident ID?

http://kb.mozillazine.org/Breakpad tells you how to give us the URL that's included in the crash report pertinent to this bug; thanks!

Comment 8

[Robert Hollar]

bp-3dec5d05-aa35-11dc-951d-001a4bd46e84

Comment 9

[Steve England [:stevee]]

Signature	nsDownloadProxy::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, unsigned int)
UUID	3dec5d05-aa35-11dc-951d-001a4bd46e84
Time	2007-12-14 03:10:36-08:00
Build ID	2007110904
OS	Windows NT
OS Version	5.1.2600 Service Pack 2
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 6
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x0
Frame  	Signature  	Source
0 	nsDownloadProxy::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, unsigned int) 	mozilla/toolkit/components/downloads/src/nsDownloadProxy.h:118
1 	nsExternalAppHandler::CreateProgressListener() 	mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp:1833
2 	nsExternalAppHandler::LaunchWithApplication(nsIFile*, int) 	mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp:2158
3 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
4 	AutoJSSuspendRequest::SuspendRequest() 	mozilla/js/src/xpconnect/src/xpcprivate.h:3363
5 	XPCCallContext::XPCCallContext(XPCContext::LangType, JSContext*, JSObject*, JSObject*, long, unsigned int, long*, long*) 	mozilla/js/src/xpconnect/src/xpccallcontext.cpp:160

Comment 10

[Shawn Wilsher :sdwilsh]

So, mInner is null, but I don't see how that's possible, unless nsDownloadManager::AddDownload is returning NS_OK *and* passing out nsnull as the nsIDownload object...

Comment 11

[Leonard Norberg]

I can reproduce this crash in one of my profiles. Some downloads crash and other downloads simply never start. The download manager is empty.
Sample crash: bp-5403a826-b15c-11dc-9f60-001a4bd46e84

I isolated the problem to downloads.sqlite. Comparing the database schema with one from a freshly created downloads.sqlite I found that the problematic file had one additional column (iconURL TEXT) and was missing one column (referrer TEXT).
Adding a 'referrer' column of type TEXT (using sqlite3Explorer) to the problematic file made the crash go away.

This particular trunk install was directly updated from 2007110405 to 2007122205.

Comment 12

[Shawn Wilsher :sdwilsh]

That's, um, neat...

Would you by chance happen to have that downloads.sqlite file still (unmodified)?

Comment 13

[Leonard Norberg]

Attachment: downloads.sqlite from a profile exhibiting crashes

Comment 14

[Robert Hollar]

yah but this happens for me on all downloads no matter what it is quick time movies(sometimes), mods, games, pictures, ect... 

Comment 15

[Shawn Wilsher :sdwilsh]

(In reply to comment #14)
> yah but this happens for me on all downloads no matter what it is quick time
> movies(sometimes), mods, games, pictures, ect... 
Can you please attach your downloads.sqlite file?  That will probably be the easiest way to determine what's up with this.

Comment 16

[Henrik Skupin [:whimboo][⌚️UTC+2]]

On OS X there is no crash but nothing happens after hitting the save button inside the save as dialog. I get an error: Can't convert to String.

If such things happen and we cannot update to the new schema shouldn't we recreate the table? Is there a way to check this? It will also apply to all other sqlite databases.

Confirming bug due to existence of crash report.

Comment 17

[Bob Nugen]

My crashes occurred while downloading several files from SourceForge.net. The first crash followed something in the order of 10 downloads, and the second occurred after ~8 downloads.  All downloads were saved into same directory. Can't reproduce specifics at this time, but will continue to attempt to find a reproducible sequence that will cause the crash.

Comment 19

[Shawn Wilsher :sdwilsh]

Seeing this more - this should block.

Comment 20

[Jonathan Turner]

Was experiencing this bug in OS X (10.5).  Moved my /Users/<username>/Library/Application\ Support/Firefox somewhere else (after seeing mention of the version different with the sqlite file) and it fixed it.  Downloads now complete and I can see the download manager window.

Not sure if that helps, just thought I'd mentioned it.  

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Shawn, when I replace my own downloads.sqlite file with the attached one the download manager code goes crazy. When it is initialized after some seconds on start of Firefox I get following error:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: anonymous :: line 988"  data: no]

This is here:
http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/browser/base/content/browser.js&rev=1.965&mark=1073-1079&#1068

Even when I try to open the Download Manager window nothing happens and I get a similar error:

Error: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: file:///Users/henrik/Projects/mozilla/bin/Minefield.app/Contents/MacOS/components/nsDownloadManagerUI.js :: show :: line 80"  data: no]

http://bonsai.mozilla.org/cvsblame.cgi?file=/mozilla/toolkit/components/downloads/src/nsDownloadManagerUI.js&rev=1.6&mark=78-81&#61

No idea if this is related to this bug or if I should file a new one.

Comment 22

[Shawn Wilsher :sdwilsh]

I haven't had time to investigate this, but I'm imagining this bug lies with mozStorage or the download manager.  I hope to investigate it this weekend or later next week.

Comment 23

[Shawn Wilsher :sdwilsh]

So, I made a test case for this and everything, but it's totally WFM:
Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008021820 Minefield/3.0b4pre

What does it *not* work on?

Maybe this was fixed with bug 416205?

Comment 24

[Shawn Wilsher :sdwilsh]

I'm seeing downloads.sqlite.corrupt files in my profile now, so I suspect that is the case.  Can anyone else confirm this?

Comment 25

[Landon Abney]

Attachment: Another downloads.sqlite file that has issues.

Here is a downloads.sqlite file that exhibits behavior close to what this bug has morphed into...doesn't seem to match what the original filer has said though.

On first opening firefox I see : 
Error: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: chrome://browser/content/browser.js :: anonymous :: line 917"  data: no]

When trying to open the download manager I get:
Error: uncaught exception: [Exception... "Component returned failure code: 0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE) [nsIJSCID.getService]"  nsresult: "0x80570016 (NS_ERROR_XPC_GS_RETURNED_FAILURE)"  location: "JS frame :: file:///C:/Program%20Files/Minefield/components/nsDownloadManagerUI.js :: show :: line 80"  data: no]

When attempting to download any file I get:
Error: uncaught exception: unknown (can't convert to string)

I am seeing this on a completely new profile that has only had this downloads.sqlite file placed in it.

Mozilla/5.0 (Windows; U; Windows NT 6.0; en-US; rv:1.9b4pre) Gecko/2008021904 Minefield/3.0b4pre

Comment 26

[Landon Abney]

Sorry forgot to mention this in my earlier comment:
I do not see any downloads.sqlite.corrupt files in my profile. Also when you try to open a file in its associated application when using this downloads.sqlite file firefox crashes. Here is an example breakpad report from this crash:
http://crash-stats.mozilla.com/report/index/b8df645c-de44-11dc-acca-001a4bd43e5c?date=2008-02-18-17

Comment 27

[Shawn Wilsher :sdwilsh]

Attachment: v1.0

So much for studying for my midterm...

OK, so in that database it's the download with the id of 152.  The issue is that the download is in the state of DOWNLOAD_FINISHED, but autoResume is set to AUTO_RESUME.  Therefore, our query in RestoreActiveDownloads try's to restore it, which will fail (I'm not sure how, since my failure on mac is different than what you'd get).

I'm not sure how this worked right before in the first place...

I've submitted this to the try-server to post builds.  We'll see if this fixes the issue for others.  I'll post the link to the builds once one is out.

Comment 28

[Shawn Wilsher :sdwilsh]

builds are here.  If folks could test, that'd be awesome.
https://build.mozilla.org/tryserver-builds/2008-02-19_19:[e-mail]-409179/

Comment 29

[Stephen Donner [:stephend] Not actively reading bugmail]

(In reply to comment #28)
> builds are here.  If folks could test, that'd be awesome.
> https://build.mozilla.org/tryserver-builds/2008-02-19_19:[e-mail]-409179/

A) Before, *I could reproduce the un-openable Download Manager window* when using the downloads.sqlite file from comment 13 with the trunk Windows XP/Vista builds

B) Now, though, with https://build.mozilla.org/tryserver-builds/2008-02-19_19:[e-mail]-409179/[e-mail], I *can* open the window and don't see errors in the Error Console.

Comment 30

[Stephen Donner [:stephend] Not actively reading bugmail]

Attachment: Screenshot showing post-patch Downloads window with corrupted downloads.sqlite

Now get back to studying, sdwilsh!

Comment 31

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Verified also by myself. This patch works great. While I was able to reproduce the crash with the 2nd test case while trying to download and open a zip file, I don't see it anymore with your tryserver build.

What makes me wonder is, that such inconsistent states could produce crashes that way.

Comment 32

[Shawn Wilsher :sdwilsh]

It crashed because the download manager service could not be instantiated, so mInner in nsDownloadProxy was null.  I'm pretty sure there's a bug on nsITransfer->Init failing for exthandler that would also fix this.

Comment 33

[Shawn Wilsher :sdwilsh]

Morphing bug.  The crashing bits are over in bug 394247.

Edward - we need to figure out how this got into the state it got into and fix that as well.  I don't quite like the existing code since it will hide any future problems we may introduce.  I'm also wondering if it's just a beta 2 bug that we since fixed, or what...

Comment 34

[Landon Abney]

I run trunk builds so if the bug that caused that is in beta 2 it is also in trunk. (My build that would have caused that would be a day or two behind the date of the download).

Comment 35

[Ed Lee :Mardak]

Comment on attachment 304392 [details] [diff] [review]
v1.0

>+  // Make sure we do not automatically resume
>+  mAutoResume = DONT_RESUME;
Odd.. the only place we set AUTO_RESUME is PauseAllDownloads and RestoreActiveDownloads. The latter only changes things at startup and is a single query, so that probably won't have some race condition. PauseAllDownloads could potentially have issues.. maybe with the virus scanner and the user quits?

That would look like..

1) start virus scan
2) quit app
2) pause downloads setting autoresume
2) update db to have autoresume
1) scan done setstate to completed
1) db already has autoresume set and only changes to FINISHED

So setting DONT_RESUME in Finalize should fix that case.

r=Mardak

(Don't forget to add the sqlite file on checkin)

Comment 36

[Shawn Wilsher :sdwilsh]

(In reply to comment #35)
> 1) start virus scan
> 2) quit app
> 2) pause downloads setting autoresume
> 2) update db to have autoresume
> 1) scan done setstate to completed
> 1) db already has autoresume set and only changes to FINISHED
That's what I was thinking maybe.  Did anyone on mac see this bug (naturally, not by using this downloads.sqlite file)?

Comment 37

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #36)
> That's what I was thinking maybe.  Did anyone on mac see this bug (naturally,
> not by using this downloads.sqlite file)?

I wasn't even able to reproduce the bug with the attached downloads.sqlite all the last time. But now I'm also able to crash Mozilla/5.0 (Macintosh; U; Intel Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022004 Minefield/3.0b4pre ID:2008022004

Do you mean that we should test it with the above steps mentioned in comment 35?

Comment 38

[Shawn Wilsher :sdwilsh]

Yeah, I'd expect comment 35 to always cause this, which means this particular case should *not* happen on anything but windows.

Comment 39

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, I'll try to run a test within the next days. Perhaps I'll find some time during FOSDEM. Meanwhile the missing breakpad id: bp-65575211-e004-11dc-8068-001a4bd43ef6

Comment 35 states "> 1) start virus scan". What should be scanned? An already downloaded file?

Comment 40

[timeless]

Signature	nsDownloadProxy::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, unsigned int)
UUID	65575211-e004-11dc-8068-001a4bd43ef6
Time	2008-02-20 14:36:52-08:00
Uptime	0
Product	Firefox
Version	3.0b4pre
Build ID	2008022004
OS	Mac OS X
OS Version	10.4.11 8S2167
CPU	x86
CPU Info	GenuineIntel family 6 model 15 stepping 6
Crash Reason	EXC_BAD_ACCESS / KERN_PROTECTION_FAILURE
Crash Address	0x1960ee7
Comments	Testing testcase on bug 409179.
Crashing Thread
Frame 	Signature 	Source
0 	nsDownloadProxy::OnStateChange(nsIWebProgress*, nsIRequest*, unsigned int, unsigned int) 	mozilla/gfx/cairo/cairo/src/cairo-hull.c:116
1 	nsExternalAppHandler::CreateProgressListener() 	mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp:1862
2 	nsExternalAppHandler::LaunchWithApplication(nsIFile*, int) 	mozilla/uriloader/exthandler/nsExternalHelperAppService.cpp:2191
3 	NS_InvokeByIndex_P 	mozilla/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179
4 	XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode) 	mozilla/js/src/xpconnect/src/xpcwrappednative.cpp:2339

anyone know why the source file is wrong?

Comment 41

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Timeless, no idea. But we should move this to a new bug if Ted says yes.

Comment 42

[(not currently active) Ted Mielczarek]

timeless: that's bug 393317 (also filed upstream, but not getting any traction on it)

Comment 43

[Shawn Wilsher :sdwilsh]

also note that this code lives in a .h file, which I imagine may cause part of the issue?

Comment 44

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Shawn, could you also give a short note to my comment 37?

Comment 45

[Shawn Wilsher :sdwilsh]

(In reply to comment #37)
> I wasn't even able to reproduce the bug with the attached downloads.sqlite all
> the last time. But now I'm also able to crash Mozilla/5.0 (Macintosh; U; Intel
> Mac OS X 10.4; en-US; rv:1.9b4pre) Gecko/2008022004 Minefield/3.0b4pre
> ID:2008022004
So, do you mean that in past nightlies you couldn't crash, but in the tryserver build you did?  Or did the nightlies suddenly start making you crash with it?

> Do you mean that we should test it with the above steps mentioned in comment
> 35?
Yes, but on the mac it's not going to happen.  I strongly suspect that this bug would only show itself on windows.

Comment 46

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #45)
> So, do you mean that in past nightlies you couldn't crash, but in the tryserver
> build you did?  Or did the nightlies suddenly start making you crash with it?

Oh sorry for the bad wording. The official nightly builds crash. I haven't tested the patch on OS X right now.

> Yes, but on the mac it's not going to happen.  I strongly suspect that this bug
> would only show itself on windows.

Mmh, one question remains... which files I have to scan before quitting Minefield?

Comment 47

[Shawn Wilsher :sdwilsh]

(In reply to comment #46)
> Mmh, one question remains... which files I have to scan before quitting
> Minefield?
The download manager automatically starts a virus scan (with supporting virus scanners) when a download completes.  If you quit before that finishes, we (being Edward and I) suspect you'll hit this bug.

Comment 48

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #47)
> The download manager automatically starts a virus scan (with supporting virus
> scanners) when a download completes.  If you quit before that finishes, we
> (being Edward and I) suspect you'll hit this bug.

Do you have a list of supported virus scanners? It seems that Norton Symantec Antivirus isn't on that list. :/ I would try to test it with another one but have to know which are supported. Also the best testcase would be to download a large file. And why it would not appear under OS X? Are no virus scanners supported?

Comment 49

[Shawn Wilsher :sdwilsh]

(In reply to comment #48)
> Do you have a list of supported virus scanners? It seems that Norton Symantec
> Antivirus isn't on that list. :/ I would try to test it with another one but
> have to know which are supported. Also the best testcase would be to download a
> large file. And why it would not appear under OS X? Are no virus scanners
> supported?
AVG implements the required interface I think.  It doesn't work on OS X because we use a windows API.

Comment 50

[Shawn Wilsher :sdwilsh]

I'm resolving this for now - I strongly suspect that the issue is fixed.

Checking in toolkit/components/downloads/src/nsDownloadManager.cpp;
new revision: 1.166; previous revision: 1.165
Checking in toolkit/components/downloads/test/unit/test_bug_409179.js;
initial revision: 1.1
Checking in toolkit/components/downloads/test/unit/bug_409179_downloads.sqlite;
initial revision: 1.1

Comment 51

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #49)
> AVG implements the required interface I think.  It doesn't work on OS X because
> we use a windows API.

I'll have a look at this within the next days. I installed Bitdefender under my newly Windows Vista Test VM and the scanner is called after the download has finished downloading.