Fix WAV export crash caused by uninitialized variables#367
Merged
Gumball2415 merged 2 commits intomainfrom Jul 30, 2025
Merged
Conversation
m_bStoppingRender was never initialized. If it happened to be nonzero, (audio) OnIdle() would count down m_iDelayedEnd to 0 and then call StopRendering() on every frame, which would fail because IsRendering() is false. When you initiated a render on the GUI thread (setting m_pWaveFile and m_bRequestRenderStart), (audio) OnIdle() -> StopRendering() could succeed and erase m_pWaveFile before (audio) CSoundGen::DispatchGuiMessage() processes WM_USER_START_RENDER from the GUI. Attempting to render without m_pWaveFile would crash. Fix this by initializing m_bStoppingRender, as well as other flags that the code doesn't access immediately but I don't want to leave uninitialized. I didn't initialize everything because I don't understand the existing code.
This prevents deadlocks while one thread's Log::dump() is holding the logger lock, and changes how dump() affects thread interleaving (releasing the mutex with the dialog open can prevent starvation).
Collaborator
i think a crash logger in general would definitely help in pinpointing some blind spots regarding the concurrency and state issues of FT, so i'd keep it for now |
Collaborator
|
lgtm, will be merging soon |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request aims to fix random crashes the first time you export a WAV file from FamiTracker.
bool CSoundGen::m_bStoppingRenderfailed to be initialized. If it happened to be nonzero,(audio) CSoundGen::OnIdle()would count downm_iDelayedEndto 0 and then callStopRendering()on every frame, which would fail becauseIsRendering()is false. (This error was silent on release builds before #353, but now dumps a log and blocks the audio thread with a dialog.)The crash happens when the user initiates a render on the GUI thread (setting
m_pWaveFileandm_bRequestRenderStart). While the audio thread is looping,OnIdle() → StopRendering()could succeed and erasem_pWaveFilebefore(audio) CSoundGen::DispatchGuiMessage()processesWM_USER_START_RENDERfrom the GUI. Attempting to render withoutm_pWaveFilewould crash.Fix this by initializing
CSoundGen::m_bStoppingRender, as well as other flags that the code doesn't access immediately but I don't want to leave uninitialized. I didn't initialize everything because I don't understand the existing code.Crash log:
This pull request improves upon and supercedes #272.
Also update the logging infrastructure to "Don't hold mutex during WAV export error dialog".
Changes in this PR: