Skip to content

C
capnproto
Commit 38541d69 authored by Kenton Varda's avatar Kenton Varda
Browse files
Options

Adjust win32 recursive delete logic. 

Now that I know the actual reasoning behind the loop, I think it's not the best answer. Instead, we should retry removing the directory if it reports not-empty, but only a few times before we give up.
parent 609ea777
Hide whitespace changes
Inline Side-by-side
Showing with 86 additions and 78 deletions
c++/src/kj/filesystem-disk-test.c++
View file @ 38541d69
...@@ -142,49 +142,53 @@ private: ...@@ -142,49 +142,53 @@ private:
auto glob = join16(path, L"\\*"); auto glob = join16(path, L"\\*");
for (;;) { WIN32_FIND_DATAW data;
WIN32_FIND_DATAW data; HANDLE handle = FindFirstFileW(glob.begin(), &data);
HANDLE handle = FindFirstFileW(glob.begin(), &data); if (handle == INVALID_HANDLE_VALUE) {
if (handle == INVALID_HANDLE_VALUE) { auto error = GetLastError();
auto error = GetLastError(); if (error == ERROR_FILE_NOT_FOUND) return;
if (error == ERROR_FILE_NOT_FOUND) return; KJ_FAIL_WIN32("FindFirstFile", error, path) { return; }
KJ_FAIL_WIN32("FindFirstFile", error, path) { return; } }
} KJ_DEFER(KJ_WIN32(FindClose(handle)) { break; });
KJ_DEFER(KJ_WIN32(FindClose(handle)) { break; });
do {
bool foundAny = false; // Ignore "." and "..", ugh.
do { if (data.cFileName[0] == L'.') {
// Ignore "." and "..", ugh. if (data.cFileName[1] == L'\0' ||
if (data.cFileName[0] == L'.') { (data.cFileName[1] == L'.' && data.cFileName[2] == L'\0')) {
if (data.cFileName[1] == L'\0' || continue;
(data.cFileName[1] == L'.' && data.cFileName[2] == L'\0')) {
continue;
}
} }
}
String utf8Name = decodeWideString(arrayPtr(data.cFileName, wcslen(data.cFileName))); String utf8Name = decodeWideString(arrayPtr(data.cFileName, wcslen(data.cFileName)));
KJ_EXPECT(!utf8Name.startsWith(".kj-tmp."), "temp file not cleaned up", utf8Name); KJ_EXPECT(!utf8Name.startsWith(".kj-tmp."), "temp file not cleaned up", utf8Name);
auto child = join16(path, data.cFileName);
if ((data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) &&
!(data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
recursiveDelete(child);
} else {
KJ_WIN32(DeleteFileW(child.begin()));
}
} while (FindNextFileW(handle, &data));
auto error = GetLastError(); auto child = join16(path, data.cFileName);
if (error != ERROR_NO_MORE_FILES) { if ((data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) &&
KJ_FAIL_WIN32("FindNextFile", error, path) { return; } !(data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
recursiveDelete(child);
} else {
KJ_WIN32(DeleteFileW(child.begin()));
} }
} while (FindNextFileW(handle, &data));
if (!foundAny) { auto error = GetLastError();
return; if (error != ERROR_NO_MORE_FILES) {
} KJ_FAIL_WIN32("FindNextFile", error, path) { return; }
} }
KJ_WIN32(RemoveDirectoryW(path.begin())); uint retryCount = 0;
retry:
KJ_WIN32_HANDLE_ERRORS(RemoveDirectoryW(path.begin())) {
case ERROR_DIR_NOT_EMPTY:
if (retryCount++ < 10) {
Sleep(10);
goto retry;
}
// fallthrough
default:
KJ_FAIL_WIN32("RemoveDirectory", error) { break; }
}
} }
}; };
......
c++/src/kj/filesystem-disk-win32.c++
View file @ 38541d69
...@@ -151,54 +151,58 @@ static String dbgStr(ArrayPtr<const wchar_t> wstr) { ...@@ -151,54 +151,58 @@ static String dbgStr(ArrayPtr<const wchar_t> wstr) {
static void rmrfChildren(ArrayPtr<const wchar_t> path) { static void rmrfChildren(ArrayPtr<const wchar_t> path) {
auto glob = join16(path, L"*"); auto glob = join16(path, L"*");
// According to Niall Douglas, on Windows, deleting all files in a directory requires repeatedly WIN32_FIND_DATAW data;
// scanning the directory until it is found to be empty. I couldn't find an explanation why, but HANDLE handle = FindFirstFileW(glob.begin(), &data);
// my guess is that deleting a file while a search is in progress could reorder the remaining if (handle == INVALID_HANDLE_VALUE) {
// files causing the iterator to possibly skip things? auto error = GetLastError();
// if (error == ERROR_FILE_NOT_FOUND) return;
// Anyway, hence the loop. KJ_FAIL_WIN32("FindFirstFile", error, dbgStr(glob)) { return; }
for (;;) { }
WIN32_FIND_DATAW data; KJ_DEFER(KJ_WIN32(FindClose(handle)) { break; });
HANDLE handle = FindFirstFileW(glob.begin(), &data);
if (handle == INVALID_HANDLE_VALUE) {
auto error = GetLastError();
if (error == ERROR_FILE_NOT_FOUND) return;
KJ_FAIL_WIN32("FindFirstFile", error, dbgStr(glob)) { return; }
}
KJ_DEFER(KJ_WIN32(FindClose(handle)) { break; });
bool foundAny = false; do {
do { // Ignore "." and "..", ugh.
// Ignore "." and "..", ugh. if (data.cFileName[0] == L'.') {
if (data.cFileName[0] == L'.') { if (data.cFileName[1] == L'\0' ||
if (data.cFileName[1] == L'\0' || (data.cFileName[1] == L'.' && data.cFileName[2] == L'\0')) {
(data.cFileName[1] == L'.' && data.cFileName[2] == L'\0')) { continue;
continue;
}
} }
}
foundAny = true; auto child = join16(path, data.cFileName);
auto child = join16(path, data.cFileName); // For rmrf purposes, we assume any "reparse points" are symlink-like, even if they aren't
// For rmrf purposes, we assume any "reparse points" are symlink-like, even if they aren't // actually the "symbolic link" reparse type, because we don't want to recursively delete any
// actually the "symbolic link" reparse type, because we don't want to recursively delete any // shared content.
// shared content. if ((data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) &&
if ((data.dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY) && !(data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) {
!(data.dwFileAttributes & FILE_ATTRIBUTE_REPARSE_POINT)) { rmrfChildren(child);
rmrfChildren(child); uint retryCount = 0;
KJ_WIN32(RemoveDirectoryW(child.begin())); retry:
} else { KJ_WIN32_HANDLE_ERRORS(RemoveDirectoryW(child.begin())) {
KJ_WIN32(DeleteFileW(child.begin())); case ERROR_DIR_NOT_EMPTY:
// On Windows, deleting a file actually only schedules it for deletion. Under heavy
// load it may take a bit for the deletion to go through. Or, if another process has
// the file open, it may not be deleted until that process closes it.
//
// We'll repeatedly retry for up to 100ms, then give up. This is awful but there's no
// way to tell for sure if the system is just being slow or if someone has the file
// open.
if (retryCount++ < 10) {
Sleep(10);
goto retry;
}
// fallthrough
default:
KJ_FAIL_WIN32("RemoveDirectory", error, dbgStr(child)) { break; }
} }
} while (FindNextFileW(handle, &data)); } else {
KJ_WIN32(DeleteFileW(child.begin()));
auto error = GetLastError();
if (error != ERROR_NO_MORE_FILES) {
KJ_FAIL_WIN32("FindNextFile", error, dbgStr(path)) { return; }
} }
} while (FindNextFileW(handle, &data));
if (!foundAny) { auto error = GetLastError();
return; if (error != ERROR_NO_MORE_FILES) {
} KJ_FAIL_WIN32("FindNextFile", error, dbgStr(path)) { return; }
} }
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment