[Kernel] Thread safety fixes for xfile, xmutant, and xtimer objects
This commit is contained in:
@@ -38,6 +38,7 @@ XFile::~XFile() {
|
||||
X_STATUS XFile::QueryDirectory(X_FILE_DIRECTORY_INFORMATION* out_info,
|
||||
size_t length, const std::string_view file_name,
|
||||
bool restart) {
|
||||
std::lock_guard<std::mutex> lock(file_lock_);
|
||||
assert_not_null(out_info);
|
||||
|
||||
vfs::Entry* entry = nullptr;
|
||||
@@ -91,6 +92,15 @@ X_STATUS XFile::QueryDirectory(X_FILE_DIRECTORY_INFORMATION* out_info,
|
||||
X_STATUS XFile::Read(uint32_t buffer_guest_address, uint32_t buffer_length,
|
||||
uint64_t byte_offset, uint32_t* out_bytes_read,
|
||||
uint32_t apc_context, bool notify_completion) {
|
||||
std::lock_guard<std::mutex> lock(file_lock_);
|
||||
return ReadInternal(buffer_guest_address, buffer_length, byte_offset,
|
||||
out_bytes_read, apc_context, notify_completion);
|
||||
}
|
||||
|
||||
X_STATUS XFile::ReadInternal(uint32_t buffer_guest_address,
|
||||
uint32_t buffer_length, uint64_t byte_offset,
|
||||
uint32_t* out_bytes_read, uint32_t apc_context,
|
||||
bool notify_completion) {
|
||||
if (byte_offset == uint64_t(-1)) {
|
||||
// Read from current position.
|
||||
byte_offset = position_;
|
||||
@@ -184,6 +194,7 @@ X_STATUS XFile::Read(uint32_t buffer_guest_address, uint32_t buffer_length,
|
||||
X_STATUS XFile::ReadScatter(uint32_t segments_guest_address, uint32_t length,
|
||||
uint64_t byte_offset, uint32_t* out_bytes_read,
|
||||
uint32_t apc_context) {
|
||||
std::lock_guard<std::mutex> lock(file_lock_);
|
||||
X_STATUS result = X_STATUS_SUCCESS;
|
||||
|
||||
// segments points to an array of buffer pointers of type
|
||||
@@ -206,12 +217,13 @@ X_STATUS XFile::ReadScatter(uint32_t segments_guest_address, uint32_t length,
|
||||
}
|
||||
|
||||
uint32_t bytes_read = 0;
|
||||
result = Read(read_buffer, read_length,
|
||||
byte_offset ? ((byte_offset != -1 && byte_offset != -2)
|
||||
? byte_offset + read_total
|
||||
: byte_offset)
|
||||
: -1,
|
||||
&bytes_read, apc_context, false);
|
||||
result =
|
||||
ReadInternal(read_buffer, read_length,
|
||||
byte_offset ? ((byte_offset != -1 && byte_offset != -2)
|
||||
? byte_offset + read_total
|
||||
: byte_offset)
|
||||
: -1,
|
||||
&bytes_read, apc_context, false);
|
||||
|
||||
if (result != X_STATUS_SUCCESS) {
|
||||
break;
|
||||
@@ -240,6 +252,7 @@ X_STATUS XFile::ReadScatter(uint32_t segments_guest_address, uint32_t length,
|
||||
X_STATUS XFile::Write(uint32_t buffer_guest_address, uint32_t buffer_length,
|
||||
uint64_t byte_offset, uint32_t* out_bytes_written,
|
||||
uint32_t apc_context) {
|
||||
std::lock_guard<std::mutex> lock(file_lock_);
|
||||
if (byte_offset == uint64_t(-1)) {
|
||||
// Write from current position.
|
||||
byte_offset = position_;
|
||||
@@ -269,7 +282,10 @@ X_STATUS XFile::Write(uint32_t buffer_guest_address, uint32_t buffer_length,
|
||||
return result;
|
||||
}
|
||||
|
||||
X_STATUS XFile::SetLength(size_t length) { return file_->SetLength(length); }
|
||||
X_STATUS XFile::SetLength(size_t length) {
|
||||
std::lock_guard<std::mutex> lock(file_lock_);
|
||||
return file_->SetLength(length);
|
||||
}
|
||||
X_STATUS XFile::Rename(const std::filesystem::path file_path) {
|
||||
entry()->Rename(file_path);
|
||||
return X_STATUS_SUCCESS;
|
||||
|
||||
@@ -10,6 +10,7 @@
|
||||
#ifndef XENIA_KERNEL_XFILE_H_
|
||||
#define XENIA_KERNEL_XFILE_H_
|
||||
|
||||
#include <mutex>
|
||||
#include <string>
|
||||
|
||||
#include "xenia/kernel/xiocompletion.h"
|
||||
@@ -185,9 +186,14 @@ class XFile : public XObject {
|
||||
private:
|
||||
XFile();
|
||||
|
||||
X_STATUS ReadInternal(uint32_t buffer_guest_address, uint32_t buffer_length,
|
||||
uint64_t byte_offset, uint32_t* out_bytes_read,
|
||||
uint32_t apc_context, bool notify_completion);
|
||||
|
||||
vfs::File* file_ = nullptr;
|
||||
std::unique_ptr<threading::Event> async_event_ = nullptr;
|
||||
|
||||
std::mutex file_lock_;
|
||||
std::mutex completion_port_lock_;
|
||||
std::vector<std::pair<uint32_t, object_ref<XIOCompletion>>> completion_ports_;
|
||||
|
||||
|
||||
@@ -59,7 +59,8 @@ bool XMutant::Save(ByteStream* stream) {
|
||||
return false;
|
||||
}
|
||||
|
||||
uint32_t owning_thread_handle = owning_thread_ ? owning_thread_->handle() : 0;
|
||||
XThread* owner = owning_thread_.load();
|
||||
uint32_t owning_thread_handle = owner ? owner->handle() : 0;
|
||||
stream->Write<uint32_t>(owning_thread_handle);
|
||||
XELOGD("XMutant {:08X} (owner: {:08X})", handle(), owning_thread_handle);
|
||||
|
||||
@@ -83,7 +84,8 @@ object_ref<XMutant> XMutant::Restore(KernelState* kernel_state,
|
||||
mutant->owning_thread_ = kernel_state->object_table()
|
||||
->LookupObject<XThread>(owning_thread_handle)
|
||||
.get();
|
||||
mutant->owning_thread_->AcquireMutantOnStartup(retain_object(mutant));
|
||||
mutant->owning_thread_.load()->AcquireMutantOnStartup(
|
||||
retain_object(mutant));
|
||||
}
|
||||
|
||||
return object_ref<XMutant>(mutant);
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef XENIA_KERNEL_XMUTANT_H_
|
||||
#define XENIA_KERNEL_XMUTANT_H_
|
||||
|
||||
#include <atomic>
|
||||
|
||||
#include "xenia/base/threading.h"
|
||||
#include "xenia/kernel/xobject.h"
|
||||
#include "xenia/xbox.h"
|
||||
@@ -42,7 +44,7 @@ class XMutant : public XObject {
|
||||
XMutant();
|
||||
|
||||
std::unique_ptr<xe::threading::Mutant> mutant_;
|
||||
XThread* owning_thread_ = nullptr;
|
||||
std::atomic<XThread*> owning_thread_{nullptr};
|
||||
};
|
||||
|
||||
} // namespace kernel
|
||||
|
||||
@@ -45,6 +45,8 @@ X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms,
|
||||
return X_STATUS_TIMER_RESUME_IGNORED;
|
||||
}
|
||||
|
||||
std::lock_guard<std::mutex> lock(timer_lock_);
|
||||
|
||||
period_ms = Clock::ScaleGuestDurationMillis(period_ms);
|
||||
WinSystemClock::time_point due_tp;
|
||||
if (due_time < 0) {
|
||||
@@ -63,9 +65,13 @@ X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms,
|
||||
callback_routine_arg_ = routine_arg;
|
||||
|
||||
// This callback will only be issued when the timer is fired.
|
||||
// Capture values by value to avoid racing with a future SetTimer() call.
|
||||
std::function<void()> callback = nullptr;
|
||||
if (callback_routine_) {
|
||||
callback = [this]() {
|
||||
auto cb_thread = callback_thread_;
|
||||
auto cb_routine = callback_routine_;
|
||||
auto cb_routine_arg = callback_routine_arg_;
|
||||
callback = [cb_thread, cb_routine, cb_routine_arg]() {
|
||||
// Queue APC to call back routine with (arg, low, high).
|
||||
// It'll be executed on the thread that requested the timer.
|
||||
uint64_t time = xe::Clock::QueryGuestSystemTime();
|
||||
@@ -73,9 +79,8 @@ X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms,
|
||||
uint32_t time_high = static_cast<uint32_t>(time >> 32);
|
||||
XELOGI(
|
||||
"XTimer enqueuing timer callback to {:08X}({:08X}, {:08X}, {:08X})",
|
||||
callback_routine_, callback_routine_arg_, time_low, time_high);
|
||||
callback_thread_->EnqueueApc(callback_routine_, callback_routine_arg_,
|
||||
time_low, time_high);
|
||||
cb_routine, cb_routine_arg, time_low, time_high);
|
||||
cb_thread->EnqueueApc(cb_routine, cb_routine_arg, time_low, time_high);
|
||||
};
|
||||
}
|
||||
|
||||
@@ -91,6 +96,7 @@ X_STATUS XTimer::SetTimer(int64_t due_time, uint32_t period_ms,
|
||||
}
|
||||
|
||||
X_STATUS XTimer::Cancel() {
|
||||
std::lock_guard<std::mutex> lock(timer_lock_);
|
||||
return timer_->Cancel() ? X_STATUS_SUCCESS : X_STATUS_UNSUCCESSFUL;
|
||||
}
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@
|
||||
#ifndef XENIA_KERNEL_XTIMER_H_
|
||||
#define XENIA_KERNEL_XTIMER_H_
|
||||
|
||||
#include <mutex>
|
||||
|
||||
#include "xenia/base/threading.h"
|
||||
#include "xenia/kernel/xobject.h"
|
||||
#include "xenia/xbox.h"
|
||||
@@ -37,6 +39,7 @@ class XTimer : public XObject {
|
||||
|
||||
private:
|
||||
std::unique_ptr<xe::threading::Timer> timer_;
|
||||
std::mutex timer_lock_;
|
||||
|
||||
XThread* callback_thread_ = nullptr;
|
||||
uint32_t callback_routine_ = 0;
|
||||
|
||||
Reference in New Issue
Block a user