Yuzu: kernel/scheduler: Data race in GlobalScheduler::Lock

[ReinUsesLisp]

current_owner can be read without holding a mutex in GlobalScheduler::Lock:
https://github.com/yuzu-emu/yuzu/blob/b8885aa03bed5f9a374163d3fdcb67e59ab4c441/src/core/hle/kernel/scheduler.cpp#L554
while GlobalScheduler::Unlock (holding a mutex) writes to it in:
https://github.com/yuzu-emu/yuzu/blob/b8885aa03bed5f9a374163d3fdcb67e59ab4c441/src/core/hle/kernel/scheduler.cpp#L572

This has been reported by clang's thread sanitizer.

I considered opening an issue to document this bug. I tried fixing it but I don't understand the code to properly fix it.

[garrettgu10]

I can't see the data race potential here, since current_owner is always set to null whenever the lock is available. This implies that a thread can never observe current_owner as itself when it doesn't hold the lock, even without using volatile or atomic variables. Can you please provide more detail? Thanks!

[ReinUsesLisp]

current_owner can be read in the expression current_thread == current_owner without holding a mutex in one thread while current_owner = Core::EmuThreadHandle::InvalidHandle(); is evaluated (holding a mutex) in the other. This means current_owner doesn't have a defined value when read in current_thread == current_owner.

[ReinUsesLisp]

It is still undefined behavior. From cppreference:

A program that has two conflicting evaluations has a data race unless

• both evaluations execute on the same thread or in the same signal handler, or
• both conflicting evaluations are atomic operations (see std::atomic), or
• one of the conflicting evaluations happens-before another (see std::memory_order)

This code doesn't do anything of that.

From a practical standpoint, having this kind of issues doesn't allow us to detect problematic data races in the code through sanitizers because everything is signalling data races.