[Memory] Close shared memory FD and properly handle its invalid value

This commit is contained in:
Triang3l 2020-11-22 14:17:37 +03:00
parent 432369ae84
commit 86ae42919d
7 changed files with 30 additions and 19 deletions

View File

@ -18,6 +18,7 @@
#include "xenia/base/assert.h" #include "xenia/base/assert.h"
#include "xenia/base/byte_order.h" #include "xenia/base/byte_order.h"
#include "xenia/base/platform.h"
namespace xe { namespace xe {
namespace memory { namespace memory {
@ -96,7 +97,15 @@ void AlignedFree(T* ptr) {
#endif // XE_COMPILER_MSVC #endif // XE_COMPILER_MSVC
} }
#if XE_PLATFORM_WIN32
// HANDLE.
typedef void* FileMappingHandle; typedef void* FileMappingHandle;
constexpr FileMappingHandle kFileMappingHandleInvalid = nullptr;
#else
// File descriptor.
typedef int FileMappingHandle;
constexpr FileMappingHandle kFileMappingHandleInvalid = -1;
#endif
FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path, FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path,
size_t length, PageAccess access, size_t length, PageAccess access,

View File

@ -86,19 +86,19 @@ FileMappingHandle CreateFileMappingHandle(const std::filesystem::path& path,
assert_always(); assert_always();
return nullptr; return nullptr;
} }
oflag |= O_CREAT; oflag |= O_CREAT;
auto full_path = "/" / path; auto full_path = "/" / path;
int ret = shm_open(full_path.c_str(), oflag, 0777); int ret = shm_open(full_path.c_str(), oflag, 0777);
if (ret > 0) { if (ret < 0) {
ftruncate64(ret, length); return kFileMappingHandleInvalid;
} }
ftruncate64(ret, length);
return ret <= 0 ? nullptr : reinterpret_cast<FileMappingHandle>(ret); return ret;
} }
void CloseFileMappingHandle(FileMappingHandle handle, void CloseFileMappingHandle(FileMappingHandle handle,
const std::filesystem::path& path) { const std::filesystem::path& path) {
close(handle);
auto full_path = "/" / path; auto full_path = "/" / path;
shm_unlink(full_path.c_str()); shm_unlink(full_path.c_str());
} }
@ -106,8 +106,8 @@ void CloseFileMappingHandle(FileMappingHandle handle,
void* MapFileView(FileMappingHandle handle, void* base_address, size_t length, void* MapFileView(FileMappingHandle handle, void* base_address, size_t length,
PageAccess access, size_t file_offset) { PageAccess access, size_t file_offset) {
uint32_t prot = ToPosixProtectFlags(access); uint32_t prot = ToPosixProtectFlags(access);
return mmap64(base_address, length, prot, MAP_PRIVATE | MAP_ANONYMOUS, return mmap64(base_address, length, prot, MAP_PRIVATE | MAP_ANONYMOUS, handle,
reinterpret_cast<intptr_t>(handle), file_offset); file_offset);
} }
bool UnmapFileView(FileMappingHandle handle, void* base_address, bool UnmapFileView(FileMappingHandle handle, void* base_address,

View File

@ -421,7 +421,7 @@ TEST_CASE("create_and_close_file_mapping", "Virtual Memory Mapping") {
auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount());
auto memory = xe::memory::CreateFileMappingHandle( auto memory = xe::memory::CreateFileMappingHandle(
path, 0x100, xe::memory::PageAccess::kReadWrite, true); path, 0x100, xe::memory::PageAccess::kReadWrite, true);
REQUIRE(memory); REQUIRE(memory != xe::memory::FileMappingHandleInvalid);
xe::memory::CloseFileMappingHandle(memory, path); xe::memory::CloseFileMappingHandle(memory, path);
} }
@ -430,7 +430,7 @@ TEST_CASE("map_view", "Virtual Memory Mapping") {
const size_t length = 0x100; const size_t length = 0x100;
auto memory = xe::memory::CreateFileMappingHandle( auto memory = xe::memory::CreateFileMappingHandle(
path, length, xe::memory::PageAccess::kReadWrite, true); path, length, xe::memory::PageAccess::kReadWrite, true);
REQUIRE(memory); REQUIRE(memory != xe::memory::FileMappingHandleInvalid);
uintptr_t address = 0x100000000; uintptr_t address = 0x100000000;
auto view = auto view =
@ -447,7 +447,7 @@ TEST_CASE("read_write_view", "Virtual Memory Mapping") {
auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount()); auto path = fmt::format("xenia_test_{}", Clock::QueryHostTickCount());
auto memory = xe::memory::CreateFileMappingHandle( auto memory = xe::memory::CreateFileMappingHandle(
path, length, xe::memory::PageAccess::kReadWrite, true); path, length, xe::memory::PageAccess::kReadWrite, true);
REQUIRE(memory); REQUIRE(memory != xe::memory::FileMappingHandleInvalid);
uintptr_t address = 0x100000000; uintptr_t address = 0x100000000;
auto view = auto view =

View File

@ -40,11 +40,11 @@ X64CodeCache::~X64CodeCache() {
} }
// Unmap all views and close mapping. // Unmap all views and close mapping.
if (mapping_) { if (mapping_ != xe::memory::kFileMappingHandleInvalid) {
xe::memory::UnmapFileView(mapping_, generated_code_base_, xe::memory::UnmapFileView(mapping_, generated_code_base_,
kGeneratedCodeSize); kGeneratedCodeSize);
xe::memory::CloseFileMappingHandle(mapping_, file_name_); xe::memory::CloseFileMappingHandle(mapping_, file_name_);
mapping_ = nullptr; mapping_ = xe::memory::kFileMappingHandleInvalid;
} }
} }
@ -67,7 +67,7 @@ bool X64CodeCache::Initialize() {
mapping_ = xe::memory::CreateFileMappingHandle( mapping_ = xe::memory::CreateFileMappingHandle(
file_name_, kGeneratedCodeSize, xe::memory::PageAccess::kExecuteReadWrite, file_name_, kGeneratedCodeSize, xe::memory::PageAccess::kExecuteReadWrite,
false); false);
if (!mapping_) { if (mapping_ == xe::memory::kFileMappingHandleInvalid) {
XELOGE("Unable to create code cache mmap"); XELOGE("Unable to create code cache mmap");
return false; return false;
} }

View File

@ -100,7 +100,8 @@ class X64CodeCache : public CodeCache {
UnwindReservation unwind_reservation) {} UnwindReservation unwind_reservation) {}
std::filesystem::path file_name_; std::filesystem::path file_name_;
xe::memory::FileMappingHandle mapping_ = nullptr; xe::memory::FileMappingHandle mapping_ =
xe::memory::kFileMappingHandleInvalid;
// NOTE: the global critical region must be held when manipulating the offsets // NOTE: the global critical region must be held when manipulating the offsets
// or counts of anything, to keep the tables consistent and ordered. // or counts of anything, to keep the tables consistent and ordered.

View File

@ -113,11 +113,11 @@ Memory::~Memory() {
heaps_.physical.Dispose(); heaps_.physical.Dispose();
// Unmap all views and close mapping. // Unmap all views and close mapping.
if (mapping_) { if (mapping_ != xe::memory::kFileMappingHandleInvalid) {
UnmapViews(); UnmapViews();
xe::memory::CloseFileMappingHandle(mapping_, file_name_); xe::memory::CloseFileMappingHandle(mapping_, file_name_);
mapping_base_ = nullptr; mapping_base_ = nullptr;
mapping_ = nullptr; mapping_ = xe::memory::kFileMappingHandleInvalid;
} }
virtual_membase_ = nullptr; virtual_membase_ = nullptr;
@ -133,9 +133,9 @@ bool Memory::Initialize() {
file_name_, file_name_,
// entire 4gb space + 512mb physical: // entire 4gb space + 512mb physical:
0x11FFFFFFF, xe::memory::PageAccess::kReadWrite, false); 0x11FFFFFFF, xe::memory::PageAccess::kReadWrite, false);
if (!mapping_) { if (mapping_ == xe::memory::kFileMappingHandleInvalid) {
XELOGE("Unable to reserve the 4gb guest address space."); XELOGE("Unable to reserve the 4gb guest address space.");
assert_not_null(mapping_); assert_always();
return false; return false;
} }

View File

@ -491,7 +491,8 @@ class Memory {
uint8_t* virtual_membase_ = nullptr; uint8_t* virtual_membase_ = nullptr;
uint8_t* physical_membase_ = nullptr; uint8_t* physical_membase_ = nullptr;
xe::memory::FileMappingHandle mapping_ = nullptr; xe::memory::FileMappingHandle mapping_ =
xe::memory::kFileMappingHandleInvalid;
uint8_t* mapping_base_ = nullptr; uint8_t* mapping_base_ = nullptr;
union { union {
struct { struct {