From 03124ec3f0d23a28429a91d5096ab5292bff51e6 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Tue, 23 Jun 2015 22:05:32 +1200 Subject: [PATCH 1/4] IPC_HLE: Simplify lifecycle of devices with shared pointers. This also fixes a memory/filehandle leak when loading savestates. --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 99 +++++++++--------------- Source/Core/Core/IPC_HLE/WII_IPC_HLE.h | 7 +- 2 files changed, 42 insertions(+), 64 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index d428573442..56bb610431 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -61,7 +61,7 @@ They will also generate a true or false return for UpdateInterrupts() in WII_IPC namespace WII_IPC_HLE_Interface { -typedef std::map TDeviceMap; +typedef std::map> TDeviceMap; static TDeviceMap g_DeviceMap; // STATE_TO_SAVE @@ -69,9 +69,9 @@ typedef std::map TFileNameMap; #define IPC_MAX_FDS 0x18 #define ES_MAX_COUNT 2 -static IWII_IPC_HLE_Device* g_FdMap[IPC_MAX_FDS]; +static std::shared_ptr g_FdMap[IPC_MAX_FDS]; static bool es_inuse[ES_MAX_COUNT]; -static IWII_IPC_HLE_Device* es_handles[ES_MAX_COUNT]; +static std::shared_ptr es_handles[ES_MAX_COUNT]; typedef std::deque ipc_msg_queue; @@ -107,42 +107,37 @@ void Init() _dbg_assert_msg_(WII_IPC_HLE, g_DeviceMap.empty(), "DeviceMap isn't empty on init"); CWII_IPC_HLE_Device_es::m_ContentFile = ""; - for (IWII_IPC_HLE_Device*& dev : g_FdMap) - { - dev = nullptr; - } - u32 i = 0; // Build hardware devices - g_DeviceMap[i] = new CWII_IPC_HLE_Device_usb_oh1_57e_305(i, "/dev/usb/oh1/57e/305"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_stm_immediate(i, "/dev/stm/immediate"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_stm_eventhook(i, "/dev/stm/eventhook"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_fs(i, "/dev/fs"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/usb/oh1/57e/305"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/stm/immediate"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/stm/eventhook"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/fs"); i++; // IOS allows two ES devices at a time for (u32 j=0; j(i, "/dev/es"); i++; es_inuse[j] = false; } - g_DeviceMap[i] = new CWII_IPC_HLE_Device_di(i, std::string("/dev/di")); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_net_kd_request(i, "/dev/net/kd/request"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_net_kd_time(i, "/dev/net/kd/time"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_net_ncd_manage(i, "/dev/net/ncd/manage"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_net_wd_command(i, "/dev/net/wd/command"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_net_ip_top(i, "/dev/net/ip/top"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_net_ssl(i, "/dev/net/ssl"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_usb_kbd(i, "/dev/usb/kbd"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_sdio_slot0(i, "/dev/sdio/slot0"); i++; - g_DeviceMap[i] = new CWII_IPC_HLE_Device_stub(i, "/dev/sdio/slot1"); i++; + g_DeviceMap[i] = std::make_shared(i, std::string("/dev/di")); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/net/kd/request"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/net/kd/time"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/net/ncd/manage"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/net/wd/command"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/net/ip/top"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/net/ssl"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/usb/kbd"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/sdio/slot0"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/sdio/slot1"); i++; #if defined(__LIBUSB__) || defined(_WIN32) - g_DeviceMap[i] = new CWII_IPC_HLE_Device_hid(i, "/dev/usb/hid"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/usb/hid"); i++; #else - g_DeviceMap[i] = new CWII_IPC_HLE_Device_stub(i, "/dev/usb/hid"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/usb/hid"); i++; #endif - g_DeviceMap[i] = new CWII_IPC_HLE_Device_stub(i, "/dev/usb/oh1"); i++; - g_DeviceMap[i] = new IWII_IPC_HLE_Device(i, "_Unimplemented_Device_"); i++; + g_DeviceMap[i] = std::make_shared(i, "/dev/usb/oh1"); i++; + g_DeviceMap[i] = std::make_shared(i, "_Unimplemented_Device_"); i++; event_enqueue = CoreTiming::RegisterEvent("IPCEvent", EnqueueEvent); } @@ -151,16 +146,15 @@ void Reset(bool _bHard) { CoreTiming::RemoveAllEvents(event_enqueue); - for (IWII_IPC_HLE_Device*& dev : g_FdMap) + for (auto& dev : g_FdMap) { - if (dev != nullptr && !dev->IsHardware()) + if (dev && !dev->IsHardware()) { // close all files and delete their resources dev->Close(0, true); - delete dev; } - dev = nullptr; + dev.reset(); } for (bool& in_use : es_inuse) @@ -174,16 +168,12 @@ void Reset(bool _bHard) { // Force close entry.second->Close(0, true); - - // Hardware should not be deleted unless it is a hard reset - if (_bHard) - delete entry.second; } } if (_bHard) { - g_DeviceMap.erase(g_DeviceMap.begin(), g_DeviceMap.end()); + g_DeviceMap.clear(); } request_queue.clear(); reply_queue.clear(); @@ -202,7 +192,7 @@ void SetDefaultContentFile(const std::string& _rFilename) { if (entry.second && entry.second->GetDeviceName().find("/dev/es") == 0) { - ((CWII_IPC_HLE_Device_es*)entry.second)->LoadWAD(_rFilename); + static_cast(entry.second.get())->LoadWAD(_rFilename); } } } @@ -214,8 +204,7 @@ void ES_DIVerify(u8 *_pTMD, u32 _sz) void SDIO_EventNotify() { - CWII_IPC_HLE_Device_sdio_slot0 *pDevice = - (CWII_IPC_HLE_Device_sdio_slot0*)GetDeviceByName("/dev/sdio/slot0"); + auto pDevice = static_cast(GetDeviceByName("/dev/sdio/slot0").get()); if (pDevice) pDevice->EventNotify(); } @@ -233,7 +222,7 @@ int getFreeDeviceId() return -1; } -IWII_IPC_HLE_Device* GetDeviceByName(const std::string& _rDeviceName) +std::shared_ptr GetDeviceByName(const std::string& _rDeviceName) { for (const auto& entry : g_DeviceMap) { @@ -246,7 +235,7 @@ IWII_IPC_HLE_Device* GetDeviceByName(const std::string& _rDeviceName) return nullptr; } -IWII_IPC_HLE_Device* AccessDeviceByID(u32 _ID) +std::shared_ptr AccessDeviceByID(u32 _ID) { if (g_DeviceMap.find(_ID) != g_DeviceMap.end()) { @@ -257,11 +246,11 @@ IWII_IPC_HLE_Device* AccessDeviceByID(u32 _ID) } // This is called from ExecuteCommand() COMMAND_OPEN_DEVICE -IWII_IPC_HLE_Device* CreateFileIO(u32 _DeviceID, const std::string& _rDeviceName) +std::shared_ptr CreateFileIO(u32 _DeviceID, const std::string& _rDeviceName) { // scan device name and create the right one INFO_LOG(WII_IPC_FILEIO, "IOP: Create FileIO %s", _rDeviceName.c_str()); - return new CWII_IPC_HLE_Device_FileIO(_DeviceID, _rDeviceName); + return std::make_shared(_DeviceID, _rDeviceName); } @@ -297,13 +286,13 @@ void DoState(PointerWrap &p) } else { - g_FdMap[i] = new CWII_IPC_HLE_Device_FileIO(i, ""); + g_FdMap[i] = std::make_shared(i, ""); g_FdMap[i]->DoState(p); } } else { - g_FdMap[i] = nullptr; + g_FdMap[i].reset(); } } @@ -318,7 +307,7 @@ void DoState(PointerWrap &p) } else { - for (IWII_IPC_HLE_Device*& dev : g_FdMap) + for (auto& dev : g_FdMap) { u32 exists = dev ? 1 : 0; p.Do(exists); @@ -354,9 +343,9 @@ void ExecuteCommand(u32 _Address) IPCCommandType Command = static_cast(Memory::Read_U32(_Address)); s32 DeviceID = Memory::Read_U32(_Address + 8); - IWII_IPC_HLE_Device* pDevice = (DeviceID >= 0 && DeviceID < IPC_MAX_FDS) ? g_FdMap[DeviceID] : nullptr; + std::shared_ptr pDevice = (DeviceID >= 0 && DeviceID < IPC_MAX_FDS) ? g_FdMap[DeviceID] : nullptr; - INFO_LOG(WII_IPC_HLE, "-->> Execute Command Address: 0x%08x (code: %x, device: %x) %p", _Address, Command, DeviceID, pDevice); + INFO_LOG(WII_IPC_HLE, "-->> Execute Command Address: 0x%08x (code: %x, device: %x) %p", _Address, Command, DeviceID, pDevice.get()); switch (Command) { @@ -420,11 +409,6 @@ void ExecuteCommand(u32 _Address) { g_FdMap[DeviceID] = pDevice; } - else - { - delete pDevice; - pDevice = nullptr; - } } } else @@ -448,14 +432,7 @@ void ExecuteCommand(u32 _Address) } } - g_FdMap[DeviceID] = nullptr; - - // Don't delete hardware - if (!pDevice->IsHardware()) - { - delete pDevice; - pDevice = nullptr; - } + g_FdMap[DeviceID].reset(); } else { diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h index 31142804b3..7863d678ba 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.h @@ -4,6 +4,7 @@ #pragma once +#include #include #include "Common/CommonTypes.h" @@ -63,10 +64,10 @@ void ES_DIVerify(u8 *_pTMD, u32 _sz); void SDIO_EventNotify(); -IWII_IPC_HLE_Device* CreateFileIO(u32 _DeviceID, const std::string& _rDeviceName); +std::shared_ptr CreateFileIO(u32 _DeviceID, const std::string& _rDeviceName); -IWII_IPC_HLE_Device* GetDeviceByName(const std::string& _rDeviceName); -IWII_IPC_HLE_Device* AccessDeviceByID(u32 _ID); +std::shared_ptr GetDeviceByName(const std::string& _rDeviceName); +std::shared_ptr AccessDeviceByID(u32 _ID); int getFreeDeviceId(); // Update From 8b4734133e9b5b74d8a987c680fb80b53b8ece27 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Tue, 23 Jun 2015 23:08:21 +1200 Subject: [PATCH 2/4] IPC_HLE: Don't open/close file for every single file operation. On the first boot of Pokemon Snap it copies a 144KB file to the Wii's NAND, but it only copies 32 bytes per IPC write call. This was causing dolphin to open and close the same file over 4000 times. This wouldn't usually be a huge issue, except some operating systems (namely Windows) allow 3rd party programs (namely antivirus software) to hook all accesses to the filesystem. This causes the antivirus software to scan the same file for viruses 4000+ times, which is kind of slow. This fix reduces the initial boot time of pokemon snap from 3+ min to just a few seconds. Could also improve other games which write things to NAND. Will break a few things, see the next commit for an improved fix. --- .../IPC_HLE/WII_IPC_HLE_Device_FileIO.cpp | 36 ++++++++++++------- .../Core/IPC_HLE/WII_IPC_HLE_Device_FileIO.h | 3 +- 2 files changed, 25 insertions(+), 14 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 573f698021..b820ecccc2 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 @@ -73,6 +73,7 @@ CWII_IPC_HLE_Device_FileIO::CWII_IPC_HLE_Device_FileIO(u32 _DeviceID, const std: : IWII_IPC_HLE_Device(_DeviceID, _rDeviceName, false) // not a real hardware , m_Mode(0) , m_SeekPos(0) + , m_file() { Common::ReadReplacements(replacements); } @@ -86,6 +87,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(); + // Close always return 0 for success if (_CommandAddress && !_bForce) Memory::Write_U32(0, _CommandAddress + 4); @@ -121,13 +124,15 @@ 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; } -File::IOFile CWII_IPC_HLE_Device_FileIO::OpenFile() +void CWII_IPC_HLE_Device_FileIO::OpenFile() { const char* open_mode = ""; @@ -147,7 +152,7 @@ File::IOFile CWII_IPC_HLE_Device_FileIO::OpenFile() break; } - return File::IOFile(m_filepath, open_mode); + m_file = File::IOFile(m_filepath, open_mode); } IPCCommandResult CWII_IPC_HLE_Device_FileIO::Seek(u32 _CommandAddress) @@ -156,11 +161,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 (auto file = OpenFile()) + if (m_file) { ReturnValue = FS_RESULT_FATAL; - const s32 fileSize = (s32) 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) @@ -204,6 +209,7 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Seek(u32 _CommandAddress) break; } } + m_file.Seek(m_SeekPos, SEEK_SET); } else { @@ -221,7 +227,7 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::Read(u32 _CommandAddress) const u32 Size = Memory::Read_U32(_CommandAddress + 0x10); - if (auto file = OpenFile()) + if (m_file) { if (m_Mode == ISFS_OPEN_WRITE) { @@ -230,9 +236,8 @@ 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()); - file.Seek(m_SeekPos, SEEK_SET); - ReturnValue = (u32)fread(Memory::GetPointer(Address), 1, Size, file.GetHandle()); - if (ReturnValue != Size && ferror(file.GetHandle())) + ReturnValue = (u32)fread(Memory::GetPointer(Address), 1, Size, m_file.GetHandle()); + if (ReturnValue != Size && ferror(m_file.GetHandle())) { ReturnValue = FS_EACCESS; } @@ -259,7 +264,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 (auto file = OpenFile()) + if (m_file) { if (m_Mode == ISFS_OPEN_READ) { @@ -268,8 +273,7 @@ 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()); - file.Seek(m_SeekPos, SEEK_SET); - if (file.WriteBytes(Memory::GetPointer(Address), Size)) + if (m_file.WriteBytes(Memory::GetPointer(Address), Size)) { ReturnValue = Size; m_SeekPos += Size; @@ -299,9 +303,9 @@ IPCCommandResult CWII_IPC_HLE_Device_FileIO::IOCtl(u32 _CommandAddress) { case ISFS_IOCTL_GETFILESTATS: { - if (auto file = OpenFile()) + if (m_file) { - u32 m_FileLength = (u32)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); @@ -337,4 +341,10 @@ void CWII_IPC_HLE_Device_FileIO::DoState(PointerWrap &p) p.Do(m_SeekPos); m_filepath = HLE_IPC_BuildFilename(m_Name); + + 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 45f7b9ed1b..fb03764ae6 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 @@ -28,7 +28,7 @@ public: IPCCommandResult IOCtl(u32 _CommandAddress) override; void DoState(PointerWrap &p) override; - File::IOFile OpenFile(); + void OpenFile(); private: enum @@ -75,4 +75,5 @@ private: u32 m_SeekPos; std::string m_filepath; + File::IOFile m_file; }; From 1d5cda98208e8dcb1a14161299f2490b6cf04b90 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Wed, 24 Jun 2015 03:18:23 +1200 Subject: [PATCH 3/4] 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; }; From 08fcc7bf843f17a2e02bbd3fd0201d5cd89da756 Mon Sep 17 00:00:00 2001 From: Scott Mansell Date: Wed, 24 Jun 2015 04:50:02 +1200 Subject: [PATCH 4/4] IPC_HLE: Cleanup device definitions with templates. Less copy/pasted code will make future modifications easier. --- Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp | 52 +++++++++++++++--------- 1 file changed, 32 insertions(+), 20 deletions(-) diff --git a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp index 56bb610431..2738ad2f7a 100644 --- a/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp +++ b/Source/Core/Core/IPC_HLE/WII_IPC_HLE.cpp @@ -102,42 +102,54 @@ static void EnqueueEvent(u64 userdata, int cycles_late = 0) Update(); } +static u32 num_devices; + +template +std::shared_ptr AddDevice(const char* deviceName) +{ + auto device = std::make_shared(num_devices, deviceName); + g_DeviceMap[num_devices] = device; + num_devices++; + return device; +} + void Init() { _dbg_assert_msg_(WII_IPC_HLE, g_DeviceMap.empty(), "DeviceMap isn't empty on init"); CWII_IPC_HLE_Device_es::m_ContentFile = ""; - u32 i = 0; + num_devices = 0; + // Build hardware devices - g_DeviceMap[i] = std::make_shared(i, "/dev/usb/oh1/57e/305"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/stm/immediate"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/stm/eventhook"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/fs"); i++; + AddDevice("/dev/usb/oh1/57e/305"); + AddDevice("/dev/stm/immediate"); + AddDevice("/dev/stm/eventhook"); + AddDevice("/dev/fs"); // IOS allows two ES devices at a time for (u32 j=0; j(i, "/dev/es"); i++; + es_handles[j] = AddDevice("/dev/es"); es_inuse[j] = false; } - g_DeviceMap[i] = std::make_shared(i, std::string("/dev/di")); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/net/kd/request"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/net/kd/time"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/net/ncd/manage"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/net/wd/command"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/net/ip/top"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/net/ssl"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/usb/kbd"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/sdio/slot0"); i++; - g_DeviceMap[i] = std::make_shared(i, "/dev/sdio/slot1"); i++; + AddDevice("/dev/di"); + AddDevice("/dev/net/kd/request"); + AddDevice("/dev/net/kd/time"); + AddDevice("/dev/net/ncd/manage"); + AddDevice("/dev/net/wd/command"); + AddDevice("/dev/net/ip/top"); + AddDevice("/dev/net/ssl"); + AddDevice("/dev/usb/kbd"); + AddDevice("/dev/sdio/slot0"); + AddDevice("/dev/sdio/slot1"); #if defined(__LIBUSB__) || defined(_WIN32) - g_DeviceMap[i] = std::make_shared(i, "/dev/usb/hid"); i++; + AddDevice("/dev/usb/hid"); #else - g_DeviceMap[i] = std::make_shared(i, "/dev/usb/hid"); i++; + AddDevice("/dev/usb/hid"); #endif - g_DeviceMap[i] = std::make_shared(i, "/dev/usb/oh1"); i++; - g_DeviceMap[i] = std::make_shared(i, "_Unimplemented_Device_"); i++; + AddDevice("/dev/usb/oh1"); + AddDevice("_Unimplemented_Device_"); event_enqueue = CoreTiming::RegisterEvent("IPCEvent", EnqueueEvent); }