1

How do I correct the piece of code below for it to work properly? The purpose of the function is only to delete directories and files inside of the directory.

Because when I run this function (even in a separate thread), it doesn't delete the folder passed in the first argument due to some handles existing (probably).

And Windows doesn't delete the folder until the end of the program, see code below.

How do I fix existing of handles before the RemoveDirectory is called?

I already tried to move the HANDLE variable (which is inside of the function below) out of stack when RemoveDirectory is called.

The problem is just in the delayed deleting of the folder (not other files, they delete normally).

int DeleteDirectory(const std::string &refcstrRootDirectory,
                    bool              bDeleteSubdirectories = true)
{
  bool            bSubdirectory = false;       // Flag, indicating whether
                                               // subdirectories have been found
  HANDLE          hFile;                       // Handle to directory
  std::string     strFilePath;                 // Filepath
  std::string     strPattern;                  // Pattern
  WIN32_FIND_DATA FileInformation;             // File information


  strPattern = refcstrRootDirectory + "\\*.*";
  hFile = ::FindFirstFile(strPattern.c_str(), &FileInformation);
  if(hFile != INVALID_HANDLE_VALUE)
  {
    do
    {
      if(FileInformation.cFileName[0] != '.')
      {
        strFilePath.erase();
        strFilePath = refcstrRootDirectory + "\\" + FileInformation.cFileName;

        if(FileInformation.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
        {
          if(bDeleteSubdirectories)
          {
            // Delete subdirectory
            int iRC = DeleteDirectory(strFilePath, bDeleteSubdirectories);
            if(iRC)
              return iRC;
          }
          else
            bSubdirectory = true;
        }
        else
        {
          // Set file attributes
          if(::SetFileAttributes(strFilePath.c_str(),
                                 FILE_ATTRIBUTE_NORMAL) == FALSE)
            return ::GetLastError();

          // Delete file
          if(::DeleteFile(strFilePath.c_str()) == FALSE)
            return ::GetLastError();
        }
      }
    } while(::FindNextFile(hFile, &FileInformation) == TRUE);

    // Close handle
    ::FindClose(hFile);

    DWORD dwError = ::GetLastError();
    if(dwError != ERROR_NO_MORE_FILES)
      return dwError;
    else
    {
      if(!bSubdirectory)
      {
        // Set directory attributes
        if(::SetFileAttributes(refcstrRootDirectory.c_str(),
                               FILE_ATTRIBUTE_NORMAL) == FALSE)
          return ::GetLastError();

        // Delete directory
        if(::RemoveDirectory(refcstrRootDirectory.c_str()) == FALSE)
          return ::GetLastError();
      }
    }
  }
  return 0;
}

p.s. the piece of code was taken from here: How to delete a folder in C++?

Ivan Silkin
  • 381
  • 1
  • 7
  • 15
  • 2
    Debug it, Visual Studio has a great debugger which is very easy to use. – Jabberwocky Mar 02 '18 at 13:41
  • 2
    Can you cut this down to a simple reproduction. Say the deletion of a single directory? – David Heffernan Mar 02 '18 at 13:42
  • 1
    Save a list of the directories you want to delete in your loop, then when you're out of the loop and all the handles are closed, loop through your saved list and delete them then? – parrowdice Mar 02 '18 at 13:56
  • And for your 'probably' comment, SysInternals Process Explorer has a great tool which will tell you exactly what process are keeping a handle to your directories, so you can confirm if that's the case, and if you're fixing it with your new code – parrowdice Mar 02 '18 at 13:57
  • I just ran and checked the code, and IMO it should work, but `RemoveDirectory` finally fails with error ERROR_SHARING_VIOLATION although the `hFile ` handle has been close sucessfully by `::FindClose(hFile);`. – Jabberwocky Mar 02 '18 at 14:01
  • You have to deal with other stuff opening a handle on the directory. Only your program can mess this up by making this directory also the default working directory of your program. But other processes can do it too, like anti-malware, indexers, shells. They'll open the directory with delete sharing so you can delete the directory. But it won't disappear from the file system until the last handle is closed. Moving it to the recycle bin with SHFileOperation() is an easy way to avoid trouble. – Hans Passant Mar 02 '18 at 14:05
  • HansPassant's comment is convincing. I just tried again after closing all Explorer Windows and then it worked. – Jabberwocky Mar 02 '18 at 14:10
  • @parrowdice - not need save any list of directories (why not all files ??). all can be possible do just in place. if use native (documented!) api solution will be much more simply and efficient – RbMm Mar 02 '18 at 14:11
  • All this redundant `StringToUnicode` / `UnicodeToString` just distracts from the actual code. Please create a [mcve]. – zett42 Mar 02 '18 at 14:38
  • @zett42, I've corrected the code in accordance with the original one – Ivan Silkin Mar 02 '18 at 14:56
  • also you need check not for `FILE_ATTRIBUTE_DIRECTORY` only, but without `FILE_ATTRIBUTE_REPARSE_POINT`. in what concrete your error ? – RbMm Mar 02 '18 at 15:01

1 Answers1

1

First of all, I apologize for asking such a misleading question but...

I have found my mistake, It is not in the function that I posted in the question.

I have a project where I used my function DirectoryExists, and there was a function from the header "opendir" (It is a portable version of linux header for Windows and there are probably WINAPI HANDLEs inside of it). And I forgot to close the directory after I've opened it to check if it exists.

bool DirectoryExists(const std::string & path)
{
    DIR *dir = opendir(path.c_str());
    if (dir) {
        return true;
    }
    else
        return false;
}

And I've corrected the error:

bool DirectoryExists(const std::string & path)
{
    DIR *dir = opendir(path.c_str());
    closedir(dir);
    if (dir) {
        return true;
    }
    else
        return false;
}

I hope that this question can help someone to understand why RemoveDirectory() could work as not expected. There could be some handles that you did not notice and It's not that clear If you're not a professional Windows programmer.

Ivan Silkin
  • 381
  • 1
  • 7
  • 15
  • 1
    The Windows filesystem is not like a Posix one, and this code can still fail because of races. This exact situation is explained in this excellent talk: https://www.youtube.com/watch?v=uhRWMGBjlO8 – Adrian McCarthy Mar 02 '18 at 19:21
  • 1
    Under Windows you don't need to open a directory to check for its existence. You only need to check the file attributes, using `GetFileAttributes()` for instance. – zett42 Mar 02 '18 at 20:11