Skip to content

Commit 230be16

Browse files
tetrominocopybara-github
authored andcommitted
Do not interleave readdir() calls with deletion of directory entries.
On macOS and some other non-Linux OSes, on some filesystems, readdir(dir) may return NULL (with zero errno) after an entry in dir has been deleted. We thus need to readdir() all directory entries before starting to delete them. A benchmark (deleting 10 copies of the Linux kernel source) seems to show that the new approach is approximately as fast as the previous one (slightly faster on Linux, slightly slower on Mac), and in any case, is noticeably faster than the system's /bin/rm. Bazel's previous unix_jni DeleteTreesBelow implementation: 6.987 s (Linux/ext4); 89.44 s (Mac/APFS) New Bazel unix_jni DeleteTreesBelow implementation: 6.971 s (Linux/ext4); 90.46 s (Mac/APFS) `rm -rf`: 7.323 s (Linux/ext4); 99.09 s (Mac/APFS) RELNOTES: None. PiperOrigin-RevId: 346790610
1 parent bf32cb8 commit 230be16

File tree

1 file changed

+28
-5
lines changed

1 file changed

+28
-5
lines changed

src/main/native/unix_jni.cc

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -907,6 +907,13 @@ static int DeleteTreesBelow(JNIEnv* env, std::vector<std::string>* dir_path,
907907
}
908908

909909
dir_path->push_back(entry);
910+
// On macOS and some other non-Linux OSes, on some filesystems, readdir(dir)
911+
// may return NULL after an entry in dir is deleted even if not all files have
912+
// been read yet - see https://support.apple.com/kb/TA21420; we thus read all
913+
// the names of dir's entries before deleting. We don't want to simply use
914+
// fts(3) because we want to be able to chmod at any point in the directory
915+
// hierarchy to retry a filesystem operation after hitting an EACCES.
916+
std::vector<std::string> dir_files, dir_subdirs;
910917
for (;;) {
911918
errno = 0;
912919
struct dirent* de = readdir(dir);
@@ -927,15 +934,31 @@ static int DeleteTreesBelow(JNIEnv* env, std::vector<std::string>* dir_path,
927934
break;
928935
}
929936
if (is_dir) {
930-
if (DeleteTreesBelow(env, dir_path, dirfd(dir), de->d_name) == -1) {
937+
dir_subdirs.push_back(de->d_name);
938+
} else {
939+
dir_files.push_back(de->d_name);
940+
}
941+
}
942+
if (env->ExceptionOccurred() == NULL) {
943+
for (const auto &file : dir_files) {
944+
if (ForceDelete(env, *dir_path, dirfd(dir), file.c_str(), false) == -1) {
931945
CHECK(env->ExceptionOccurred() != NULL);
932946
break;
933947
}
934948
}
935-
936-
if (ForceDelete(env, *dir_path, dirfd(dir), de->d_name, is_dir) == -1) {
937-
CHECK(env->ExceptionOccurred() != NULL);
938-
break;
949+
// DeleteTreesBelow is recursive; don't hold on to file names unnecessarily.
950+
dir_files.clear();
951+
}
952+
if (env->ExceptionOccurred() == NULL) {
953+
for (const auto &subdir : dir_subdirs) {
954+
if (DeleteTreesBelow(env, dir_path, dirfd(dir), subdir.c_str()) == -1) {
955+
CHECK(env->ExceptionOccurred() != NULL);
956+
break;
957+
}
958+
if (ForceDelete(env, *dir_path, dirfd(dir), subdir.c_str(), true) == -1) {
959+
CHECK(env->ExceptionOccurred() != NULL);
960+
break;
961+
}
939962
}
940963
}
941964
if (closedir(dir) == -1) {

0 commit comments

Comments
 (0)