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

Restore saving logger settings to .ini #4292

Merged
merged 1 commit into from Oct 5, 2016
Merged

Restore saving logger settings to .ini #4292

merged 1 commit into from Oct 5, 2016

Conversation

shuffle2
Copy link
Contributor

@shuffle2 shuffle2 commented Oct 4, 2016

This was broken by #4231

The callstack is:

>   DolphinD.exe!LogConfigWindow::SaveSettings() Line 136   C++
    DolphinD.exe!LogConfigWindow::~LogConfigWindow() Line 35    C++
    DolphinD.exe!LogConfigWindow::`scalar deleting destructor'(unsigned int)    C++
    DolphinD.exe!wxWindowBase::Destroy() Line 574   C++
    DolphinD.exe!CFrame::DoRemovePage(wxWindow * Win, bool bHide) Line 453  C++
    DolphinD.exe!CFrame::ToggleLogConfigWindow(bool bShow) Line 145 C++
    DolphinD.exe!CFrame::ClosePages() Line 181  C++
    DolphinD.exe!CFrame::~CFrame() Line 558 C++
    DolphinD.exe!CFrame::`scalar deleting destructor'(unsigned int) C++
    DolphinD.exe!wxAppConsoleBase::DeletePendingObjects() Line 637  C++
    DolphinD.exe!wxAppConsoleBase::ProcessIdle() Line 445   C++
    DolphinD.exe!wxAppBase::ProcessIdle() Line 373  C++
    DolphinD.exe!wxEventLoopBase::ProcessIdle() Line 98 C++
    DolphinD.exe!wxEventLoopManual::DoRun() Line 263    C++
    DolphinD.exe!wxEventLoopBase::Run() Line 76 C++
    DolphinD.exe!wxAppConsoleBase::MainLoop() Line 380  C++
    DolphinD.exe!wxAppConsoleBase::OnRun() Line 302 C++
    DolphinD.exe!wxAppBase::OnRun() Line 312    C++
    DolphinD.exe!wxEntryReal(int & argc, wchar_t * * argv) Line 503 C++
    DolphinD.exe!wxEntry(int & argc, wchar_t * * argv) Line 181 C++
    DolphinD.exe!wxEntry(HINSTANCE__ * hInstance, HINSTANCE__ * __formal, char * __formal, int nCmdShow) Line 290   C++
    DolphinD.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * __formal, int nCmdShow) Line 59   C++
    DolphinD.exe!invoke_main() Line 99  C++
    DolphinD.exe!__scrt_common_main_seh() Line 253  C++
    DolphinD.exe!__scrt_common_main() Line 296  C++
    DolphinD.exe!WinMainCRTStartup() Line 17    C++
    kernel32.dll!BaseThreadInitThunk�() Unknown
    ntdll.dll!RtlUserThreadStart�() Unknown

Is this bad on OSX for some reason?


This change is Reviewable

@shuffle2 shuffle2 mentioned this pull request Oct 4, 2016
14 tasks
@EmptyChaos
Copy link
Contributor

I think the problem is that it goes CFrame::OnClose -> DolphinApp::OnExit -> CFrame::~CFrame -> CLogWindow::~CLogWindow -> Dereference dead LogManager -> Crash.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Oct 4, 2016

Oh so m_LogManager is free'd while in LogConfigWindow::SaveSettings...?

@EmptyChaos
Copy link
Contributor

By UICommon::Shutdown in DolphinApp::OnExit then the destructor is called after that happened. I'm guessing, but that's the only obvious potential problem I see.

@shuffle2
Copy link
Contributor Author

shuffle2 commented Oct 4, 2016

LogConfigWindow::SaveSettings is called before UICommon::Shutdown so I'm not seeing the problem :/

@EmptyChaos
Copy link
Contributor

If you're testing on OS X then the problem supposedly only happens when exiting through the Dock Icon, Right Click Dock Icon > Exit or something. If it doesn't crash then there isn't a problem any more, I guess?

@ligfx
Copy link
Contributor

ligfx commented Oct 4, 2016

@shuffle2 what OS did you see this issue on?

@shuffle2
Copy link
Contributor Author

shuffle2 commented Oct 4, 2016

@ligfx as @EmptyChaos said, OnClose is not fired for this class, at least on windows...

@ligfx
Copy link
Contributor

ligfx commented Oct 4, 2016

This does break quitting from the Dock on macOS, but upon further investigation it seems to be because of something weird with wxWidgets (OnEndSession gets called with events pending, so main_frame doesn't get destroyed until after OnExit), not an issue with this code. So 👍 on this PR.

@shuffle2 shuffle2 merged commit 9be6f38 into dolphin-emu:master Oct 5, 2016
@shuffle2 shuffle2 deleted the fix-saving-log-settings branch June 2, 2017 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
3 participants