From 1d5cda98208e8dcb1a14161299f2490b6cf04b90 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Wed, 24 Jun 2015 03:18:23 +1200 Subject: [PATCH] IPC_HLE: Reimplement fix for issues 2917/5232 with more sanity. Instead of opening... and... closing... files... after... every... file... operation... (which can be slow, especially if a virus scanner is running), we catch attempts to open the same file twice and only open one copy of the file. --- .../IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp | 80 ++++++++++++------- .../Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h | 2 +- 2 files changed, 52 insertions(+), 30 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp index b820ecccc2..23067f0dbb 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp @@ -19,6 +19,8 @@ static Common::replace_v replacements; +static std::map > openFiles; + // This is used by several of the FileIO and /dev/fs functions std::string HLE_IPC_BuildFilename(std::string path_wii) { @@ -87,7 +89,8 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Close(u32 _CommandAddress, bool _bF INFO_LOG(WII_IPC_FILEIO, "FileIO: Close %s (DeviceID=%08x)", m_Name.c_str(), m_DeviceID); m_Mode = 0; - m_file.Close(); + // Let go of our pointer to the file, it will automatically close if we are the last handle accessing it. + m_file.reset(); // Close always return 0 for success if (_CommandAddress && !_bForce) @@ -116,6 +119,7 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Open(u32 _CommandAddress, u32 _Mode if (File::Exists(m_filepath) && !File::IsDirectory(m_filepath)) { INFO_LOG(WII_IPC_FILEIO, "FileIO: Open %s (%s == %08X)", m_Name.c_str(), Modes[_Mode], _Mode); + OpenFile(); ReturnValue = m_DeviceID; } else @@ -124,35 +128,53 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Open(u32 _CommandAddress, u32 _Mode ReturnValue = FS_FILE_NOT_EXIST; } - OpenFile(); - if (_CommandAddress) Memory::Write_U32(ReturnValue, _CommandAddress+4); m_Active = true; return IPC_DEFAULT_REPLY; } +// This isn't theadsafe, but it's only called from the CPU thread. void CWII_IPC_HLE_Device_FileIO::OpenFile() { - const char* open_mode = ""; + // On the wii, all file operations are strongly ordered. + // If a game opens the same file twice (or 8 times, looking at you PokePark Wii) + // and writes to one file handle, it will be able to immediately read the written + // data from the other handle. + // On 'real' operating systems, there are various buffers and caches meaning + // applications doing such naughty things will not get expected results. - switch (m_Mode) + // So we fix this by catching any attempts to open the same file twice and + // only opening one file. Accesses to a single file handle are ordered. + // + // Hall of Shame: + // - PokePark Wii (gets stuck on the loading screen of Pikachu falling) + // - PokePark 2 (Also gets stuck while loading) + // - Wii System Menu (Can't access the system settings, gets stuck on blank screen) + // - The Beatles: Rock Band (saving doesn't work) + + // Check if the file has already been opened. + auto search = openFiles.find(m_Name); + if (search != openFiles.end()) { - case ISFS_OPEN_READ: - open_mode = "rb"; - break; - - case ISFS_OPEN_WRITE: - case ISFS_OPEN_RW: - open_mode = "r+b"; - break; - - default: - PanicAlert("FileIO: Unknown open mode : 0x%02x", m_Mode); - break; + m_file = search->second.lock(); // Lock a shared pointer to use. } + else + { + std::string path = m_Name; + // This code will be called when all references to the shared pointer below have been removed. + auto deleter = [path](File::IOFile* ptr) + { + delete ptr; // IOFile's deconstructor closes the file. + openFiles.erase(path); // erase the weak pointer from the list of open files. + }; - m_file = File::IOFile(m_filepath, open_mode); + // All files are opened read/write. Actual access rights will be controlled per handle by the read/write functions below + m_file = std::shared_ptr(new File::IOFile(m_filepath, "r+b"), deleter); // Use the custom deleter from above. + + // Store a weak pointer to our newly opened file in the cache. + openFiles[path] = std::weak_ptr(m_file); + } } IPCCommandResult CWII_IPC_HLE_Device_FileIO::Seek(u32 _CommandAddress) @@ -161,11 +183,11 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Seek(u32 _CommandAddress) const s32 SeekPosition = Memory::Read_U32(_CommandAddress + 0xC); const s32 Mode = Memory::Read_U32(_CommandAddress + 0x10); - if (m_file) + if (m_file->IsOpen()) { ReturnValue = FS_RESULT_FATAL; - const s32 fileSize = (s32) m_file.GetSize(); + const s32 fileSize = (s32) m_file->GetSize(); INFO_LOG(WII_IPC_FILEIO, "FileIO: Seek Pos: 0x%08x, Mode: %i (%s, Length=0x%08x)", SeekPosition, Mode, m_Name.c_str(), fileSize); switch (Mode) @@ -209,7 +231,6 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Seek(u32 _CommandAddress) break; } } - m_file.Seek(m_SeekPos, SEEK_SET); } else { @@ -227,7 +248,7 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Read(u32 _CommandAddress) const u32 Size = Memory::Read_U32(_CommandAddress + 0x10); - if (m_file) + if (m_file->IsOpen()) { if (m_Mode == ISFS_OPEN_WRITE) { @@ -236,8 +257,9 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Read(u32 _CommandAddress) else { INFO_LOG(WII_IPC_FILEIO, "FileIO: Read 0x%x bytes to 0x%08x from %s", Size, Address, m_Name.c_str()); - ReturnValue = (u32)fread(Memory::GetPointer(Address), 1, Size, m_file.GetHandle()); - if (ReturnValue != Size && ferror(m_file.GetHandle())) + m_file->Seek(m_SeekPos, SEEK_SET); // File might be opened twice, need to seek before we read + ReturnValue = (u32)fread(Memory::GetPointer(Address), 1, Size, m_file->GetHandle()); + if (ReturnValue != Size && ferror(m_file->GetHandle())) { ReturnValue = FS_EACCESS; } @@ -264,7 +286,7 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Write(u32 _CommandAddress) const u32 Address = Memory::Read_U32(_CommandAddress + 0xC); // Write data from this memory address const u32 Size = Memory::Read_U32(_CommandAddress + 0x10); - if (m_file) + if (m_file->IsOpen()) { if (m_Mode == ISFS_OPEN_READ) { @@ -273,7 +295,8 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Write(u32 _CommandAddress) else { INFO_LOG(WII_IPC_FILEIO, "FileIO: Write 0x%04x bytes from 0x%08x to %s", Size, Address, m_Name.c_str()); - if (m_file.WriteBytes(Memory::GetPointer(Address), Size)) + m_file->Seek(m_SeekPos, SEEK_SET); // File might be opened twice, need to seek before we write + if (m_file->WriteBytes(Memory::GetPointer(Address), Size)) { ReturnValue = Size; m_SeekPos += Size; @@ -303,9 +326,9 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::IOCtl(u32 _CommandAddress) { case ISFS_IOCTL_GETFILESTATS: { - if (m_file) + if (m_file->IsOpen()) { - u32 m_FileLength = (u32)m_file.GetSize(); + u32 m_FileLength = (u32)m_file->GetSize(); const u32 BufferOut = Memory::Read_U32(_CommandAddress + 0x18); INFO_LOG(WII_IPC_FILEIO, " File: %s, Length: %i, Pos: %i", m_Name.c_str(), m_FileLength, m_SeekPos); @@ -345,6 +368,5 @@ void CWII_IPC_HLE_Device_FileIO::DoState(PointerWrap &p) if (p.GetMode() == PointerWrap::MODE_READ) { OpenFile(); - m_file.Seek(m_SeekPos, SEEK_SET); } } diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h index fb03764ae6..c8715a4a86 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h @@ -75,5 +75,5 @@ private: u32 m_SeekPos; std::string m_filepath; - File::IOFile m_file; + std::shared_ptr m_file; };