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

WiimoteReal: Fix crash on real Wii Remote disconnect on Windows. #9460

Merged
merged 1 commit into from Jan 27, 2021

Conversation

jordan-woyak
Copy link
Member

Code for attaching a real Wii remote's HID interface to our emulated Bluetooth device is a bit messy.
A dangling pointer in the disconnect routine was causing a crash on Windows.

Fixes:
https://bugs.dolphin-emu.org/issues/12329
https://bugs.dolphin-emu.org/issues/12343
https://bugs.dolphin-emu.org/issues/12351

Possibly fixes this issue on macOS?
https://bugs.dolphin-emu.org/issues/9520

@dolphin-emu-bot

This comment has been minimized.

@jordan-woyak jordan-woyak changed the title WiimoteReal: Fix crash on real Wii Remote disconnect on Windows cause… WiimoteReal: Fix crash on real Wii Remote disconnect on Windows. Jan 20, 2021
g_wiimotes[index] = nullptr;
// The Wii Remote object must exist through the call to UpdateSource
// to prevent WiimoteDevice from having a dangling HIDWiimote pointer.
const auto temp_real_wiimote = std::move(g_wiimotes[index]);
Copy link
Contributor

@iwubcode iwubcode Jan 21, 2021

Choose a reason for hiding this comment

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

I guess alternatively we could have just swapped these calls?

WiimoteCommon::UpdateSource(index);
g_wiimotes[index] = nullptr;

Copy link
Member Author

Choose a reason for hiding this comment

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

No. Unfortunately UpdateSource gives WiimoteDevice a new pointer by grabbing it from g_wiimotes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't UpdateSource going to reach here:

hid_source = WiimoteReal::g_wiimotes[index].get();
and .get() the moved-from index?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. We want it to get a null pointer here, to disconnect it from the HID interface.
The problem was a dangling pointer a bit earlier in Activate / SetBasebandState / m_hid_source->EventUnlinked().

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, the comment is a bit confusing to me for some reason. anyway lgtm

@leoetlino leoetlino merged commit 2537ea7 into dolphin-emu:master Jan 27, 2021
10 checks passed
@ghost
Copy link

ghost commented Jan 27, 2021

Possibly fixes this issue on macOS?
https://bugs.dolphin-emu.org/issues/9520

I can't say I've ever had the issue of it crashing in the situation described in that bug report, but it still hangs and crashes in MacOS 11.0 when closing down Dolphin.

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