diff --git a/cpp/src/arrow/filesystem/mockfs.cc b/cpp/src/arrow/filesystem/mockfs.cc index ad7d00923154..4253b2c168b2 100644 --- a/cpp/src/arrow/filesystem/mockfs.cc +++ b/cpp/src/arrow/filesystem/mockfs.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -224,10 +225,15 @@ class MockFileSystem::Impl { TimePoint current_time; // The root directory Entry root; + std::mutex mutex; explicit Impl(TimePoint current_time) : current_time(current_time), root(Directory("", current_time)) {} + std::unique_lock lock_guard() { + return std::unique_lock(mutex); + } + Directory& RootDir() { return root.as_dir(); } template @@ -376,12 +382,14 @@ Status MockFileSystem::CreateDir(const std::string& path, bool recursive) { auto parts = SplitAbstractPath(path); RETURN_NOT_OK(ValidateAbstractPathParts(parts)); + auto guard = impl_->lock_guard(); + size_t consumed; Entry* entry = impl_->FindEntry(parts, &consumed); if (!entry->is_dir()) { auto file_path = JoinAbstractPath(parts.begin(), parts.begin() + consumed); return Status::IOError("Cannot create directory '", path, "': ", "ancestor '", - file_path, "' is a regular file"); + file_path, "' is not a directory"); } if (!recursive && (parts.size() - consumed) > 1) { return Status::IOError("Cannot create directory '", path, @@ -392,6 +400,7 @@ Status MockFileSystem::CreateDir(const std::string& path, bool recursive) { std::unique_ptr child(new Entry(Directory(name, impl_->current_time))); Entry* child_ptr = child.get(); bool inserted = entry->as_dir().CreateEntry(name, std::move(child)); + // No race condition on insertion is possible, as all operations are locked DCHECK(inserted); entry = child_ptr; } @@ -402,6 +411,8 @@ Status MockFileSystem::DeleteDir(const std::string& path) { auto parts = SplitAbstractPath(path); RETURN_NOT_OK(ValidateAbstractPathParts(parts)); + auto guard = impl_->lock_guard(); + Entry* parent = impl_->FindParent(parts); if (parent == nullptr || !parent->is_dir()) { return PathNotFound(path); @@ -424,6 +435,8 @@ Status MockFileSystem::DeleteDirContents(const std::string& path) { auto parts = SplitAbstractPath(path); RETURN_NOT_OK(ValidateAbstractPathParts(parts)); + auto guard = impl_->lock_guard(); + if (parts.empty()) { // Wipe filesystem return internal::InvalidDeleteDirContents(path); @@ -441,6 +454,8 @@ Status MockFileSystem::DeleteDirContents(const std::string& path) { } Status MockFileSystem::DeleteRootDirContents() { + auto guard = impl_->lock_guard(); + impl_->RootDir().entries.clear(); return Status::OK(); } @@ -449,6 +464,8 @@ Status MockFileSystem::DeleteFile(const std::string& path) { auto parts = SplitAbstractPath(path); RETURN_NOT_OK(ValidateAbstractPathParts(parts)); + auto guard = impl_->lock_guard(); + Entry* parent = impl_->FindParent(parts); if (parent == nullptr || !parent->is_dir()) { return PathNotFound(path); @@ -470,6 +487,8 @@ Result MockFileSystem::GetFileInfo(const std::string& path) { auto parts = SplitAbstractPath(path); RETURN_NOT_OK(ValidateAbstractPathParts(parts)); + auto guard = impl_->lock_guard(); + FileInfo info; Entry* entry = impl_->FindEntry(parts); if (entry == nullptr) { @@ -485,6 +504,8 @@ Result> MockFileSystem::GetFileInfo(const FileSelector& se auto parts = SplitAbstractPath(selector.base_dir); RETURN_NOT_OK(ValidateAbstractPathParts(parts)); + auto guard = impl_->lock_guard(); + std::vector results; Entry* base_dir = impl_->FindEntry(parts); @@ -504,6 +525,8 @@ Result> MockFileSystem::GetFileInfo(const FileSelector& se return results; } +namespace { + // Helper for binary operations (move, copy) struct BinaryOp { std::vector src_parts; @@ -523,6 +546,8 @@ struct BinaryOp { RETURN_NOT_OK(ValidateAbstractPathParts(src_parts)); RETURN_NOT_OK(ValidateAbstractPathParts(dest_parts)); + auto guard = impl->lock_guard(); + // Both source and destination must have valid parents Entry* src_parent = impl->FindParent(src_parts); if (src_parent == nullptr || !src_parent->is_dir()) { @@ -552,6 +577,8 @@ struct BinaryOp { } }; +} // namespace + Status MockFileSystem::Move(const std::string& src, const std::string& dest) { return BinaryOp::Run(impl_.get(), src, dest, [&](const BinaryOp& op) -> Status { if (op.src_entry == nullptr) { @@ -609,31 +636,43 @@ Status MockFileSystem::CopyFile(const std::string& src, const std::string& dest) Result> MockFileSystem::OpenInputStream( const std::string& path) { + auto guard = impl_->lock_guard(); + return impl_->OpenInputReader(path); } Result> MockFileSystem::OpenInputFile( const std::string& path) { + auto guard = impl_->lock_guard(); + return impl_->OpenInputReader(path); } Result> MockFileSystem::OpenOutputStream( const std::string& path) { + auto guard = impl_->lock_guard(); + return impl_->OpenOutputStream(path, false /* append */); } Result> MockFileSystem::OpenAppendStream( const std::string& path) { + auto guard = impl_->lock_guard(); + return impl_->OpenOutputStream(path, true /* append */); } std::vector MockFileSystem::AllDirs() { + auto guard = impl_->lock_guard(); + std::vector result; impl_->DumpDirs("", impl_->RootDir(), &result); return result; } std::vector MockFileSystem::AllFiles() { + auto guard = impl_->lock_guard(); + std::vector result; impl_->DumpFiles("", impl_->RootDir(), &result); return result; @@ -642,6 +681,7 @@ std::vector MockFileSystem::AllFiles() { Status MockFileSystem::CreateFile(const std::string& path, const std::string& contents, bool recursive) { auto parent = fs::internal::GetAbstractPathParent(path).first; + if (parent != "") { RETURN_NOT_OK(CreateDir(parent, recursive)); }