From 3906ebf750b80e36c2d6c52130cf40771e1b55fb Mon Sep 17 00:00:00 2001 From: Konstantin Varlamov Date: Mon, 28 Feb 2022 12:55:27 -0500 Subject: [PATCH] [libc++] Fix double file closing in `std::filesystem::remove_all()`. According to Linux documentation (see e.g. https://linux.die.net/man/3/closedir): > A successful call to `closedir()` also closes the underlying file > descriptor associated with `dirp`. Thus, calling `close()` after a successful call to `closedir()` is at best redundant. Worse, should a different thread open a file in-between the calls to `closedir()` and `close()` and get the same file descriptor, the call to `close()` might actually close a different file than was intended. rdar://89251874 Differential Revision: https://reviews.llvm.org/D120453 --- libcxx/src/filesystem/operations.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp index 0e2ebdfe5aa71..7392b83a83ddc 100644 --- a/libcxx/src/filesystem/operations.cpp +++ b/libcxx/src/filesystem/operations.cpp @@ -1416,12 +1416,14 @@ uintmax_t remove_all_impl(int parent_directory, const path& p, error_code& ec) { if (fd != -1) { // If that worked, iterate over the contents of the directory and // remove everything in it, recursively. - scope_exit close_fd([=] { ::close(fd); }); DIR* stream = ::fdopendir(fd); if (stream == nullptr) { + ::close(fd); ec = detail::capture_errno(); return 0; } + // Note: `::closedir` will also close the associated file descriptor, so + // there should be no call to `close(fd)`. scope_exit close_stream([=] { ::closedir(stream); }); uintmax_t count = 0;