Closed Bug 63105 Opened 24 years ago Closed 23 years ago

Temporary files used by helper apps are not removed.

Categories

(Core Graveyard :: File Handling, defect, P2)

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.3

People

(Reporter: kcha-ns-yka, Assigned: mscott)

References

()

Details

Attachments

(2 files)

Win32 build 2000121520 (Win98SE), RealPlayer 8 plug-in

1. Go to the above url
2. Click on any of the RP links
3. The Downloading dialog opens. RealPlayer displays as the app in the Using box
4. Select the "Open Using" radio button
5. A file is created in c:\windows\temp containing the url of the sound file
6. RP starts up. The temp file name displays in its location box
7. The temp file is not removed after closing RP

Since I use RP for MP3 files as well, this also happens when I pick the MPEG
files on this site, except that the entire MP3 file is downloaded to temp before
RP starts. The expected results are that any file created in c:\windows\temp is
removed when another sound file is selected, or when the app is closed.

c:\windows\temp is getting very cluttered.
well, I see no different behaviour in 4.7 as well. So ,what to do...
Win98, build 2000121804: Confirmed and marked NEW. The most reasonable time to
delete such temporary files, however, would probably be when Mozilla is exited?
Status: UNCONFIRMED → NEW
Ever confirmed: true
av, is this something that we need to fix, or Real needs to fix?  Or do we know?
Does it also happen on NT4.0? I doubt this is us. Further investigation needed.
CC:ing nhart@real.com.

If Real creates the temp files it should remove them, otherwise, we should.
Keywords: realplayer
Moving to m1.0 and reassigning to peterl.
Assignee: av → peterl
Target Milestone: --- → mozilla1.0
Does this still happen? adding qawanted

Nick from Real: can you provide some input?

Keywords: qawanted
As of build 2001031604, RP downloads the files to the c:\windows\temp directory
and leaves them there if the mime types have not been manually added to helper apps.

I added the mime types as per the instructions on the RP website, but now RP
doesn't work at all. I'm having other PC/app problems at the moment, so the RP
bustage could be caused by something else altogether.

I can't confirm what RP does with mime types added until I get my general PC
problems resolved.
K Chayka: Are you sure that RP acts as a plugin, not as an "ordinary" helper
application? Because your step-by-step sure looks the same for helpers. RP shows
up in "about:plugins"?
Would you fix for Acrobat work for this too?
Probably not. That fix is for PostUrl from file. Looks like this is not the case 
here.
You are right, this is not plugins. I think this is something with the way 
the URILoader handles helper apps. How did 4.x handle this?

Over to mscott who may know more about this.
Assignee: peterl → mscott
Summary: RealPlayer creates temp files that are not removed → Temporary files used by helper apps are not removed.
Target Milestone: mozilla1.0 → ---
*** Bug 76092 has been marked as a duplicate of this bug. ***
This is a problem on all operating systems.
Blocks: 78106
OS: Windows 98 → All
Hardware: PC → All
Copying from bug 76092:

A potential solution for Unix helper apps would be spawning sh -c "helper_app
tmpfile; rm -f tmpfile". This would also solve bug #56662 and probably bug
#58667 and also allow people to use shell syntax in the helper app
specification. If I remember correctly, this is exactly what 4.x did

Keywords: 4xp
Doug/Arun: is this bug impacted after our metting about https and plugins?
*** Bug 86242 has been marked as a duplicate of this bug. ***
> How did 4.x handle this?

The 4.x WinFE (not sure about the MacFE or XFE) kept a list of temporary files
in the Registry which were deleted when the app exited or started up (in case of
crash). There didn't seem to be a way to know when we were done with the temp file.

