Make the Loader API thread-safe

Since the majority of the code is using ReadAt API already, map this to
a `readp` "syscall" which does not mutate any state about the file
descriptor therefore making it fairly safe multi-threading wise.

This allows to get rid of read-time mutexes in RamCachedFileLoader and
therefore fixes #9803

Core/FileLoaders/CachingFileLoader.cpp

@@ -25,7 +25,7 @@
 
 // Takes ownership of backend.
 CachingFileLoader::CachingFileLoader(FileLoader *backend)
-	: filesize_(0), filepos_(0), backend_(backend), exists_(-1), isDirectory_(-1), aheadThread_(false), prepared_(false) {
+	: filesize_(0), backend_(backend), exists_(-1), isDirectory_(-1), aheadThread_(false), prepared_(false) {
 }
 
 void CachingFileLoader::Prepare() {
@@ -82,10 +82,6 @@ std::string CachingFileLoader::Path() const {
 	return backend_->Path();
 }
 
-void CachingFileLoader::Seek(s64 absolutePos) {
-	filepos_ = absolutePos;
-}
-
 size_t CachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags) {
 	Prepare();
 	if (absolutePos >= filesize_) {
@@ -114,7 +110,6 @@ size_t CachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flag
 		StartReadAhead(absolutePos + readSize);
 	}
 
-	filepos_ = absolutePos + readSize;
 	return readSize;
 }
 

Core/FileLoaders/CachingFileLoader.h

@@ -34,13 +34,6 @@ public:
 	s64 FileSize() override;
 	std::string Path() const override;
 
-	void Seek(s64 absolutePos) override;
-	size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, count, data, flags);
-	}
-	size_t Read(size_t bytes, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, data, flags);
-	}
 	size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
 		return ReadAt(absolutePos, bytes * count, data, flags) / bytes;
 	}
@@ -65,7 +58,6 @@ private:
 	};
 
 	s64 filesize_;
-	s64 filepos_;
 	FileLoader *backend_;
 	int exists_;
 	int isDirectory_;

Core/FileLoaders/DiskCachingFileLoader.cpp

@@ -41,7 +41,7 @@ std::mutex DiskCachingFileLoader::cachesMutex_;
 
 // Takes ownership of backend.
 DiskCachingFileLoader::DiskCachingFileLoader(FileLoader *backend)
-	: prepared_(false), filesize_(0), filepos_(0), backend_(backend), cache_(nullptr) {
+	: prepared_(false), filesize_(0), backend_(backend), cache_(nullptr) {
 }
 
 void DiskCachingFileLoader::Prepare() {
@@ -88,10 +88,6 @@ std::string DiskCachingFileLoader::Path() const {
 	return backend_->Path();
 }
 
-void DiskCachingFileLoader::Seek(s64 absolutePos) {
-	filepos_ = absolutePos;
-}
-
 size_t DiskCachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags) {
 	Prepare();
 	size_t readSize;
@@ -119,7 +115,6 @@ size_t DiskCachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data,
 		readSize = backend_->ReadAt(absolutePos, bytes, data, flags);
 	}
 
-	filepos_ = absolutePos + readSize;
 	return readSize;
 }
 

Core/FileLoaders/DiskCachingFileLoader.h

@@ -37,13 +37,6 @@ public:
 	s64 FileSize() override;
 	std::string Path() const override;
 
-	void Seek(s64 absolutePos) override;
-	size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, count, data, flags);
-	}
-	size_t Read(size_t bytes, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, data, flags);
-	}
 	size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
 		return ReadAt(absolutePos, bytes * count, data, flags) / bytes;
 	}
@@ -58,7 +51,6 @@ private:
 
 	bool prepared_;
 	s64 filesize_;
-	s64 filepos_;
 	FileLoader *backend_;
 	DiskCachingFileLoaderCache *cache_;
 

Core/FileLoaders/HTTPFileLoader.cpp

@@ -115,10 +115,6 @@ std::string HTTPFileLoader::Path() const {
 	return filename_;
 }
 
-void HTTPFileLoader::Seek(s64 absolutePos) {
-	filepos_ = absolutePos;
-}
-
 size_t HTTPFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags) {
 	Prepare();
 	s64 absoluteEnd = std::min(absolutePos + (s64)bytes, filesize_);

Core/FileLoaders/HTTPFileLoader.h

@@ -34,13 +34,6 @@ public:
 	virtual s64 FileSize() override;
 	virtual std::string Path() const override;
 
-	virtual void Seek(s64 absolutePos) override;
-	virtual size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, count, data, flags);
-	}
-	virtual size_t Read(size_t bytes, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, data, flags);
-	}
 	virtual size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
 		return ReadAt(absolutePos, bytes * count, data, flags) / bytes;
 	}

Core/FileLoaders/LocalFileLoader.cpp

@@ -15,14 +15,15 @@
 // Official git repository and contact information can be found at
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
-#include <cstdio>
 #include "file/file_util.h"
 #include "Common/FileUtil.h"
 #include "Core/FileLoaders/LocalFileLoader.h"
 
 LocalFileLoader::LocalFileLoader(const std::string &filename)
 	: fd_(0), f_(nullptr), filesize_(0), filename_(filename) {
+    // FIXME: perhaps at this point we should just use plain open?
 	f_ = File::OpenCFile(filename, "rb");
+	fd_ = fileno(f_);
 	if (!f_) {
 		return;
 	}
@@ -30,8 +31,6 @@ LocalFileLoader::LocalFileLoader(const std::string &filename)
 #ifdef __ANDROID__
 	// Android NDK does not support 64-bit file I/O using C streams
 	// so we fall back onto syscalls
-	fd_ = fileno(f_);
-
 	off64_t off = lseek64(fd_, 0, SEEK_END);
 	filesize_ = off;
 	lseek64(fd_, 0, SEEK_SET);
@@ -73,23 +72,6 @@ std::string LocalFileLoader::Path() const {
 	return filename_;
 }
 
-void LocalFileLoader::Seek(s64 absolutePos) {
-#ifdef __ANDROID__
-	lseek64(fd_, absolutePos, SEEK_SET);
-#else
-	fseeko(f_, absolutePos, SEEK_SET);
-#endif
-}
-
-size_t LocalFileLoader::Read(size_t bytes, size_t count, void *data, Flags flags) {
-#ifdef __ANDROID__
-	return read(fd_, data, bytes * count) / bytes;
-#else
-	return fread(data, bytes, count, f_);
-#endif
-}
-
 size_t LocalFileLoader::ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags) {
-	Seek(absolutePos);
+    return pread(fd_, data, bytes * count, absolutePos) / bytes;
-	return Read(bytes, count, data);
 }

Core/FileLoaders/LocalFileLoader.h

@@ -29,9 +29,6 @@ public:
 	virtual bool IsDirectory() override;
 	virtual s64 FileSize() override;
 	virtual std::string Path() const override;
-
-	virtual void Seek(s64 absolutePos) override;
-	virtual size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override;
 	virtual size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override;
 
 private:

Core/FileLoaders/RamCachingFileLoader.cpp

@@ -28,7 +28,7 @@
 
 // Takes ownership of backend.
 RamCachingFileLoader::RamCachingFileLoader(FileLoader *backend)
-	: filesize_(0), filepos_(0), backend_(backend), exists_(-1), isDirectory_(-1), aheadThread_(false) {
+	: filesize_(0), backend_(backend), exists_(-1), isDirectory_(-1), aheadThread_(false) {
 	filesize_ = backend->FileSize();
 	if (filesize_ > 0) {
 		InitCache();
@@ -45,7 +45,6 @@ RamCachingFileLoader::~RamCachingFileLoader() {
 
 bool RamCachingFileLoader::Exists() {
 	if (exists_ == -1) {
-		std::lock_guard<std::mutex> guard(backendMutex_);
 		exists_ = backend_->Exists() ? 1 : 0;
 	}
 	return exists_ == 1;
@@ -53,7 +52,6 @@ bool RamCachingFileLoader::Exists() {
 
 bool RamCachingFileLoader::ExistsFast() {
 	if (exists_ == -1) {
-		std::lock_guard<std::mutex> guard(backendMutex_);
 		return backend_->ExistsFast();
 	}
 	return exists_ == 1;
@@ -61,7 +59,6 @@ bool RamCachingFileLoader::ExistsFast() {
 
 bool RamCachingFileLoader::IsDirectory() {
 	if (isDirectory_ == -1) {
-		std::lock_guard<std::mutex> guard(backendMutex_);
 		isDirectory_ = backend_->IsDirectory() ? 1 : 0;
 	}
 	return isDirectory_ == 1;
@@ -72,18 +69,12 @@ s64 RamCachingFileLoader::FileSize() {
 }
 
 std::string RamCachingFileLoader::Path() const {
-	std::lock_guard<std::mutex> guard(backendMutex_);
 	return backend_->Path();
 }
 
-void RamCachingFileLoader::Seek(s64 absolutePos) {
-	filepos_ = absolutePos;
-}
-
 size_t RamCachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags) {
 	size_t readSize = 0;
 	if (cache_ == nullptr || (flags & Flags::HINT_UNCACHED) != 0) {
-		std::lock_guard<std::mutex> guard(backendMutex_);
 		readSize = backend_->ReadAt(absolutePos, bytes, data, flags);
 	} else {
 		readSize = ReadFromCache(absolutePos, bytes, data);
@@ -100,8 +91,6 @@ size_t RamCachingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, F
 
 		StartReadAhead(absolutePos + readSize);
 	}
-
-	filepos_ = absolutePos + readSize;
 	return readSize;
 }
 
@@ -195,10 +184,8 @@ void RamCachingFileLoader::SaveIntoCache(s64 pos, size_t bytes, Flags flags) {
 		}
 	}
 
-	backendMutex_.lock();
 	s64 cacheFilePos = cacheStartPos << BLOCK_SHIFT;
 	size_t bytesRead = backend_->ReadAt(cacheFilePos, blocksToRead << BLOCK_SHIFT, &cache_[cacheFilePos], flags);
-	backendMutex_.unlock();
 
 	// In case there was an error, let's not mark blocks that failed to read as read.
 	u32 blocksActuallyRead = (u32)((bytesRead + BLOCK_SIZE - 1) >> BLOCK_SHIFT);

Core/FileLoaders/RamCachingFileLoader.h

@@ -34,13 +34,6 @@ public:
 	s64 FileSize() override;
 	std::string Path() const override;
 
-	void Seek(s64 absolutePos) override;
-	size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, count, data, flags);
-	}
-	size_t Read(size_t bytes, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, data, flags);
-	}
 	size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
 		return ReadAt(absolutePos, bytes * count, data, flags) / bytes;
 	}
@@ -63,7 +56,6 @@ private:
 	};
 
 	s64 filesize_;
-	s64 filepos_;
 	FileLoader *backend_;
 	u8 *cache_;
 	int exists_;
@@ -71,7 +63,6 @@ private:
 
 	std::vector<u8> blocks_;
 	std::mutex blocksMutex_;
-	mutable std::mutex backendMutex_;
 	u32 aheadRemaining_;
 	s64 aheadPos_;
 	bool aheadThread_;

Core/FileLoaders/RetryingFileLoader.cpp

@@ -19,7 +19,7 @@
 
 // Takes ownership of backend.
 RetryingFileLoader::RetryingFileLoader(FileLoader *backend)
-	: filepos_(0), backend_(backend) {
+	: backend_(backend) {
 }
 
 RetryingFileLoader::~RetryingFileLoader() {
@@ -60,10 +60,6 @@ std::string RetryingFileLoader::Path() const {
 	return backend_->Path();
 }
 
-void RetryingFileLoader::Seek(s64 absolutePos) {
-	filepos_ = absolutePos;
-}
-
 size_t RetryingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags) {
 	size_t readSize = backend_->ReadAt(absolutePos, bytes, data, flags);
 
@@ -74,6 +70,5 @@ size_t RetryingFileLoader::ReadAt(s64 absolutePos, size_t bytes, void *data, Fla
 		++retries;
 	}
 
-	filepos_ = absolutePos + readSize;
 	return readSize;
 }

Core/FileLoaders/RetryingFileLoader.h

@@ -31,13 +31,6 @@ public:
 	s64 FileSize() override;
 	std::string Path() const override;
 
-	void Seek(s64 absolutePos) override;
-	size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, count, data, flags);
-	}
-	size_t Read(size_t bytes, void *data, Flags flags = Flags::NONE) override {
-		return ReadAt(filepos_, bytes, data, flags);
-	}
 	size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) override {
 		return ReadAt(absolutePos, bytes * count, data, flags) / bytes;
 	}
@@ -48,6 +41,5 @@ private:
 		MAX_RETRIES = 3,
 	};
 
-	s64 filepos_;
 	FileLoader *backend_;
 };

Core/FileSystems/BlockDevices.cpp

@@ -38,7 +38,6 @@ BlockDevice *constructBlockDevice(FileLoader *fileLoader) {
 		return nullptr;
 	char buffer[4]{};
 	size_t size = fileLoader->ReadAt(0, 1, 4, buffer);
-	fileLoader->Seek(0);
 	if (size == 4 && !memcmp(buffer, "CISO", 4))
 		return new CISOFileBlockDevice(fileLoader);
 	else if (size == 4 && !memcmp(buffer, "\x00PBP", 4))

Core/Loaders.h

@@ -80,12 +80,6 @@ public:
 			return filename.substr(pos);
 		}
 	}
-
-	virtual void Seek(s64 absolutePos) = 0;
-	virtual size_t Read(size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) = 0;
-	virtual size_t Read(size_t bytes, void *data, Flags flags = Flags::NONE) {
-		return Read(1, bytes, data, flags);
-	}
 	virtual size_t ReadAt(s64 absolutePos, size_t bytes, size_t count, void *data, Flags flags = Flags::NONE) = 0;
 	virtual size_t ReadAt(s64 absolutePos, size_t bytes, void *data, Flags flags = Flags::NONE) {
 		return ReadAt(absolutePos, 1, bytes, data, flags);