Skip to content

Commit

Permalink
Windows: Fix deleting directory in JNI code
Browse files Browse the repository at this point in the history
`bazel clean` command sometimes fails on Windows with
"Directory not empty" error. But when checking the directory after the
failure, we don't see any file or directory under it.
After debugging, we cannot find any unclosed file handle, either.

However, when trying to delete the bazel output directory with `rm -rf" it
succeeds everytime. The reason is that Cygwin worked around a Windows
issue at this commit:
Alexpux/Cygwin@28fa2a7

The problem is, I quote:
""
Intensive testing shows that sometimes directories, for which
the delete disposition has already been set, and the deleting
handle is already closed, can linger in the parent dir for a
couple of ms for no apparent reason (Windows Defender or other
real-time scanners are suspect).
""

In this commit, we use a similar way to retry deleting the directory when
its "deleted" sub-dirs are still lingering under it.

Related bazelbuild#5907
  • Loading branch information
meteorcloudy committed Feb 19, 2019
1 parent 1b4c37c commit 5cb281e
Showing 1 changed file with 87 additions and 4 deletions.
91 changes: 87 additions & 4 deletions src/main/native/windows/file.cc
Original file line number Diff line number Diff line change
Expand Up @@ -446,20 +446,97 @@ int DeletePath(const wstring& path, wstring* error) {
// DeleteFileW failed with access denied, but the path exists.
if (attr & FILE_ATTRIBUTE_DIRECTORY) {
// It's a directory or a junction.
if (!RemoveDirectoryW(wpath)) {

// Sometimes a deleted directory lingers in its parent dir
// after the deleting handle has already been closed.
// In this case we check the content of the parent directory,
// if we don't find any valid file, we try to delete it again after 5 ms.
// But we don't want to hang infinitely because another application
// can hold the handle for a long time. So try at most 20 times,
// which means a process time of 100-120ms.
// Inspired by https://github.com/Alexpux/Cygwin/commit/28fa2a72f810670a0562ea061461552840f5eb70
int count = 20;
while (count > 0) {
if (RemoveDirectoryW(wpath)) {
break;
}
// Failed to delete the directory.
err = GetLastError();
if (err == ERROR_SHARING_VIOLATION || err == ERROR_ACCESS_DENIED) {
// The junction or directory is in use by another process, or we have
// no permission to delete it.
return DeletePathResult::kAccessDenied;
} else if (err == ERROR_DIR_NOT_EMPTY) {
// The directory is not empty.
return DeletePathResult::kDirectoryNotEmpty;
} else if (err == ERROR_FILE_NOT_FOUND || err == ERROR_PATH_NOT_FOUND) {
// The directory or one of its directories disappeared or is no longer
// a directory.
return DeletePathResult::kDoesNotExist;
} else if (err == ERROR_DIR_NOT_EMPTY) {
static const wstring kDot(L".");
static const wstring kDotDot(L"..");
bool found_valid_file = false;
bool found_pending_delete_file = false;
WIN32_FIND_DATAW metadata;
HANDLE handle = ::FindFirstFileW((winpath + L"\\*").c_str(), &metadata);
if (handle != INVALID_HANDLE_VALUE) {
do {
if (kDot != metadata.cFileName && kDotDot != metadata.cFileName) {
std::wstring wname = winpath + L"\\" + metadata.cFileName;
HANDLE hFile = CreateFileW(wname.c_str(),
GENERIC_READ,
FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE,
NULL,
OPEN_EXISTING,
FILE_FLAG_BACKUP_SEMANTICS,
NULL);
if (hFile != INVALID_HANDLE_VALUE) {
// If there is a valid file under the directory, we should stop
// and return kDirectoryNotEmpty.
CloseHandle(hFile);
found_valid_file = true;
break;
} else {
DWORD error_code = GetLastError();
// If the file or directory is in deleting process,
// CreateFileW returns ERROR_ACCESS_DENIED,
// If it's already deleted at the time we check,
// CreateFileW returns ERROR_FILE_NOT_FOUND.
// If CreateFileW fails with other reason, we consider there is a
// valid file that we cannot open, thus set found_valid_file to true.
if (error_code != ERROR_ACCESS_DENIED &&
error_code != ERROR_FILE_NOT_FOUND) {
found_valid_file = true;
break;
} else if (error_code == ERROR_ACCESS_DENIED) {
found_pending_delete_file = true;
}
}
}
} while (::FindNextFileW(handle, &metadata));
::FindClose(handle);
if (found_valid_file) {
// The directory is truely not empty.
return DeletePathResult::kDirectoryNotEmpty;
} else {
// If we find pending delete files, wait for the system to clean them up
// and try to delete the directory again.
// If we didn't find any pending delete files, it means at the time we check,
// the deleted files are gone, the directory is now empty, we can then try to
// delete again without waiting.
if (found_pending_delete_file) {
Sleep(5L);
}
count--;
continue;
}
} else {
// This case should never happen, because ERROR_DIR_NOT_EMPTY means the directory exists.
// But if it does happen, return an error message.
if (error) {
*error = MakeErrorMessage(WSTR(__FILE__), __LINE__,
L"RemoveDirectoryW", path, GetLastError());
}
return DeletePathResult::kError;
}
}

// Some unknown error occurred.
Expand All @@ -469,6 +546,12 @@ int DeletePath(const wstring& path, wstring* error) {
}
return DeletePathResult::kError;
}

if (count == 0) {
// After trying 20 times, the "deleted" sub-directories or files still won't go away,
// so just return kDirectoryNotEmpty error.
return DeletePathResult::kDirectoryNotEmpty;
}
} else if (attr & FILE_ATTRIBUTE_READONLY) {
// It's a file and it's probably read-only.
// Make it writable then try deleting it again.
Expand Down

0 comments on commit 5cb281e

Please sign in to comment.