Nominating for mozilla0.9.2 since these temp files seem to just leak forever,
filling up the temp directory :-(
Severity: normal → major
Keywords: mozilla0.9.2
Peter: this bug does not impacted https and plugins.  https cached files are 
maintained seperately.
Attached patch the fixSplinter Review
I just attached the fix. The external helper app service now supports the
ability to delete temporary files on exit. There's a public API which you can
use to add a nsIFile you want deleted on exit. We end up creating a pref branch
called browser.temporaryFiles.temp-0, temp-1, etc. The value of each pref entry
is a nsILocalFile. When we receive a shut down notification, we enumerate over
this pref branch and delete each temp file.

One problems with this patch: 
nsIPrefBranch::DeleteBranch isn't working currently. It doesn't really delete
the branch! I have to follow up with the pref module owners to fix this. I also
ran into another pref bug with nsIPrefBranch::GetComplexValue which I hacked
around but that needs fixed too. 

Adding keyword mumbo jumbo.
Status: NEW → ASSIGNED
Keywords: nsbeta1, nsBranch
Priority: -- → P2
Target Milestone: --- → mozilla0.9.3
Depends on: 88500
I made a one line change to the above patch to work around the DeleteBranch bug.
I now explicitly save out the prefs after deleting the branch by calling:

prefService->SavePrefFile(nsnull)

I have to do this because I'm deleting the branch inside a shut down observer
notification we are getting from XPCOM. There seems to be a bug in how prefs
shuts down such that they save the prefs file for the last time BEFORE all
observers are notified that we are going to shutdown. As a result any changes a
shutdown observer makes to pref values has no effect. 

For those that care, the extra one line comes right after the call to DeleteBranch:
  // now delete the branch completely since we've tried to delete all the
relevant files...
  prefBranch->DeleteBranch(nsnull);

  // the prefs service may have already saved prefs on exit. so we need to
manually resave the prefs.
  prefService->SavePrefFile(nsnull);
  return NS_OK;
r=sspitzer

for the benefit of the super reviewer, here are some addtional questions I had 
and the answers mscott gave me:

Q: my only worry is are prefs still ok during shutdown?
A: they are ok. we haven't actually shut down yet this is the notifiation to 
observers telling folks we are going to be shutting down

Q: what does the pref service do on that notification?  what happens if prefs 
gets notified first, before you?
A: the pref service does nothing, it doesn't even pay attention to the shut 
down observer notification
sorry for the delay, I was trying to track you down on AIM/etc
Anyway, my big question about all of this is why you are going through the pref
service to store these? i.e. if you're not going to persist the values across
sessions, why use prefs at all? why not just store the nsIFile pointers in an
nsISupportsArray which is stored in memory and destroyed on exit? 
because I wanted a persistent means to manage this (like 4.x). If you crash
during a session, we'll never remember to clean up these temp files the next
time you run.  By storing the files in a pref, the next time you cleanly exit,
we'll clean everything up.
But what if you exit Mozilla while the helper app is still running? I believe
that the best thing to do (at least of Unix) is what 4.x did - to run the helper
app using 
sh "<helper app cmdline>; rm -f <tmpfile>"
See also my comment from 2001-06-17 02:30
My argument about storing the temporary file list in prefs for persistence
reasons is faulty. unless I explicitly saved the prefs every time I added a temp
file to the list, there's not guaruntee my pref changes would be there after a
crash. 

I simpliefied the code and the concerns of modifying prefs during the shutdown
process by just storing the temp files in an nsISupportsArray. This simplifies
the code and the risk. 
after Seth's code review, he noticed that I was calling
tempFile->Delete(PR_TRUE) when I should be passing PR_FALSE (for non recursive
delete). So changed the delete call to pass in false. 
sr=bienvenu, with the change that Seth suggested.
r=sspitzer from an aim message I got from him. 
this has been checked into the trunk. 
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
QA Contact: shrir → sairuh
sorry for the delay. this is fixed on winNT [2001.11.27.13-comm] and linux
[rh7.2, 2001.11.28.12-comm]. but i still run into this problem on mac --at least
on OS X. filed bug 112526 for that, and verifying this one.
Status: RESOLVED → VERIFIED
ooh, forgot to mention the verification tests i used:

linux: 
* tested with a .ram and .rm file [added RealPlayer to handle those types].
* RealPlayer successfully launched.
* temp files were saved to /tmp while n6.x was running, but when i quit both
RealPlayer and n6.x, the temp files were removed, as expected.

winNT:
* tested with the Windows nightly build on http://mozilla.org/ [WinZip was the
default app].
* WinZip successfully launched.
* the temp file was saved to C:\TEMP while n6.x was running, but when quit both
WinZip and n6.x., the temp file was removed --again, as expected.

Mac OS X: see bug 112526 for details. :)
Component: Plug-ins → File Handling
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: