Open Bug 485651 Opened 15 years ago Updated 2 years ago

.part file is blown away on completed download, leaving 0-byte file

Categories

(Toolkit :: Downloads API, defect, P3)

Product:
Toolkit
Component:
Downloads API
Platform:
x86
Windows Vista
Type:
defect

Tracking

()

People

(Reporter: vlad, Unassigned)

Details

I've seen this happen randomly before, but now I have it 100% reproducible.  I download a file, I see the .part file growing, and when the download finishes it's just deleted leaving me with a 0-byte file with the original name (which was created at the start).

I think this failure at least might have to do with virus scanning; my virus scanner is waiting for me to reboot to complete an update, and so I'm wondering if firefox can't manage to do the scan for some reason, and just blows away the .part file.

Regardless, we should be -super- careful to not delete .part files randomly -- perhaps even going so far as to prompt the user if the .part file is above some threshold.  The file that I just lost was a 3gb download :(
Flags: blocking1.9.1?
I should mention -- I have showInConsole set to true, and there aren't any errors/exceptions in the js console.
Can you catch it in a debugger to see where we're going wrong? Or can it be easily replicated if we post an almost-done large .part file somewhere and "resume" it?
Well, I can attach a debugger if need be, but I think that the relevant code is all in js.. no useful way to debug that, especially not without restarting =/  I can reproduce it on demand though, so if someone who knows the dl mgr/virus checker/etc. has any ideas on where to set breakpoints and stuff, I can give it a shot.  Right now, anything that I download ends up as 0-size -- save as, download, whatever.
You don't have any of the Windows parental controls set such that they delete the downloads on completion, do you?

Shawn: can you meander over to Vlad's desk and see what's going on?
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
(In reply to comment #0)
> I've seen this happen randomly before, but now I have it 100% reproducible.  I
> download a file, I see the .part file growing, and when the download finishes
> it's just deleted leaving me with a 0-byte file with the original name (which
> was created at the start).
> 
> I think this failure at least might have to do with virus scanning; my virus
> scanner is waiting for me to reboot to complete an update, and so I'm wondering
> if firefox can't manage to do the scan for some reason, and just blows away the
> .part file.
> 

Any scan would take place on the final file, not the part. 

You can try turning off virus scanning using - 

browser.download.manager.scanWhenDone

The code for scanning is in toolkit and is C++ so you can step through it. (Although the virus scan stuff is really just an api call.)

You could also kill off all the security policy check stuff - 

browser.download.manager.skipWinSecurityPolicyChecks

Which might help as well.
(In reply to comment #4)
> Shawn: can you meander over to Vlad's desk and see what's going on?
I would suspect that rob or jimm would be more useful in this regard.
This was unfortunately on my home machine, and the browser that was doing it crashed (due to some stuff that I was doing), so it's no longer in that state.  Regardless, a close examination of when we delete/rename .part files would probably be a good thing.
(In reply to comment #7)
> This was unfortunately on my home machine, and the browser that was doing it
> crashed (due to some stuff that I was doing), so it's no longer in that state. 
> Regardless, a close examination of when we delete/rename .part files would
> probably be a good thing.

I was searching around because I thought we had an open bug on left over zero byte part files, but can't find it. I was wondering if you were seeing two bugs - one with part files and one with a dis-functional 3rd party scanner. 

A common theme in these apis seems to be to wrap the call in an exception handler and if anything goes wrong in the scan, delete the file to be safe. So if you could get this to reproduce, turning off virus scanning and seeing if the part file problem remains while the file delete problems goes away would be interesting.
(In reply to comment #8)
> A common theme in these apis seems to be to wrap the call in an exception
> handler and if anything goes wrong in the scan, delete the file to be safe. So
> if you could get this to reproduce, turning off virus scanning and seeing if
> the part file problem remains while the file delete problems goes away would be
> interesting.

The idea of the exception handler is a poor-man's sandbox for the untrusted virus scanner code. Given this common behavior of deleting the file when it fails, we could create a hardlink to the original file and let the scanner scan that. If it does delete it, then we still have the original without consuming a lot more disk space. We do need to figure out when it deletes it on accident so that we don't defeat the purpose of the virus scanner. A working copy of such a scanner would be useful for figuring out return values and such.
Does the -scanner- delete?  I was under the impression that -we- deleted if anything went wrong during virus scanning (that is, if an exception was thrown), even if virus scanning itself didn't report an error.
(In reply to comment #10)
> Does the -scanner- delete?  I was under the impression that -we- deleted if
> anything went wrong during virus scanning (that is, if an exception was
> thrown), even if virus scanning itself didn't report an error.


The individual scanner is expected to scan and modify a file as needed. That might include deleting a file it can't clean up. This is third party code so how developers treat the situation isn't guaranteed across all vendors. (What's your virus software by the way?)

Also, we make one of two calls - with skipWinSecurityPolicyChecks set, instead of calling the virus scanner directly, we ask windows to do it for us, so that internet policy is applied and security meta added to the file. In cases where security policy dictates the file download should not be allowed, windows will delete the file. This case however would be clear to the user through a specific error message in the download manager.
I'm using AVG, I believe the latest version.

So there are no cases where we delete the .part file ourselves (whether related to virus scanning or not)?
(In reply to comment #12)
> I'm using AVG, I believe the latest version.
> 
> So there are no cases where we delete the .part file ourselves (whether related
> to virus scanning or not)?

Shawn could better explain what part files are and what they do, but from my understanding part files are never scanned, just the final file.
Blocks: 443215
.part files just exist from the nsExternalHelperAppService as the temporary location for downloaded files.  AFAIK, we only scan the file when we move the .part file.
We had some problems with crashes in avg, but they apparently fixed this in a release. (Bug 412565)
Okay, so this sounds like it's unrelated to virus scanning -- the original file is created with 0 size, and it remains 0 size... it isn't deleted.  So basically we're just failing to move the part file over correctly for some reason.
(In reply to comment #16)
> Okay, so this sounds like it's unrelated to virus scanning -- the original file
> is created with 0 size, and it remains 0 size... it isn't deleted.  So
> basically we're just failing to move the part file over correctly for some
> reason.

Didn't you mention earlier that you were able to see the part file growing?
Taking this off the blocker list due to the low number of reports and inability to reproduce at this time. Please feel free to renominate if we get a better handle on it, and Jim, if you could noodle on it a bit, especially around the code where we go from .part -> final file, as Vlad's reports are consistently pegging that as the point of failure.
Flags: blocking1.9.1+ → blocking1.9.1-
(In reply to comment #17)
> (In reply to comment #16)
> > Okay, so this sounds like it's unrelated to virus scanning -- the original file
> > is created with 0 size, and it remains 0 size... it isn't deleted.  So
> > basically we're just failing to move the part file over correctly for some
> > reason.
> 
> Didn't you mention earlier that you were able to see the part file growing?

Correct, the full download happens to the .part file, then the .part file goes away.
(In reply to comment #19)
> (In reply to comment #17)
> > (In reply to comment #16)
> > > Okay, so this sounds like it's unrelated to virus scanning -- the original file
> > > is created with 0 size, and it remains 0 size... it isn't deleted.  So
> > > basically we're just failing to move the part file over correctly for some
> > > reason.
> > 
> > Didn't you mention earlier that you were able to see the part file growing?
> 
> Correct, the full download happens to the .part file, then the .part file goes
> away.

Did you mean the final file goes away? From looking back over this my guess as to what you're seeing is:

1) download starts
2) part file created
3) part file grows
4) download finishes
5) part file goes to zero bytes, no final file

Does that fit?
Assignee: nobody → jmathies
Hrm, what I'm seeing is (or was):

1) download starts
2) both part file and real filename file get created (to hold the name, I guess?)
3) part file grows
4) download finishes
5) part file goes away, leaving original 0-byte final file
(In reply to comment #21)
> Hrm, what I'm seeing is (or was):
> 
> 1) download starts
> 2) both part file and real filename file get created (to hold the name, I
> guess?)
> 3) part file grows
> 4) download finishes
> 5) part file goes away, leaving original 0-byte final file

Ok, that's weird. :) I'll take a look, see what I can find. I think next week will mostly be stuff like this as it's starting to pile up in my queue.
(In reply to comment #21)
> Hrm, what I'm seeing is (or was):
> 
> 1) download starts
> 2) both part file and real filename file get created (to hold the name, I
> guess?)
> 3) part file grows
> 4) download finishes
> 5) part file goes away, leaving original 0-byte final file

From bug 224692, the steps we follow are :

1) Placeholder file ($P) is created in %TEMP%
2) $P is passed to the download code (which is treated internally as a plugin)
3) Final file ($F) is opened as a 0-byte file at the download folder
4) $P is moved to the download folder and renamed with a ".part" extension
5) Permissions are set on $P (after bug fix)
6) When download completes, $F is deleted
7) $P is renamed to remove the ".part"

So this sounds more like something internal and unrelated to virus scanning. The fact that the part file goes away and the final zero byte file stays is rather strange, as the zero byte file should get deleted before anything happens to the part. 

Shawn any ideas?
(In reply to comment #23)
> Shawn any ideas?
Not so much.  Maybe I'm missing something that bz will catch though :/
No idea; I've never really seen our .part code...

Could there be a permissions issue where we fail to move but succeed in deleting or some such?  I've had Vista give me very very random security errors when trying to copy/move files...
The relevant code is right here - 

http://mxr.mozilla.org/mozilla-central/source/uriloader/exthandler/nsExternalHelperAppService.cpp#2022

2038     fileToUse->Equals(mTempFile, &equalToTempFile);
2039     fileToUse->Exists(&filetoUseAlreadyExists);
2040     if (filetoUseAlreadyExists && !equalToTempFile)
2041       fileToUse->Remove(PR_FALSE);

My guess is both these flags get set as expected (equalToTempFile has to be false because Vlad is seeing both files, and filetoUseAlreadyExists has to be true because the destination is present.)

So my guess is the Remove call is failing, in which case the subsequent MoveTo call would fail and I'm guessing on the SendStatusChange of the error, the download manager deletes the temp file. That would result in what Vlad is seeing, with the zero byte file left and the temp file gone.

Why Remove is failing I have no idea. We could try and track this by adding some code to check the result of Remove, which should probably be in there anyway.
No longer blocks: 443215
Assignee: jmathies → nobody
Priority: P2 → P3
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.