Closed Bug 114778 Opened 23 years ago Closed 23 years ago

formpost temp files not cleaned up when browser exits

Categories

(Core :: Networking, defect, P2)

x86
Windows 98
defect

Tracking

()

VERIFIED FIXED
mozilla0.9.8

People

(Reporter: darin.moz, Assigned: badami)

Details

(Whiteboard: [ready-to-land])

Attachments

(4 files)

formpost temp files not cleaned up when browser exits... seems like the patch for bug 15320 didn't do the trick for Win9x systems.
-> 0.9.8
Severity: normal → major
Status: NEW → ASSIGNED
Priority: -- → P2
Target Milestone: --- → mozilla0.9.8
Attached patch v1.0 patch (deleted) — Splinter Review
this patch cleans up the "delete-on-close" impl... it's now an XP impl that first attempts to unlink the file after opening a file descriptor. if that fails, the file input stream remembers the nsIFile passed via nsIFileInputStream::Init, and calls Remove on the nsIFile after closing the file descriptor. i think it was wrong to assume that, for example, linux systems would always operate on POSIX compatible file systems. afterall, it is possible to mount a Fat filesystem under linux... not that i can think of any reason why one would use one as their temp directory =)
Keywords: patch
Comment on attachment 61533 [details] [diff] [review] v1.0 patch a. why even bother with trying to delete the file prior to your done with it? b. what happens if you the remove file fails?
Attachment #61533 - Flags: review+
a) because if the file can be removed in the Init() method, then i don't have to hold onto the nsLocalFile object for the lifetime of the nsFileInputStream. for multipart formpost data, this would correspond to the lifetime of a browser window. b) if Remove fails, then we'll leave/leak a file in the users temp directory :(
Comment on attachment 61533 [details] [diff] [review] v1.0 patch sr=mscott
Attachment #61533 - Flags: superreview+
The patch is already marked "has-review", but I didn't see an r= anywhere, so here's mine: r=gordon.
Whiteboard: [ready-to-land]
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
build 2001122003, win98se Ran some html files through http://validator.w3.org/file-upload.html formpost files are still left in the temp dir after the browser closes. :-( In case it matters, I override the default c:\windows\temp in my autoexec.bat file: SET temp=e:\tmpdir SET tmp=e:\tmpdir The formpost files get created in e:\tmpdir OK, but don't get removed.
K Chayka: could you try to reproduce the problem using a more recent build? i suspect the 2001122003 build might not of picked up this fix.... though i'm not really certain how much latency there is between code changes and nightly builds.
build 2001122103 - same thing 1. Start mozilla 2. Run one html file through the W3C validator upload service - the formpost file gets created in the temp dir 3. Close browser window - mozilla shuts down and the formpost file sits there
ok... guess i need to find myself a win98 system to test this on :-( reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
vinay: do you have access to a win98 build system (w/ a debug build) where you can test this bug?
Will find out from the sysadmin here and let u know on Monday.
I've seen this bug for months on Windows NT4 as well. Still happening in 2002010803-trunk.
david: any chance you can supply a testcase or example of this bug somewhere on the internet? my own tests with WINNT all passed. thx!
Darin I'am able to reproduce this on W2K using the url mentioned in the bug and uploading a html page to that url. ie http://validator.w3.org/file-upload.html One bug i do see is that rv always comes back as NS_OK from mozilla/xpcom/io/nsLocalFileWin.cpp. This is because the return from remove is being ignored. So we need this patch. Index: nsLocalFileWin.cpp =================================================================== RCS file: /cvsroot/mozilla/xpcom/io/nsLocalFileWin.cpp,v retrieving revision 1.71 diff -u -r1.71 nsLocalFileWin.cpp --- nsLocalFileWin.cpp 2002/01/09 20:03:45 1.71 +++ nsLocalFileWin.cpp 2002/01/14 11:41:36 @@ -1223,15 +1223,15 @@ iterator->HasMoreElements(&more); } } - rmdir(filePath); // todo: save return value? + rv = rmdir(filePath); // todo: save return value? } else { - remove(filePath); // todo: save return value? + rv = remove(filePath); // todo: save return value? } MakeDirty(); - return NS_OK; + return rv; } However, even after this patch the file does not get removed. We are now remembering mFileToDelete. However, looks like the Close is never getting called. Looking into this.
I'am wondering if the Close is not getting called on shutdown becuase th io stuff is an XPCOM service ???
Er, testcase? I will attach a formpost file that this build (2002010803) left behind when I posted a comment at http://www.rocknerd.org/rocknerd/1010725277/index_html .
This is also present on WinXP as well, but it seems only to manifest it self on Win32 funning on FAT32, not NTFS. Which is why testing on NT passes (no FAT32 support (and WinInternals FAT32 for NT driver doesn't count)). From a cursory examination of the problem (not the code, yet), I would say that the FAT32 file system services is not deleting the file because: A) Moz isn't properly asking for the right API call, and/or B) FAT32 FS services suck, and need to be led by the hand into deleting the exact file in a very basic manner. Now, the FAT32 support for @k and XP are based on the IFS services component for Windows 98/98SE. 98/98SE introduced the WDM system, which the file system services plugged into as well. this made it more portable to 2k (if you examine the FS services files you'll see references to NTFS, bearing out the shared codebase). This is why the bug manifests itself on both 9x/ME and 2k/XP; the FSS code is the shared on both platforms. I'll have time in the next day or two and I'll look into it too. The more eyes the better. :)
Grey: hrmmm. I'm seeing this on NT4, and both partitions on this box are NTFS, not FAT16 or anything else (no FAT32 support on this thing). I suppose I should get a more recent nightly and keep trying. Is there anything in particular I can do to test for this one? Anyone got a particular debugging build they'd like me to try? (I have no compiler here.)
sounds like vinay is on top of this one. -> badami
Assignee: darin → badami
Status: REOPENED → NEW
I was making that assumption based on hearing that it does not manifest itself on NTFS disks. If it does, then it's not related to the Win32 FAT32 FSS code.
Attached patch To cause file removal (deleted) — Splinter Review
1. mozilla/xpcom/io/nsLocalFileWin.cpp Return proper return value as opposed to NS_OK always. 2. netwerk/base/src/nsBufferedStreams.cpp Should release reference on mstream when buffered stream is being closed. 3.mozilla/netwerk/base/src/nsFileStreams.h ~nsFileInputStream() should call it's close.
Attached patch adding in fix for OS2 also (deleted) — Splinter Review
same problem in sLocalFileOS2.cpp as in the windows version. The OS2 version is not tested and not sure if it compiles either.
Attachment #66280 - Flags: superreview+
Comment on attachment 66280 [details] [diff] [review] adding in fix for OS2 also sr=darin (nice work nailing this one!)
rpotts Can i get a r= on this please ?
Comment on attachment 66280 [details] [diff] [review] adding in fix for OS2 also r=rpotts@netscape.com looks good!
Attachment #66280 - Flags: review+
fixed-on-trunk
Status: NEW → RESOLVED
Closed: 23 years ago23 years ago
Resolution: --- → FIXED
win32 build 2002012608, win98se Looks like it is indeed fixed. Thank you!
vrfy
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: