views:

219

answers:

7

Please:
There are no leaked handles [1]. The file is not open by anything in our software (it could be held open by an AntiVirus or a system indexing service, etc.) The code below can be compiled and run on your computer at home/office: you can see instantly that this loop fails and by itself NEVER attempts the Delete until after ALL handles are successfully closed, and it NEVER attempts the Create until after the file is DeleteFile'd.

The Problem:
Our software is in large part an interpreter engine for a proprietary scripting language. That scripting language has the ability to create a file, process it, and then delete the file. These are all separate operations, and no file handles are kept open in between these operations. (i.e. during the file create a handle is created, used for writing, then closed. During the file processing portion, a separate file handle opens the file, reads from it, and is closed at EOF. and Finally, delete uses ::DeleteFile which only has use of a filename, not a file handle at all).

Recently we've come to realize that a particular macro (script) fails sometimes to be able to create the file at some random subsequent time (i.e. it succeeds during the first hundred iterations of "create, process, delete", but when it comes back to creating it a hundred and first time, Windows replies "Access Denied").

Looking deeper into the issue, I have written a very simple program that loops over something like this:

while (true) {
  HANDLE hFile = CreateFileA(pszFilename, FILE_ALL_ACCESS, FILE_SHARE_READ, NULL, CREATE_NEW, FILE_ATTRIBUTE_NORMAL, NULL);
  if (hFile == INVALID_HANDLE_VALUE)
      return OpenFailed;
  const DWORD dwWrite = strlen(pszFilename);
  DWORD dwWritten;
  if (!WriteFile(hFile, pszFilename, dwWrite, &dwWritten, NULL) || dwWritten != dwWrite)
      return WriteFailed;
  if (!CloseHandle(hFile))
      return CloseFailed;
  if (!DeleteFileA(pszFilename))
      return DeleteFailed;
}

As you can see, this is direct to the Win32 API, and pretty darn simple. I create a file, write to it, close the handle, delete it, rinse, repeat...

But somewhere along the line, I'll get an Access Denied (5) error during the CreateFile() call. Looking at sysinternal's ProcessMonitor, I can see that the underlying issue is that there is a pending delete on the file while I'm trying to create it again.

Questions:
* Is there a way to wait for the delete to complete?
* Is there a way to detect that a file is pending deletion?

We have tried the first option, by simply WaitForSingleObject() on the HFILE. But the HFILE is always closed before the WaitForSingleObject executes, and so WaitForSingleObject always returns WAIT_FAILED. Clearly, trying to wait for the closed handle doesn't work.

I could wait on a change notification for the folder that the file exists in. However, that seems like an extremely overhead-intensive kludge to what is a problem only occasionally (to whit: in my tests on my Win7 x64 E6600 PC it typically fails on iteration 12000+ -- on other machines, it can happen as soon as iteration 7 or 15 or 56 or never).

I have been unable to discern any CreateFile() arguments that would explicitly allow for this ether. No matter what arguments CreateFile has, it really is not okay with opening a file for any access when the file is pending deletion. And since I can see this behavior on both an XP box and on an x64 Win7 box, I am quite certain that this is core NTFS behavior "as intended" by Microsoft. So I need a solution that allows the OS to complete the delete before I attempt to proceed, preferably w/o tying up CPU cycles needlessly, and without the extreme overhead of watching the folder that this file is in (if possible).

Thanks for taking the time to read this and post a response. Clarifying Questions welcome!

[1] Yes, this loop returns on a failure to write or a failure to close which leaks, but since this is a simple console test app, the app itself exits, and Windows guarantees that all handles are closed by the OS when a process completes. So no leaks exist here.

EDIT: Solution (@sbi)

