Closed Bug 96388 Opened 22 years ago Closed 12 years ago
IFile::Remove() will fail to delete an open file on win32
nsIFile::Remove() will fail to delete an open file on win32 This came up in the course of bug 68045 where, if the fastload file is corrupt, it is supposed to be deleted with nsIFile::Remove(). That's what happens on Linux, but the file is not deleted on win2k. Looking at the docs for win32's |int remove(const char *path)|, it says: "This function fails and returns -1 if the file is open. ... All handles to a file must be closed before it can be deleted." http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore98/HTML/_crt_remove.2c_._wremove.asp I'm not sure what the intended semantics of Remove() are, but it seems like it should be the same for win32, linux and mac.
Er, to be clear nsLocalFileWin.cpp uses |remove|, while nsLocalFileUnix.cpp uses |unlink|.
Hmm, it seems that nsLocalFileMac.cpp is similar to win32, in that |HDelete| is used on the Mac, and the file must be closed before calling that. It's only *nix, using |unlink| that will expunge an open file (although unlink defers deletion until noone has a handle; not sure how this quite works in the *nix case).
Yeah, Unix does open-unlink and other OSes do not. The question is, should nsIFile::Remove do as Unix does? If there is a way to detect the open remove case, rename the file (does Windows allow this, even? Within the same drive, or same directory?), and remove it later when the handle is closed, then we should do that. But I'm pessimistic. I had better prepare a workaround in FastLoad code.... /be
references to how windows handles installer code ... If fastload has the file open and tries to nuke it, then that's a bug in fastload :-) $what_brendan_said =~ s/workaround/fix/
what would you propose that nsIFile do if a file can not be deleted? Should we do what XPInstall does and manage a list of files which could not be deleted, and try again when xpcom restarts??
Doug, all I think this bug should mean is to doc-comment the open file failure case in nsIFile.idl. Right? /be
Pete, could you add this to your changes to the interface?
Assignee: dougt → petejc
No, this is a great bug and i hate the fact that i can't remove a "locked" file on win and mac. I needed this to implement an uninstall for Chameleon, which worked great on unix but left the jar files on win and mac. I think if you remove, then that means kill it. Soug, are you just saying i comment that remove will not delete open files an win and mac? I would prefer it if the files were actually deleted. --pete
Status: NEW → ASSIGNED
It's not the most well known fact, but there was actually a little extension (INIT if you go far enough back) with some Apple Installers called something like "Uninstall extension". It's job was to take a list of files that were opened by the installer, and delete them upon reboot. What do other vendors do?
Pete: the bug interface does not illustrate it clearly enough, but code contributors (owners) generally assign the "priority" field. At-large contributors give input by setting the "severity" field. Can you update the severity field to the level you think is appropriate?
normal is fine. I am mass changing pre 1.0 nsIFile bugs assigned to me to P1 so I can knock them off one at a time. --pete
Target Milestone: --- → mozilla0.9.8
jrgm: can you update, is this still happening in 1.4x? -> whiteboxqa If there is an end user set of steps, I can test this
QA Contact: benc → ashishbhatt
Well, this is mainly just a bug to note the nsIFile::Remove has slightly different semantics on unix-based-OS vs. win32. (See the first 6 or so comments). I don't know what petejc's intentions are for this bug, but as at comment #6 this was just about changing the doc-comment in the IDL to make it clear that "Remove" can not remove an open file on Win32.
Hmmm, it looks like MS Windows has an "unlink" function. See http://msdn.microsoft.com/library/default.asp?url=/library/en-us/vccore98/HTML/_crt__unlink.2c_._wunlink.asp for more info. Actually, this seems to be a Visual Studio function, so I'd hate to think what this means for GCC on Win32.
The MSVC unlink() and remove() CRT functions both end up calling the Win32 DeleteFile function, so there's no real difference (unlink() is literally just a wrapper for remove()). Interestingly, the documentation for DeleteFile says both: * The DeleteFile function fails if an application attempts to delete a file that is open for normal I/O or as a memory-mapped file. and * The DeleteFile function marks a file for deletion on close. Therefore, the file deletion does not occur until the last handle to the file is closed. Subsequent calls to CreateFile to open the file fail with ERROR_ACCESS_DENIED. http://msdn.microsoft.com/library/default.asp?url=/library/en-us/fileio/base/deletefile.asp I don't quite see how both can be correct.
Yea, this needs to be addressed. I haven't looked at this bug for a while but the inconsistencies between platforms needs to be fixed. Here is the implementation in question: http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileWin.cpp#1361 The implementation is using remove(). Has anyone tried removing a file on win and then shutting down Mozilla to see if it is deleted once the file handle is released?
While I haven't tried the procedure in comment #16, I can say that the documentation, although vague, is correct. From my experience with patching Windows (with MS supplied patches), you often need to reboot windows for the sole purpose of deleting an inuse file and replacing it with a new version of that file. So, DeleteFile will return an error if the file is in use. This error will need to be caught and a process designed to ensure whatever is using the file is closed so that the file can be deleted and/or replaced.
You can't (*** see below) remove a file that is exclusively opened on Windows. Other programs that open a file first, can specify no FILE_SHARE_DELETE flag. You can ask that Windows delete a file on reboot if it is exclusively opened by using MoveFileEx with MOVEFILE_DELAY_UNTIL_REBOOT. I don't think we should do that though. I nominate that this bug be resolved as invalid. The user of nsIFile should not assume that delete will work on every platform. They should check for an error of NS_ERROR_FILE_IS_LOCKED and behave appropriately depending on the user's situation. *** There is an actual way to do the delete at the driver level, but we would definitely not do it. I.e. you could use a mini filter driver, or a file system driver to do the work.
Agree wholeheartedly with comment #18. Also, Linux has mount -o mand though I haven't verified whether it will also prevent deletion as does Windows.
> Also, Linux has mount -o mand though I haven't verified whether it will also prevent deletion as does Windows I haven't tried either but I assume it will give errors. Locking in unix/linux is by default advisory, meaning other processes don't need to follow the locking rules that are set by fcntl, lockf. But as Robert mentioned, you can mount as mandatory instead of advisory and then deletions will probably not work even on unix/linux. Resolving as invalid for now.
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → INVALID
You need to log in before you can comment on or make changes to this bug.