11

I have a single threaded program (C++, Win32, NTFS) which first creates a quite long temporary file, closes it, opens for read, reads, closes again and tries to delete using DeleteFile().

Usually it goes smoothly, but sometimes DeleteFile() fails, and GetLastError() returns ERROR_ACCESS_DENIED. File is not read-only for sure. It happens on files on any size, but the probability grows with the file size.

Any ideas what may be locking the file? I tried WinInternals tools to check and found nothing suspicious.

Brian Tompsett - 汤莱恩
  • 5,753
  • 72
  • 57
  • 129
Dmitry Shkolnik
  • 323
  • 1
  • 3
  • 12
  • Are you sure you are closing the file properly before trying to delete it? Did you missed any handles? – RageZ Nov 18 '09 at 02:21
  • As I said, I even checked that with WinInternals tools. All opens are paired with closes, but the deletion fails. And adding sleeping for 1 sec fixes the problem. – Dmitry Shkolnik Nov 18 '09 at 02:25
  • It might be windows being buggy but I am kind of doubtful on that. if adding the `sleep` make it work should be fine ^^ – RageZ Nov 18 '09 at 02:27
  • 3
    As a rule, if adding a sleep() call somewhere makes a problem go away, you need to get rid of the sleep() call and fix the problem properly. It will be back. I've never seen exceptions to this rule. – Ori Pessach Nov 18 '09 at 03:30
  • This does not provide an answer to the question. To critique or request clarification from an author, leave a comment below their post. – Fred Nov 13 '12 at 01:05

7 Answers7

12

Windows is notorious for this issue. SQLite handles the problem by retrying the delete operation every 100 milliseconds up to a maximum number.

I believe if you are sure that you have no open handles, doing this in your implementation will save you some headaches when things like antivirus software open the file.

For reference, the comment from SQLite source:

/*                                                                     
** Delete the named file.                                              
**                                                                     
** Note that windows does not allow a file to be deleted if some other
** process has it open.  Sometimes a virus scanner or indexing program
** will open a journal file shortly after it is created in order to do
** whatever it does.  While this other process is holding the          
** file open, we will be unable to delete it.  To work around this     
** problem, we delay 100 milliseconds and try to delete again.  Up     
** to MX_DELETION_ATTEMPTs deletion attempts are run before giving     
** up and returning an error.                                          
*/
Ajay
  • 18,086
  • 12
  • 59
  • 105
Snazzer
  • 7,704
  • 5
  • 27
  • 25
10

Just a wild guess - do you have any anti-virus software installed? Have you tried disabling any real-time protection features in it, if you do?

Ori Pessach
  • 6,777
  • 6
  • 36
  • 51
5

I believe this is covered in Windows Internals. The short story is that even though you've called CloseHandle on the file handle, the kernel may still have outstanding references that take a few milliseconds to close.

A more reliable way to delete the file when you're done is to use the FILE_FLAG_DELETE_ON_CLOSE flag when opening the last handle. This works even better if you can avoid closing the file between reads/writes.

#include <windows.h>
#include <stdio.h>

int wmain(int argc, wchar_t** argv)
{
    LPCWSTR fileName = L"c:\\temp\\test1234.bin";

    HANDLE h1 = CreateFileW(
        fileName,
        GENERIC_WRITE,
        // make sure the next call to CreateFile can succeed if this handle hasn't been closed yet
        FILE_SHARE_READ | FILE_SHARE_DELETE,
        NULL,
        CREATE_ALWAYS,
        FILE_FLAG_SEQUENTIAL_SCAN | FILE_ATTRIBUTE_TEMPORARY,
        NULL);
    if (h1 == INVALID_HANDLE_VALUE)
    {
        fprintf(stderr, "h1 failed: 0x%x\n", GetLastError());
        return GetLastError();
    }

    HANDLE h2 = CreateFileW(
        fileName,
        GENERIC_READ,
        // FILE_SHARE_WRITE is required in case h1 with GENERIC_WRITE access is still open
        FILE_SHARE_WRITE | FILE_SHARE_READ | FILE_SHARE_DELETE,
        NULL,
        OPEN_EXISTING,
        // tell the OS to delete the file as soon as it is closed, no DeleteFile call needed
        FILE_FLAG_DELETE_ON_CLOSE | FILE_FLAG_SEQUENTIAL_SCAN | FILE_ATTRIBUTE_TEMPORARY,
        NULL);
    if (h2 == INVALID_HANDLE_VALUE)
    {
        fprintf(stderr, "h2 failed: 0x%x\n", GetLastError());
        return GetLastError();
    }

    return 0;
}
Nathan Howell
  • 4,627
  • 1
  • 22
  • 30
  • Won't this fail if another process has opened the same file, given the following description from the documentation? "If there are existing open handles to a file, the call fails unless they were all opened with the FILE_SHARE_DELETE share mode." – Ori Pessach Nov 18 '09 at 17:39
  • Yes, that's why I recommended not closing the file handle between the writes and reads. Create the first handle with FILE_FLAG_DELETE_ON_CLOSE and then use ReOpenFile or DuplicateHandle if you really need a file handle that doesn't have write access. – Nathan Howell Nov 18 '09 at 18:06
  • Maybe I'm slow today, but won't this still be a problem if somebody sneaky opens the file before the last call to CreateFile? the last call will still fail. – Ori Pessach Nov 18 '09 at 19:20
  • You want to use FILE_FLAG_DELETE_ON_CLOSE on a *single* call to CreateFile, then there is no chance of the file leaking. Otherwise all bets are off. If you need a file handle with different flags or rights then you should use ReOpenFile. – Nathan Howell Nov 18 '09 at 23:23
  • Gotcha. That's not what the question is about, but there's a good argument for structuring the code this way, it would seem. – Ori Pessach Nov 19 '09 at 00:18
4

Add a MessageBox() call before you invoke DeleteFile(), When it shows up, run the sysinternals tool Process Explorer. Search for an open handle to the file. In all probability you have not closed all handles to the file...

rep_movsd
  • 6,675
  • 4
  • 30
  • 34
  • 1
    That's what I had started from. No handles. So then I logged all access to the file, and nothing special. – Dmitry Shkolnik Nov 18 '09 at 02:30
  • It sounds like a race condition (possibly on the order of milliseconds), so unless you freeze everything, you may not be able to reproduce the bug this way. (But trying certainly helps narrow down possibilities.) –  Nov 18 '09 at 02:34
1

Maybe the changes are still cached and haven't been saved yet?

You can check this by adding a WaitForSingleObject on the file handle to be sure.

Amirshk
  • 8,170
  • 2
  • 35
  • 64
1

You may have a race condition. 1. Operating system is requested to write data. 2. Operating system is requested to close file. This prompts final buffer flushing. The file will not be closed until the buffer flushing is done. Meanwhile The OS will return control to the program while working on buffer flushing. 3. Operating system is requested to delete the file. If the flushing is not done yet then the file will still be open and the request rejected.

roy30103
  • 21
  • 3
-3
#include <iostream>
#include <windows.h>

int main(int argc, const char * argv[])
{
    // Get a pointer to the file name/path
    const char * pFileToDelete = "h:\\myfile.txt";
    bool RemoveDirectory("h:\\myfile.txt");

    // try deleting it using DeleteFile
    if(DeleteFile(pFileToDelete ))
    {
        // succeeded
        std::cout << "Deleted file"  << std::endl;
    }
    else
    {
        // failed
        std::cout << "Failed to delete the file" << std::endl;
    }
    std::cin.get();
    return 0;
}  
nhahtdh
  • 55,989
  • 15
  • 126
  • 162