Merge pull request #10884 from liamwhite/spaghetti-vfs

vfs_real: lock concurrent accesses
This commit is contained in:
liamwhite 2023-06-23 09:26:48 -04:00 committed by GitHub
commit 575d467d95
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 45 additions and 25 deletions

View file

@ -76,6 +76,7 @@ VfsEntryType RealVfsFilesystem::GetEntryType(std::string_view path_) const {
VirtualFile RealVfsFilesystem::OpenFileFromEntry(std::string_view path_, std::optional<u64> size, VirtualFile RealVfsFilesystem::OpenFileFromEntry(std::string_view path_, std::optional<u64> size,
Mode perms) { Mode perms) {
const auto path = FS::SanitizePath(path_, FS::DirectorySeparator::PlatformDefault); const auto path = FS::SanitizePath(path_, FS::DirectorySeparator::PlatformDefault);
std::scoped_lock lk{list_lock};
if (auto it = cache.find(path); it != cache.end()) { if (auto it = cache.find(path); it != cache.end()) {
if (auto file = it->second.lock(); file) { if (auto file = it->second.lock(); file) {
@ -88,7 +89,7 @@ VirtualFile RealVfsFilesystem::OpenFileFromEntry(std::string_view path_, std::op
} }
auto reference = std::make_unique<FileReference>(); auto reference = std::make_unique<FileReference>();
this->InsertReferenceIntoList(*reference); this->InsertReferenceIntoListLocked(*reference);
auto file = std::shared_ptr<RealVfsFile>( auto file = std::shared_ptr<RealVfsFile>(
new RealVfsFile(*this, std::move(reference), path, perms, size)); new RealVfsFile(*this, std::move(reference), path, perms, size));
@ -103,7 +104,10 @@ VirtualFile RealVfsFilesystem::OpenFile(std::string_view path_, Mode perms) {
VirtualFile RealVfsFilesystem::CreateFile(std::string_view path_, Mode perms) { VirtualFile RealVfsFilesystem::CreateFile(std::string_view path_, Mode perms) {
const auto path = FS::SanitizePath(path_, FS::DirectorySeparator::PlatformDefault); const auto path = FS::SanitizePath(path_, FS::DirectorySeparator::PlatformDefault);
{
std::scoped_lock lk{list_lock};
cache.erase(path); cache.erase(path);
}
// Current usages of CreateFile expect to delete the contents of an existing file. // Current usages of CreateFile expect to delete the contents of an existing file.
if (FS::IsFile(path)) { if (FS::IsFile(path)) {
@ -133,8 +137,11 @@ VirtualFile RealVfsFilesystem::CopyFile(std::string_view old_path_, std::string_
VirtualFile RealVfsFilesystem::MoveFile(std::string_view old_path_, std::string_view new_path_) { VirtualFile RealVfsFilesystem::MoveFile(std::string_view old_path_, std::string_view new_path_) {
const auto old_path = FS::SanitizePath(old_path_, FS::DirectorySeparator::PlatformDefault); const auto old_path = FS::SanitizePath(old_path_, FS::DirectorySeparator::PlatformDefault);
const auto new_path = FS::SanitizePath(new_path_, FS::DirectorySeparator::PlatformDefault); const auto new_path = FS::SanitizePath(new_path_, FS::DirectorySeparator::PlatformDefault);
{
std::scoped_lock lk{list_lock};
cache.erase(old_path); cache.erase(old_path);
cache.erase(new_path); cache.erase(new_path);
}
if (!FS::RenameFile(old_path, new_path)) { if (!FS::RenameFile(old_path, new_path)) {
return nullptr; return nullptr;
} }
@ -143,7 +150,10 @@ VirtualFile RealVfsFilesystem::MoveFile(std::string_view old_path_, std::string_
bool RealVfsFilesystem::DeleteFile(std::string_view path_) { bool RealVfsFilesystem::DeleteFile(std::string_view path_) {
const auto path = FS::SanitizePath(path_, FS::DirectorySeparator::PlatformDefault); const auto path = FS::SanitizePath(path_, FS::DirectorySeparator::PlatformDefault);
{
std::scoped_lock lk{list_lock};
cache.erase(path); cache.erase(path);
}
return FS::RemoveFile(path); return FS::RemoveFile(path);
} }
@ -182,14 +192,17 @@ bool RealVfsFilesystem::DeleteDirectory(std::string_view path_) {
return FS::RemoveDirRecursively(path); return FS::RemoveDirRecursively(path);
} }
void RealVfsFilesystem::RefreshReference(const std::string& path, Mode perms, std::unique_lock<std::mutex> RealVfsFilesystem::RefreshReference(const std::string& path,
Mode perms,
FileReference& reference) { FileReference& reference) {
std::unique_lock lk{list_lock};
// Temporarily remove from list. // Temporarily remove from list.
this->RemoveReferenceFromList(reference); this->RemoveReferenceFromListLocked(reference);
// Restore file if needed. // Restore file if needed.
if (!reference.file) { if (!reference.file) {
this->EvictSingleReference(); this->EvictSingleReferenceLocked();
reference.file = reference.file =
FS::FileOpen(path, ModeFlagsToFileAccessMode(perms), FS::FileType::BinaryFile); FS::FileOpen(path, ModeFlagsToFileAccessMode(perms), FS::FileType::BinaryFile);
@ -199,12 +212,16 @@ void RealVfsFilesystem::RefreshReference(const std::string& path, Mode perms,
} }
// Reinsert into list. // Reinsert into list.
this->InsertReferenceIntoList(reference); this->InsertReferenceIntoListLocked(reference);
return lk;
} }
void RealVfsFilesystem::DropReference(std::unique_ptr<FileReference>&& reference) { void RealVfsFilesystem::DropReference(std::unique_ptr<FileReference>&& reference) {
std::scoped_lock lk{list_lock};
// Remove from list. // Remove from list.
this->RemoveReferenceFromList(*reference); this->RemoveReferenceFromListLocked(*reference);
// Close the file. // Close the file.
if (reference->file) { if (reference->file) {
@ -213,14 +230,14 @@ void RealVfsFilesystem::DropReference(std::unique_ptr<FileReference>&& reference
} }
} }
void RealVfsFilesystem::EvictSingleReference() { void RealVfsFilesystem::EvictSingleReferenceLocked() {
if (num_open_files < MaxOpenFiles || open_references.empty()) { if (num_open_files < MaxOpenFiles || open_references.empty()) {
return; return;
} }
// Get and remove from list. // Get and remove from list.
auto& reference = open_references.back(); auto& reference = open_references.back();
this->RemoveReferenceFromList(reference); this->RemoveReferenceFromListLocked(reference);
// Close the file. // Close the file.
if (reference.file) { if (reference.file) {
@ -229,10 +246,10 @@ void RealVfsFilesystem::EvictSingleReference() {
} }
// Reinsert into closed list. // Reinsert into closed list.
this->InsertReferenceIntoList(reference); this->InsertReferenceIntoListLocked(reference);
} }
void RealVfsFilesystem::InsertReferenceIntoList(FileReference& reference) { void RealVfsFilesystem::InsertReferenceIntoListLocked(FileReference& reference) {
if (reference.file) { if (reference.file) {
open_references.push_front(reference); open_references.push_front(reference);
} else { } else {
@ -240,7 +257,7 @@ void RealVfsFilesystem::InsertReferenceIntoList(FileReference& reference) {
} }
} }
void RealVfsFilesystem::RemoveReferenceFromList(FileReference& reference) { void RealVfsFilesystem::RemoveReferenceFromListLocked(FileReference& reference) {
if (reference.file) { if (reference.file) {
open_references.erase(open_references.iterator_to(reference)); open_references.erase(open_references.iterator_to(reference));
} else { } else {
@ -271,7 +288,7 @@ std::size_t RealVfsFile::GetSize() const {
bool RealVfsFile::Resize(std::size_t new_size) { bool RealVfsFile::Resize(std::size_t new_size) {
size.reset(); size.reset();
base.RefreshReference(path, perms, *reference); auto lk = base.RefreshReference(path, perms, *reference);
return reference->file ? reference->file->SetSize(new_size) : false; return reference->file ? reference->file->SetSize(new_size) : false;
} }
@ -288,7 +305,7 @@ bool RealVfsFile::IsReadable() const {
} }
std::size_t RealVfsFile::Read(u8* data, std::size_t length, std::size_t offset) const { std::size_t RealVfsFile::Read(u8* data, std::size_t length, std::size_t offset) const {
base.RefreshReference(path, perms, *reference); auto lk = base.RefreshReference(path, perms, *reference);
if (!reference->file || !reference->file->Seek(static_cast<s64>(offset))) { if (!reference->file || !reference->file->Seek(static_cast<s64>(offset))) {
return 0; return 0;
} }
@ -297,7 +314,7 @@ std::size_t RealVfsFile::Read(u8* data, std::size_t length, std::size_t offset)
std::size_t RealVfsFile::Write(const u8* data, std::size_t length, std::size_t offset) { std::size_t RealVfsFile::Write(const u8* data, std::size_t length, std::size_t offset) {
size.reset(); size.reset();
base.RefreshReference(path, perms, *reference); auto lk = base.RefreshReference(path, perms, *reference);
if (!reference->file || !reference->file->Seek(static_cast<s64>(offset))) { if (!reference->file || !reference->file->Seek(static_cast<s64>(offset))) {
return 0; return 0;
} }

View file

@ -4,6 +4,7 @@
#pragma once #pragma once
#include <map> #include <map>
#include <mutex>
#include <optional> #include <optional>
#include <string_view> #include <string_view>
#include "common/intrusive_list.h" #include "common/intrusive_list.h"
@ -48,22 +49,24 @@ private:
std::map<std::string, std::weak_ptr<VfsFile>, std::less<>> cache; std::map<std::string, std::weak_ptr<VfsFile>, std::less<>> cache;
ReferenceListType open_references; ReferenceListType open_references;
ReferenceListType closed_references; ReferenceListType closed_references;
std::mutex list_lock;
size_t num_open_files{}; size_t num_open_files{};
private: private:
friend class RealVfsFile; friend class RealVfsFile;
void RefreshReference(const std::string& path, Mode perms, FileReference& reference); std::unique_lock<std::mutex> RefreshReference(const std::string& path, Mode perms,
FileReference& reference);
void DropReference(std::unique_ptr<FileReference>&& reference); void DropReference(std::unique_ptr<FileReference>&& reference);
void EvictSingleReference();
private:
void InsertReferenceIntoList(FileReference& reference);
void RemoveReferenceFromList(FileReference& reference);
private: private:
friend class RealVfsDirectory; friend class RealVfsDirectory;
VirtualFile OpenFileFromEntry(std::string_view path, std::optional<u64> size, VirtualFile OpenFileFromEntry(std::string_view path, std::optional<u64> size,
Mode perms = Mode::Read); Mode perms = Mode::Read);
private:
void EvictSingleReferenceLocked();
void InsertReferenceIntoListLocked(FileReference& reference);
void RemoveReferenceFromListLocked(FileReference& reference);
}; };
// An implementation of VfsFile that represents a file on the user's computer. // An implementation of VfsFile that represents a file on the user's computer.