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

InputCommon/ControllerEmu: Use more locks #10712

Merged
merged 1 commit into from Jul 5, 2022

Conversation

TellowKrinkle
Copy link
Contributor

Loading configs while another thread is messing with stuff just doesn't feel like a good idea. It's hard to tell how much LoadConfig and UpdateReferences overlap in object usage, but it's definitely not zero because Thread Sanitizer keeps complaining about things.
Hopefully fixes Wiimote Scanning Thread crashes on startup (where main thread calls LoadConfig while wiimote scanning thread is in UpdateReferences)

Loading configs while another thread is messing with stuff just doesn't feel like a good idea
Hopefully fixes Wiimote Scanning Thread crashes on startup
@JMC47
Copy link
Contributor

JMC47 commented Jun 1, 2022

@dolphin-emu-bot rebuild

@AdmiralCurtiss
Copy link
Contributor

I... don't quite understand when the state lock is supposed to be held. The comments on it say that it's for when you call State()/GetState() but it's clearly for more than that if you check where it's all used. Is it for protecting modifications to anything within the groups vector while another thread is reading from it? @jordan-woyak I'm guessing you probably know this.

@jordan-woyak
Copy link
Member

jordan-woyak commented Jun 21, 2022

I'm not even sure any more. The code has changed a lot. I was talking with someone about caching control reference states each frame in UpdateInput or similar so every GetState call wouldn't need to reach into containers and devices. This would probably make things way more manageable.

Copy link
Contributor

@AdmiralCurtiss AdmiralCurtiss left a comment

Choose a reason for hiding this comment

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

I think this is fine. It would probably make more sense to have more granular locks (or a better multithreaded design in general), but this doesn't really make the situation any worse than it is so yeah.

@AdmiralCurtiss AdmiralCurtiss merged commit 24498ca into dolphin-emu:master Jul 5, 2022
10 checks passed
@TellowKrinkle TellowKrinkle deleted the ControllerLocks branch July 8, 2022 04:13
@iJunkie22
Copy link

Hopefully fixes Wiimote Scanning Thread crashes on startup (where main thread calls LoadConfig while wiimote scanning thread is in UpdateReferences)

Can confirm that the bug I repeatedly experienced up to and in the 5.0-16380 beta before this commit where launching a game on macOS with a DS4 connected/configured using bluetooth would cause the WiiMote Scanning Thread to crash, is no longer happening as of the 5.0-16793 beta following this commit.

👍

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