bool DeleteFileNowA(const char * pszFilename)
{
    // determine the path in which to store the temp filename
    char szPath[MAX_PATH];
    strcpy(szPath, pszFilename);
    PathRemoveFileSpecA(szPath);

    // generate a guaranteed to be unique temporary filename to house the pending delete
    char szTempName[MAX_PATH];
    if (!GetTempFileNameA(szPath, ".xX", 0, szTempName))
        return false;

    // move the real file to the dummy filename
    if (!MoveFileExA(pszFilename, szTempName, MOVEFILE_REPLACE_EXISTING))
        return false;

    // queue the deletion (the OS will delete it when all handles (ours or other processes) close)
    if (!DeleteFileA(szTempName))
        return false;

    return true;
}
+4  A: 
sbi
Cool idea. I'm not sure if it will work for our real-scenario (which is much more complex than my question), but it might! :)
Mordachai
@Mordachai: You might want to read my second edit. If Hans is right, this might not help. But you can always try it in your test program first, to see whether complicating your real app is actually worth trying.
sbi
Yeah, I think Hans is right too. :( So this just pushes the problem back one step, but solves nothing. Sigh.
Mordachai
+1. probably doesn't matter if Hans is right. You can rename open files. This is no problem because file handles are mostly independent of file names. The name only matters at the time you create a file handle from a file name. The hypothetical process with an open file handle won't be affected.
MSalters
Interesting idea. I could take all files that are to be deleted, rename them to a temp filename, then delete that, which will happen *whenever* w/o affecting the script writers. Hmmm... ;)
Mordachai
@MSalters: I didn't expect renaming to work for open files, but didn't rule it out either. ("But then you might, I don't know this.") If you are right and it works, this might indeed solve the problem.
sbi
I'm giving this a shot now. I'll let you know. BTW: how in the world did the "answer" switch to here? I was debating moving it, but then it seemed to move itself! huh?
Mordachai
@Mordachai: You must have switched it by clicking on the check mark next to my answer. Not that I'd mind it, but I think Hans' answer is the better one (his first up-vote was from me), as it correctly explains what happens, while I just made a shot in the dark.
sbi
Sorry about that! No idea how I managed that! d'oh!
Mordachai
Looking into this idea: but running into problems. Mainly that the GetTempFileName cannot give me a guaranteed to be unique filename unless it *creates* the file for me, which makes moving the old file to this name problematical (i.e. the new file has to be deleted before the old one can be moved!). So although I can think of a hack (prefix [deleting] to the target filename arbitrarily) it's perhaps not that easy to do in practice (i.e. I may end up having to do a form of uniqueness generation myself)
Mordachai
This does seem to work! Basically, I am creating a temporary file (actually creating it via GetTempFileName), and then using MoveFileEx to overwrite the unique temp file with the real one, then issuing the DeleteFile on the temp filename. So far, I can't get that to go wrong. I expect there's a performance hit - it cannot be as fast to do all of this extra overhead, but to insulate our scipters from the subtleties of the multitasking filesystem, it's probably worth it.
Mordachai
@Mordachai: `GetTempFileName()` _has_ to create the file, otherwise a equally named file could be created by someone else between the time you get the unique file name and when you actually create the file. Anyway, glad you got it working.
sbi
Yeah, I get why it works the way it does. I was just worried that this would mean that the move operation would fail while someone had a handle open to the original file. Apparently, this is not the case (though I cannot be absolutely certain: 1.5 million iterations says it's working, but...)
Mordachai
A: 
Pineapple
I think _Windows replies "Access Denied"_ is what `GetLastError()` returns.
sbi
The GetLastError() is always "(5) Access Denied" -- which is a pretty lame, limited response from the OS. If it was "(91929) Pending Delete" then that would be GREAT! :)
Mordachai
Thanks for the ideas, Pineapple. Unfortunately, the scripting engine can't know when the script is going to delete the target file (if ever), so it can't mark the file for deletion prior to the script asking for that to happen. And once that does happen, we're at the mercy of the OS to complete the operation. If the script tries to do another command which generates that file again and the previous hasn't completed, then they'll get this error. It's not really under my control what the script writer does (and on the face of it, that script is reasonable)
Mordachai
+6  A: 

There are other processes in Windows that want a piece of that file. The search indexer is an obvious candidate. Or a virus scanner. They'll open the file for full sharing, including FILE_SHARE_DELETE, so that other processes aren't heavily affected by them opening the file.

That usually works out well, unless you create/write/delete at a high rate. The delete will succeed but the file cannot disappear from the file system until the last handle to it got closed. The handle held by, say, the search indexer. Any program that tries to open that pending-delete file will be slapped by error 5.

This is otherwise a generic problem on a multitasking operating system, you cannot know what other process might want to mess with your files. Your usage pattern seems unusual, review that first. A workaround would be to catch the error, sleep and try again.

Hans Passant
Yes, it might be sensible to avoid this pattern entirely! lol - unfortunately, I'm in a role akin to a language developer who has programmers who use said language. I can tell them that some patterns are better avoided, but in the end, they write their software (scripts) and I have to do my best to deal with their limited capacity to pay attention to "best practices".
Mordachai
Well, trying to hide *real* operating system behavior from those script programmers would be a drastic mistake. Improve your api, provide a way for those lowly souls to handle the problem by themselves. Error code, exception, whatever. Avoid the God attitude, you'll just create something that everybody think will suck. Because they've got that problem with their script that doesn't work without a diagnostic and God is not answering their email.
Hans Passant
Mordachai
A: 

I think this is just by poor design in the file system. I have seen same problem when I worked with communication ports, opening/closing them.

Unfortunately I think the most simple solution would be to just retry to create the file a number of times if you get an INVALID_HANDLE_VALUE. GetLastError() might also give you a better way of detecting this particular INVALID_HANDLE_VALUE.

I would have preferred overlapped I/O, but there CloseHandle() and DeleteFile() don't handle overlapped operations :(

Max Kielland
+2  A: 

Silly suggestion - since it fails so infrequently, how about simply waiting some milliseconds on failure and trying again? Or, if latency is important, switch to another file name, leaving the old file to be deleted later.

Arkadiy
A: 

Since you're creating a new file, processing it, then deleting it, it sounds like you don't really care about what the file name is. If that's truly the case, you should consider always creating a temporary file. That way, each time through the process, you don't have to care that the file didn't yet get deleted.

Jacob
-1 his question specifies that he doesn't get to control the content of the scripts causing this
Elemental
@Elemental, exactly. He said the *content* of the scripts, not the *location* of the scripts, so the downvote is invalid.
Jacob
@Jocab - unfortunately the scripts themselves contain the commands including the delete command which uses the file name directly (literally) and he can't change the scripts to use temp file names instead.
Elemental
It is a nice idea, Jacob, but indeed our script writers are in control of the name of the file. And although this particular scenario has arisen, in general, it does not. It is impossible at the language-level (scripting interpreter) to assume the nature of the file, and enforce any particular rules around it (it's just an output filename in one context, an input filename in another, and a target of a delete command in yet another, all unrelated as far as the interpreter knows).
Mordachai
A: 
  • Is there a way to detect that a file is pending deletion?

Use GetFileInformationByHandleEx function with FILE_STANDARD_INFO structure.

But the function can't solve your problem. @sbi's solution either.

Benjamin
Erm, I don't think that this is possible [in our situation]. Quite frankly, it's not possible to get a handle for a file which is pending deletion. So by the time the CreateFile() executes, the file is pending deletion, but it is impossible to obtain a handle on it to check. :(
Mordachai
Onces a file pend as deletion, you can't open the file again. It is only possible when you got another handle before the file deleted.
Benjamin
Right - so any useful solution would need to answer the question "why is access denied" when I call CreateFile()? If there is a way to ask the OS "is the file pending delete" at that point in time, then this would be useful. Or if there was a way to get CreateFile() to be more forthcoming with details instead of a blank "denied" then that would be more useful.
Mordachai
Mordachai