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 crash when pressing Tab #92

Merged
merged 1 commit into from Apr 14, 2014
Merged

Fix crash when pressing Tab #92

merged 1 commit into from Apr 14, 2014

Conversation

Tilka
Copy link
Member

@Tilka Tilka commented Feb 22, 2014

When pressing Tab for a long time, Dolphin will sooner or later crash because the result of FindFocus() has become NULL (toctou). No idea why that happens. Obviously, we are passing key events to WX even after handling them ourselves. I don't think we should do that. In any case, this fix makes the code cleaner.

@@ -676,13 +676,16 @@ bool CFrame::RendererHasFocus()
if (m_RenderParent->GetParent()->GetHWND() == GetForegroundWindow())
return true;
#else
if (wxWindow::FindFocus() == NULL)
wxWindow *window = wxWindow::FindFocus();

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

This comment was marked as off-topic.

@vinnymac
Copy link

http://trac.wxwidgets.org/ticket/15409
http://trac.wxwidgets.org/ticket/12713

These wx bugs are pretty old. It may be a bread crumb trail, but it is worth reading. At least it is a known issue.

@Tilka
Copy link
Member Author

Tilka commented Apr 12, 2014

I still think this PR is a good way to fix it. If wx documents its API with "might return NULL" we should make sure to handle NULL correctly whether it is a bug in wx or not.

@Parlane
Copy link
Member

Parlane commented Apr 14, 2014

LGTM just needs a rebase.

FindFocus is a static function, it returns a pointer. Adding code to handle it returning NULL should not be an issue. Why it returns NULL? Because the focus was lost. Call it a workaround for a known issue if you'd like, we don't need to spend time fixing wx. Especially considering we want to move to QT, right.. right?! :(

When pressing Tab for a long time, Dolphin will sooner or later crash
because the result of FindFocus() has become NULL (toctou).
delroth added a commit that referenced this pull request Apr 14, 2014
Fix crash when pressing Tab
@delroth delroth merged commit c614a70 into dolphin-emu:master Apr 14, 2014
@Tilka Tilka deleted the tab_crash branch April 14, 2014 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
4 participants