From 8e41d5a7074f405fc0d44c3546ac3e1206254452 Mon Sep 17 00:00:00 2001 From: "Dr. Chat" Date: Wed, 22 Jul 2015 19:34:33 -0500 Subject: [PATCH] The kernel object table now keeps track of handle reference counts. --- src/xenia/kernel/object_table.cc | 64 +++++++++++++++++++++++++---- src/xenia/kernel/object_table.h | 13 ++++-- src/xenia/kernel/objects/xthread.cc | 3 ++ src/xenia/kernel/xboxkrnl_ob.cc | 3 +- src/xenia/kernel/xobject.cc | 14 +++---- src/xenia/kernel/xobject.h | 1 - 6 files changed, 75 insertions(+), 23 deletions(-) diff --git a/src/xenia/kernel/object_table.cc b/src/xenia/kernel/object_table.cc index 50b1bcdb6..1e27eda3f 100644 --- a/src/xenia/kernel/object_table.cc +++ b/src/xenia/kernel/object_table.cc @@ -22,13 +22,12 @@ ObjectTable::ObjectTable() : table_capacity_(0), table_(nullptr), last_free_entry_(0) {} ObjectTable::~ObjectTable() { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); // Release all objects. for (uint32_t n = 0; n < table_capacity_; n++) { ObjectTableEntry& entry = table_[n]; if (entry.object) { - entry.object->ReleaseHandle(); entry.object->Release(); } } @@ -90,7 +89,7 @@ X_STATUS ObjectTable::AddHandle(XObject* object, X_HANDLE* out_handle) { uint32_t slot = 0; { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); // Find a free slot. result = FindFreeSlot(&slot); @@ -99,9 +98,9 @@ X_STATUS ObjectTable::AddHandle(XObject* object, X_HANDLE* out_handle) { if (XSUCCEEDED(result)) { ObjectTableEntry& entry = table_[slot]; entry.object = object; + entry.handle_ref_count = 1; // Retain so long as the object is in the table. - object->RetainHandle(); object->Retain(); } } @@ -128,6 +127,36 @@ X_STATUS ObjectTable::DuplicateHandle(X_HANDLE handle, X_HANDLE* out_handle) { return result; } +X_STATUS ObjectTable::RetainHandle(X_HANDLE handle) { + std::lock_guard lock(table_mutex_); + + ObjectTableEntry* entry = LookupTable(handle); + if (!entry) { + return X_STATUS_INVALID_HANDLE; + } + + entry->handle_ref_count++; + return X_STATUS_SUCCESS; +} + +X_STATUS ObjectTable::ReleaseHandle(X_HANDLE handle) { + std::lock_guard lock(table_mutex_); + + ObjectTableEntry* entry = LookupTable(handle); + if (!entry) { + return X_STATUS_INVALID_HANDLE; + } + + if (--entry->handle_ref_count == 0) { + // No more references. Remove it from the table. + return RemoveHandle(handle); + } + + // FIXME: Return a status code telling the caller it wasn't released + // (but not a failure code) + return X_STATUS_SUCCESS; +} + X_STATUS ObjectTable::RemoveHandle(X_HANDLE handle) { X_STATUS result = X_STATUS_SUCCESS; @@ -138,7 +167,7 @@ X_STATUS ObjectTable::RemoveHandle(X_HANDLE handle) { XObject* object = NULL; { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); // Lower 2 bits are ignored. uint32_t slot = handle >> 2; @@ -167,6 +196,23 @@ X_STATUS ObjectTable::RemoveHandle(X_HANDLE handle) { return result; } +ObjectTable::ObjectTableEntry* ObjectTable::LookupTable(X_HANDLE handle) { + handle = TranslateHandle(handle); + if (!handle) { + return nullptr; + } + + std::lock_guard lock(table_mutex_); + + // Lower 2 bits are ignored. + uint32_t slot = handle >> 2; + if (slot <= table_capacity_) { + return &table_[slot]; + } + + return nullptr; +} + XObject* ObjectTable::LookupObject(X_HANDLE handle, bool already_locked) { handle = TranslateHandle(handle); if (!handle) { @@ -203,7 +249,7 @@ XObject* ObjectTable::LookupObject(X_HANDLE handle, bool already_locked) { void ObjectTable::GetObjectsByType(XObject::Type type, std::vector>& results) { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); for (uint32_t slot = 0; slot < table_capacity_; ++slot) { auto& entry = table_[slot]; if (entry.object) { @@ -229,7 +275,7 @@ X_HANDLE ObjectTable::TranslateHandle(X_HANDLE handle) { } X_STATUS ObjectTable::AddNameMapping(const std::string& name, X_HANDLE handle) { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); if (name_table_.count(name)) { return X_STATUS_OBJECT_NAME_COLLISION; } @@ -238,7 +284,7 @@ X_STATUS ObjectTable::AddNameMapping(const std::string& name, X_HANDLE handle) { } void ObjectTable::RemoveNameMapping(const std::string& name) { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); auto it = name_table_.find(name); if (it != name_table_.end()) { name_table_.erase(it); @@ -247,7 +293,7 @@ void ObjectTable::RemoveNameMapping(const std::string& name) { X_STATUS ObjectTable::GetObjectByName(const std::string& name, X_HANDLE* out_handle) { - std::lock_guard lock(table_mutex_); + std::lock_guard lock(table_mutex_); auto it = name_table_.find(name); if (it == name_table_.end()) { *out_handle = X_INVALID_HANDLE_VALUE; diff --git a/src/xenia/kernel/object_table.h b/src/xenia/kernel/object_table.h index 340a01b9a..fd47f9139 100644 --- a/src/xenia/kernel/object_table.h +++ b/src/xenia/kernel/object_table.h @@ -28,7 +28,10 @@ class ObjectTable { X_STATUS AddHandle(XObject* object, X_HANDLE* out_handle); X_STATUS DuplicateHandle(X_HANDLE orig, X_HANDLE* out_handle); + X_STATUS RetainHandle(X_HANDLE handle); + X_STATUS ReleaseHandle(X_HANDLE handle); X_STATUS RemoveHandle(X_HANDLE handle); + template object_ref LookupObject(X_HANDLE handle) { auto object = LookupObject(handle, false); @@ -48,6 +51,12 @@ class ObjectTable { } private: + typedef struct { + int handle_ref_count = 0; + XObject* object = nullptr; + } ObjectTableEntry; + + ObjectTableEntry* LookupTable(X_HANDLE handle); XObject* LookupObject(X_HANDLE handle, bool already_locked); void GetObjectsByType(XObject::Type type, std::vector>& results); @@ -55,9 +64,7 @@ class ObjectTable { X_HANDLE TranslateHandle(X_HANDLE handle); X_STATUS FindFreeSlot(uint32_t* out_slot); - typedef struct { XObject* object; } ObjectTableEntry; - - xe::mutex table_mutex_; + xe::recursive_mutex table_mutex_; uint32_t table_capacity_; ObjectTableEntry* table_; uint32_t last_free_entry_; diff --git a/src/xenia/kernel/objects/xthread.cc b/src/xenia/kernel/objects/xthread.cc index 0df954287..2add67891 100644 --- a/src/xenia/kernel/objects/xthread.cc +++ b/src/xenia/kernel/objects/xthread.cc @@ -292,6 +292,7 @@ X_STATUS XThread::Create() { // Always retain when starting - the thread owns itself until exited. Retain(); + RetainHandle(); xe::threading::Thread::CreationParameters params; params.stack_size = 16 * 1024 * 1024; // Ignore game, always big! @@ -350,6 +351,7 @@ X_STATUS XThread::Exit(int exit_code) { running_ = false; Release(); + ReleaseHandle(); // NOTE: this does not return! xe::threading::Thread::Exit(exit_code); @@ -361,6 +363,7 @@ X_STATUS XThread::Terminate(int exit_code) { running_ = false; Release(); + ReleaseHandle(); thread_->Terminate(exit_code); return X_STATUS_SUCCESS; diff --git a/src/xenia/kernel/xboxkrnl_ob.cc b/src/xenia/kernel/xboxkrnl_ob.cc index 7bf99bdb5..6846dff9f 100644 --- a/src/xenia/kernel/xboxkrnl_ob.cc +++ b/src/xenia/kernel/xboxkrnl_ob.cc @@ -190,8 +190,7 @@ dword_result_t NtDuplicateObject(dword_t handle, lpdword_t new_handle_ptr, DECLARE_XBOXKRNL_EXPORT(NtDuplicateObject, ExportTag::kImplemented); dword_result_t NtClose(dword_t handle) { - // FIXME: This needs to be removed once handle count reaches 0! - return kernel_state()->object_table()->RemoveHandle(handle); + return kernel_state()->object_table()->ReleaseHandle(handle); } DECLARE_XBOXKRNL_EXPORT(NtClose, ExportTag::kImplemented); diff --git a/src/xenia/kernel/xobject.cc b/src/xenia/kernel/xobject.cc index 2fb798dcd..5338e4359 100644 --- a/src/xenia/kernel/xobject.cc +++ b/src/xenia/kernel/xobject.cc @@ -21,7 +21,6 @@ namespace kernel { XObject::XObject(KernelState* kernel_state, Type type) : kernel_state_(kernel_state), - handle_ref_count_(0), pointer_ref_count_(1), type_(type), handle_(X_INVALID_HANDLE_VALUE), @@ -34,7 +33,6 @@ XObject::XObject(KernelState* kernel_state, Type type) } XObject::~XObject() { - assert_zero(handle_ref_count_); assert_zero(pointer_ref_count_); if (allocated_guest_object_) { @@ -58,20 +56,20 @@ XObject::Type XObject::type() { return type_; } X_HANDLE XObject::handle() const { return handle_; } -void XObject::RetainHandle() { ++handle_ref_count_; } +void XObject::RetainHandle() { + kernel_state_->object_table()->RetainHandle(handle_); +} bool XObject::ReleaseHandle() { - if (--handle_ref_count_ == 0) { - return true; - } - return false; + // FIXME: Return true when handle is actually released. + return kernel_state_->object_table()->ReleaseHandle(handle_) == + X_STATUS_SUCCESS; } void XObject::Retain() { ++pointer_ref_count_; } void XObject::Release() { if (--pointer_ref_count_ == 0) { - assert_true(pointer_ref_count_ >= handle_ref_count_); delete this; } } diff --git a/src/xenia/kernel/xobject.h b/src/xenia/kernel/xobject.h index 9017422e8..70bdc4e4c 100644 --- a/src/xenia/kernel/xobject.h +++ b/src/xenia/kernel/xobject.h @@ -180,7 +180,6 @@ class XObject { KernelState* kernel_state_; private: - std::atomic handle_ref_count_; std::atomic pointer_ref_count_; Type type_;