Closed Bug 409179 Opened 17 years ago Closed 16 years ago

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

Categories

(Toolkit :: Downloads API, defect, P2)

Product:
Toolkit
Component:
Downloads API
Type:
defect

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla1.9beta4

People

(Reporter: Roberthollar1990, Assigned: sdwilsh)

References

Details

(Keywords: crash)

Attachments

(4 files)

downloads.sqlite from a profile exhibiting crashes
5.00 KB, application/octet-stream
Details
Another downloads.sqlite file that has issues.
35.00 KB, application/octet-stream
Details
v1.0
5.55 KB, patch
Mardak
: review+
Details | Diff | Splinter Review
Screenshot showing post-patch Downloads window with corrupted downloads.sqlite
168.50 KB, image/png
Details 
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
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?
Version: unspecified → Trunk
Severity: major → critical
Keywords: crash
any site and yes in the tools options main and downloads ill screen capture a vid if you want
Did you submit crash data with breakpad after your crash?
Without a crash report, this is going to be hard to fix.  I'm not reproducing it...
Keywords: regression
Keywords: regression → qawanted
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.
it happens on any download form any website
(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!
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
Summary: Firefox Crashes → Firefox Crashes [@ nsDownloadProxy::OnStateChange]
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...
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.

That's, um, neat...

Would you by chance happen to have that downloads.sqlite file still (unmodified)?
yah but this happens for me on all downloads no matter what it is quick time movies(sometimes), mods, games, pictures, ect... 
(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.
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.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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.
Seeing this more - this should block.
Flags: blocking-firefox3?
Keywords: qawanted
Priority: -- → P1
Target Milestone: --- → Firefox 3 beta4
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.
Flags: blocking-firefox3? → blocking-firefox3+
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.
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.
Assignee: nobody → sdwilsh
Whiteboard: [needs patch]
Priority: P1 → P2
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?
I'm seeing downloads.sqlite.corrupt files in my profile now, so I suspect that is the case.  Can anyone else confirm this?
Keywords: qawanted
Whiteboard: [needs patch] → [fixed?]
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
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
Attached patch v1.0 — — Splinter Review 
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.
Attachment #304392 - Flags: review?(edilee)
Keywords: qawanted
Whiteboard: [fixed?] → [has patch][needs review Mardak]
builds are here.  If folks could test, that'd be awesome.
https://build.mozilla.org/tryserver-builds/2008-02-19_19:57-swilsher@mozilla.com-409179/
(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:57-swilsher@mozilla.com-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:57-swilsher@mozilla.com-409179/swilsher@mozilla.com-409179-firefox-try-win32.installer.exe, I *can* open the window and don't see errors in the Error Console.
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.
Status: NEW → ASSIGNED
Flags: in-testsuite?
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.
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...
Blocks: 394247
Summary: Firefox Crashes [@ nsDownloadProxy::OnStateChange] → Inconsistent autoresume state causes download manager instantiation to fail (can lead to crash)
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 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)
Attachment #304392 - Flags: review?(edilee) → review+
(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)?
Whiteboard: [has patch][needs review Mardak] → [has patch][has review]
Whiteboard: [has patch][has review] → [has patch][has review][can land]
(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?
OS: Windows XP → All
Hardware: PC → All
Yeah, I'd expect comment 35 to always cause this, which means this particular case should *not* happen on anything but windows.
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?
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?
Timeless, no idea. But we should move this to a new bug if Ted says yes.
timeless: that's bug 393317 (also filed upstream, but not getting any traction on it)

also note that this code lives in a .h file, which I imagine may cause part of the issue?
Shawn, could you also give a short note to my comment 37?
(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.
(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?

(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.
(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?
(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.
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
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Flags: in-testsuite?
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [has patch][has review][can land]
(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.
Product: Firefox → Toolkit
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: