Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Core: Assert that only the Host thread may call PauseAndLock(). #11873

Merged
merged 1 commit into from Jun 6, 2023

Conversation

AdmiralCurtiss
Copy link
Contributor

Core's PauseAndLock() states that // WARNING: PauseAndLock is not fully threadsafe so is only valid on the Host Thread, but there's absolutely no protection against doing this.

This adds an ASSERT() that we are indeed the host thread. To do that, this moves DolphinQt's IsHostThread() function into Core, and adds relevant DeclareAsHostThread() calls to all entry points.

This may expose erroneous usages of Core::RunAsCPUThread() and similar functions, which if found should be fixed before this is merged.

Copy link
Contributor

@iwubcode iwubcode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pretty neat idea. LGTM

@JosJuice JosJuice merged commit 2d56daf into dolphin-emu:master Jun 6, 2023
14 checks passed
@rpaulucci3
Copy link

Hi. Sorry if this is not the correct place to post this, but the assertion seems to introduce an error right after connecting a real Wii Remote on Windows. This happens in build 5.0-19548, as shown in the screenshot.
dolphin-error-qt

@AdmiralCurtiss AdmiralCurtiss deleted the pause-and-lock-host branch June 7, 2023 01:21
@AdmiralCurtiss
Copy link
Contributor Author

AdmiralCurtiss commented Jun 7, 2023

Yeah I'm not surprised to see a few of those pop up, we'll fix them as they get reported or run into them ourselves.

More repro notes: No game has to be running, Continous Scanning has to be enabled. The Wiimote Scanning thread is quite explicitly calling RunAsCPUThread, that's definitely evil.

Core::RunAsCPUThread([i, &wm] {
g_wiimotes[i] = std::move(wm);
WiimoteCommon::UpdateSource(i);
});

@rpaulucci3
Copy link

I'm not at all familiar with the Dolphin codebase, but if this is a relatively simple fix, I'd be happy to try my hand at helping address it whenever possible :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants