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

Fix hotkey suppression crash #9674

Merged
merged 2 commits into from Apr 28, 2021

Conversation

Filoppi
Copy link
Contributor

@Filoppi Filoppi commented Apr 26, 2021

EnableSuppression() was called in UpdateReferences() but then not executed if m_suppressor is already valid, causing input to stay nullptr in the m_suppressions map, which would fail to find an element and crash because it wasn't checked for validity.

I fixed the source of that happening, but also added safety checks around against nullptr.

I had gotten this was crash twice in months, both times in Debug, on startup, without doing anything, while Dolphin was loading (from the hotkey thread). It was probably just an order/timing dependency between threads.

If I had to guess, I'd say that it worked before because EnableSuppression() is called constantly by GetValue() so it would constantly refresh, but if things didn't happen in a specific order, it would crash.

The crash (on the it iterator) (callstack in on my branch, slightly different, but this bug was present and triggerable on master as well):
CqRFlvgQzq

I've also included a small consistency fix for final_input_state being checked < and then > against CONDITION_THRESHOLD and not <= and then >.

Tested.

…e added to the map

Update references was failing to update the references, causing input to stay nullptr and crashing.
I fixed the case that triggered that, though also added checks against nullptrs for safety.

(cherry picked from commit 4bdcf70)
@Filoppi Filoppi changed the title Fix hotkey suppresion crash Fix hotkey suppression crash Apr 26, 2021
{
if (!m_suppressor)
if (!m_suppressor || force)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm confused why this force is needed. The only place you made it not the default is where the m_suppressor is already true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

EnableSuppression is called in two places and I didn't want to change the other call to it, so to keep changes to a min I added a force param which defaults to false.
Before it was calling EnableSuppression only if m_suppressor was true, but then the function inside would only proceed if it was false, which made no sense.

@jordan-woyak jordan-woyak merged commit 1daefeb into dolphin-emu:master Apr 28, 2021
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants