From 71dd677ea18e4071bbf6d6baff903e097f54c74a Mon Sep 17 00:00:00 2001 From: "Dmitry D. Chernov" Date: Tue, 13 Feb 2024 19:58:37 +1000 Subject: [PATCH] Fix program termination sequence and a lot of memory leaks during it Along with this, the following bugs have been fixed, I hope: 1. Network operations didn't complete if game was ended by closing the window ('wLoadingQuit' had no effect whatsoever). 2. Restored and fixed the menu cleanup (was disabled in 1f93eba2). 3. Potential crash if new events appeared in the system queue when the game was shut down (i.e. after g_Game_Quit() call). --- src/game/g_game.pas | 17 +++++++----- src/game/g_main.pas | 31 ++++++++++----------- src/game/g_menu.pas | 67 +++++++++++++++++++++++++--------------------- src/game/g_netmaster.pas | 2 +- src/game/g_window.pas | 30 +++++++++------------ src/game/sdl/g_system.pas | 4 +-- src/game/sdl2/g_system.pas | 11 +++----- src/game/stub/g_system.pas | 10 +++---- 8 files changed, 86 insertions(+), 86 deletions(-) diff --git a/src/game/g_game.pas b/src/game/g_game.pas index 824595b..8509049 100644 --- a/src/game/g_game.pas +++ b/src/game/g_game.pas @@ -1048,7 +1048,7 @@ begin if gMapOnce then begin // Ýòî áûë òåñò - g_Game_Quit(); + gExit := EXIT_QUIT; end else begin // Âûõîä â ãëàâíîå ìåíþ @@ -1527,6 +1527,7 @@ end; procedure g_Game_Free(freeTextures: Boolean=true); begin + e_WriteLog('g_Game_Free: completion of the gameplay', TMsgType.Notify); if NetMode = NET_CLIENT then g_Net_Disconnect(); if NetMode = NET_SERVER then g_Net_Host_Die(); @@ -1909,7 +1910,7 @@ begin if gGameSettings.GameType in [GT_CUSTOM, GT_SERVER] then begin // Âûõîä â ãëàâíîå ìåíþ: - g_Game_Free; + g_Game_Free(); g_GUI_ShowWindow('MainMenu'); gMusic.SetByName('MUSIC_MENU'); gMusic.Play(); @@ -2742,7 +2743,7 @@ var begin e_TextureFontGetSize(gStdFont, ww2, hh2); - sys_HandleInput; + sys_HandleEvents(); if g_Console_Action(ACTION_SCORES) then begin @@ -4286,8 +4287,11 @@ begin g_Touch_Draw; end; +// FIXME: This cannot be called from anywhere other than ProcessMessages(), because otherwise +// remaining events in the system queue may cause use-after-free! Do 'gExit := EXIT_QUIT' instead. procedure g_Game_Quit(); begin + e_WriteLog('g_Game_Quit: cleanup assets before shutting down', TMsgType.Notify); g_Game_StopAllSounds(True); gMusic.Free(); g_Game_FreeData(); @@ -4295,7 +4299,7 @@ begin g_Texture_DeleteAll(); g_Frames_DeleteAll(); {$IFNDEF HEADLESS} - //g_Menu_Free(); //k8: this segfaults after resolution change; who cares? + g_Menu_Free(); {$ENDIF} if NetInitDone then g_Net_Free; @@ -4304,8 +4308,7 @@ begin if gMapToDelete <> '' then g_Game_DeleteTestMap(); - gExit := EXIT_QUIT; - sys_RequestQuit; + sys_RequestQuit(); // FIXME: this posts an event that have no sense anymore at this moment. end; procedure g_FatalError(Text: String); @@ -7844,7 +7847,7 @@ begin 'exit', 'quit': begin g_Game_Free(); - g_Game_Quit(); + gExit := EXIT_QUIT; end; 'r_reset': begin diff --git a/src/game/g_main.pas b/src/game/g_main.pas index f7d5467..43110d9 100644 --- a/src/game/g_main.pas +++ b/src/game/g_main.pas @@ -573,35 +573,36 @@ begin //g_Res_CreateDatabases(true); // it will be done before connecting to the server for the first time - e_WriteLog('Entering SDLMain', TMsgType.Notify); + e_WriteLog('Entering PerformExecution', TMsgType.Notify); - {$WARNINGS OFF} - SDLMain(); - {$WARNINGS ON} +{$WARNINGS OFF} + PerformExecution(); +{$WARNINGS ON} - {$IFDEF ENABLE_HOLMES} - if assigned(oglDeinitCB) then oglDeinitCB; - {$ENDIF} +{$IFDEF ENABLE_HOLMES} + if assigned(oglDeinitCB) then oglDeinitCB; +{$ENDIF} g_Console_WriteGameConfig; sys_Final; end; procedure Init(); - {$IFDEF USE_SDLMIXER} - var timiditycfg: AnsiString; - var oldcwd, newcwd: RawByteString; - {$ENDIF} - var NoSound: Boolean; +var +{$IFDEF USE_SDLMIXER} + timiditycfg: AnsiString; + oldcwd, newcwd: RawByteString; +{$ENDIF} + NoSound: Boolean; begin Randomize; {$IFDEF HEADLESS} - {$IFDEF USE_SDLMIXER} + {$IFDEF USE_SDLMIXER} NoSound := False; // hope env has set SDL_AUDIODRIVER to dummy - {$ELSE} + {$ELSE} NoSound := True; // FMOD backend will sort it out - {$ENDIF} + {$ENDIF} {$ELSE} NoSound := False; {$ENDIF} diff --git a/src/game/g_menu.pas b/src/game/g_menu.pas index 8fe06a3..1babcc9 100644 --- a/src/game/g_menu.pas +++ b/src/game/g_menu.pas @@ -1237,40 +1237,45 @@ var snd: TPlayableSound; res: Boolean; begin - if yes then + if not yes then begin - g_Game_StopAllSounds(True); - case (Random(18)) of - 0: s := 'SOUND_MONSTER_PAIN'; - 1: s := 'SOUND_MONSTER_DIE_3'; - 2: s := 'SOUND_MONSTER_SLOP'; - 3: s := 'SOUND_MONSTER_DEMON_DIE'; - 4: s := 'SOUND_MONSTER_IMP_DIE_2'; - 5: s := 'SOUND_MONSTER_MAN_DIE'; - 6: s := 'SOUND_MONSTER_BSP_DIE'; - 7: s := 'SOUND_MONSTER_VILE_DIE'; - 8: s := 'SOUND_MONSTER_SKEL_DIE'; - 9: s := 'SOUND_MONSTER_MANCUB_ALERT'; - 10: s := 'SOUND_MONSTER_PAIN_PAIN'; - 11: s := 'SOUND_MONSTER_BARON_DIE'; - 12: s := 'SOUND_MONSTER_CACO_DIE'; - 13: s := 'SOUND_MONSTER_CYBER_DIE'; - 14: s := 'SOUND_MONSTER_KNIGHT_ALERT'; - 15: s := 'SOUND_MONSTER_SPIDER_ALERT'; - else s := 'SOUND_PLAYER_FALL'; - end; - snd := TPlayableSound.Create(); - res := snd.SetByName(s); - if not res then res := snd.SetByName('SOUND_PLAYER_FALL'); - if res then - begin - snd.Play(True); - while snd.IsPlaying() do begin end; - end; - g_Game_Quit(); + g_GUI_HideWindow(); exit; end; - g_GUI_HideWindow(); + + g_Game_StopAllSounds(True); + case (Random(18)) of + 0: s := 'SOUND_MONSTER_PAIN'; + 1: s := 'SOUND_MONSTER_DIE_3'; + 2: s := 'SOUND_MONSTER_SLOP'; + 3: s := 'SOUND_MONSTER_DEMON_DIE'; + 4: s := 'SOUND_MONSTER_IMP_DIE_2'; + 5: s := 'SOUND_MONSTER_MAN_DIE'; + 6: s := 'SOUND_MONSTER_BSP_DIE'; + 7: s := 'SOUND_MONSTER_VILE_DIE'; + 8: s := 'SOUND_MONSTER_SKEL_DIE'; + 9: s := 'SOUND_MONSTER_MANCUB_ALERT'; + 10: s := 'SOUND_MONSTER_PAIN_PAIN'; + 11: s := 'SOUND_MONSTER_BARON_DIE'; + 12: s := 'SOUND_MONSTER_CACO_DIE'; + 13: s := 'SOUND_MONSTER_CYBER_DIE'; + 14: s := 'SOUND_MONSTER_KNIGHT_ALERT'; + 15: s := 'SOUND_MONSTER_SPIDER_ALERT'; + else s := 'SOUND_PLAYER_FALL'; + end; + + snd := TPlayableSound.Create(); + res := snd.SetByName(s); + if not res then + res := snd.SetByName('SOUND_PLAYER_FALL'); + + if res then + begin + snd.Play(True); + repeat until not snd.IsPlaying(); + end; + + gExit := EXIT_QUIT; end; procedure ProcLoadMenu(); diff --git a/src/game/g_netmaster.pas b/src/game/g_netmaster.pas index ef68c00..9436d62 100644 --- a/src/game/g_netmaster.pas +++ b/src/game/g_netmaster.pas @@ -1893,7 +1893,7 @@ begin if gConsoleShow or gChatShow then Exit; - qm := sys_HandleInput(); // this updates kbd + qm := sys_HandleEvents(); // this updates kbd if qm or e_KeyPressed(IK_ESCAPE) or e_KeyPressed(VK_ESCAPE) or e_KeyPressed(JOY0_JUMP) or e_KeyPressed(JOY1_JUMP) or diff --git a/src/game/g_window.pas b/src/game/g_window.pas index 0649801..6e8d3a4 100644 --- a/src/game/g_window.pas +++ b/src/game/g_window.pas @@ -20,7 +20,7 @@ interface uses utils; -function SDLMain (): Integer; +function PerformExecution (): Integer; procedure ResetTimer (); procedure ProcessLoading (forceUpdate: Boolean = False); @@ -55,7 +55,6 @@ var flag: Boolean; wNeedTimeReset: Boolean = false; wMinimized: Boolean = false; - wLoadingQuit: Boolean = false; procedure ResetTimer (); begin @@ -73,7 +72,7 @@ var stt: UInt64; {$ENDIF} begin - if sys_HandleInput() = True then + if sys_HandleEvents() then Exit; {$IFNDEF HEADLESS} @@ -121,7 +120,14 @@ function ProcessMessage (): Boolean; var i, t: Integer; begin - result := sys_HandleInput(); + // BEWARE: Short-circuit evaluation matters here! + Result := (gExit = EXIT_QUIT) or sys_HandleEvents(); + if Result then + begin + g_Game_Free(); + g_Game_Quit(); + exit; + end; Time := sys_GetTicks(); Time_Delta := Time-Time_Old; @@ -149,18 +155,6 @@ begin g_Map_ProfilersEnd(); g_Mons_ProfilersEnd(); - if wLoadingQuit then - begin - g_Game_Free(); - g_Game_Quit(); - end; - - if (gExit = EXIT_QUIT) then - begin - result := true; - exit; - end; - // Âðåìÿ ïðåäûäóùåãî îáíîâëåíèÿ if flag then Time_Old := Time - (Time_Delta mod 28); @@ -236,7 +230,7 @@ begin e_LogWritefln('GL Extensions: %s', [glGetString(GL_EXTENSIONS)]); end; -function SDLMain (): Integer; +function PerformExecution (): Integer; var idx: Integer; arg: AnsiString; @@ -374,7 +368,7 @@ begin e_WriteLog('Entering the main loop', TMsgType.Notify); // main loop - while not ProcessMessage() do begin end; + repeat until ProcessMessage(); g_Net_Slist_ShutdownAll(); diff --git a/src/game/sdl/g_system.pas b/src/game/sdl/g_system.pas index 9f56262..cc70f58 100644 --- a/src/game/sdl/g_system.pas +++ b/src/game/sdl/g_system.pas @@ -30,7 +30,7 @@ interface procedure sys_Repaint; (* --- Input --- *) - function sys_HandleInput (): Boolean; + function sys_HandleEvents (): Boolean; procedure sys_RequestQuit; (* --- Init --- *) @@ -660,7 +660,7 @@ implementation InitWindow(ev.w, ev.h, gBPP, gFullscreen) end; - function sys_HandleInput (): Boolean; + function sys_HandleEvents (): Boolean; var ev: TSDL_Event; begin result := false; diff --git a/src/game/sdl2/g_system.pas b/src/game/sdl2/g_system.pas index 6f48d28..6d79d7d 100644 --- a/src/game/sdl2/g_system.pas +++ b/src/game/sdl2/g_system.pas @@ -30,7 +30,7 @@ interface procedure sys_Repaint; (* --- Input --- *) - function sys_HandleInput (): Boolean; + function sys_HandleEvents (): Boolean; procedure sys_RequestQuit; (* --- Init --- *) @@ -263,10 +263,7 @@ implementation procedure sys_EnableVSync (yes: Boolean); begin - if yes then - SDL_GL_SetSwapInterval(1) - else - SDL_GL_SetSwapInterval(0) + SDL_GL_SetSwapInterval(Ord(yes)); end; function sys_GetDisplayModes (bpp: Integer): SSArray; @@ -275,7 +272,7 @@ implementation result := nil; num := SDL_GetNumDisplayModes(display); if num < 0 then - e_LogWritefln('SDL: unable to get numer of available display modes: %s', [SDL_GetError]); + e_LogWritefln('SDL: unable to get number of available display modes: %s', [SDL_GetError]); if num > 0 then begin e_LogWritefln('Video modes for display %s:', [display]); @@ -537,7 +534,7 @@ implementation CharPress(sch); end; - function sys_HandleInput (): Boolean; + function sys_HandleEvents (): Boolean; var ev: TSDL_Event; begin result := false; diff --git a/src/game/stub/g_system.pas b/src/game/stub/g_system.pas index c3b4b23..db5b67f 100644 --- a/src/game/stub/g_system.pas +++ b/src/game/stub/g_system.pas @@ -30,7 +30,7 @@ interface procedure sys_Repaint; (* --- Input --- *) - function sys_HandleInput (): Boolean; + function sys_HandleEvents (): Boolean; procedure sys_RequestQuit; (* --- Init --- *) @@ -65,19 +65,19 @@ implementation function sys_GetDisplayModes (bpp: Integer): SSArray; begin - result := nil + Result := nil end; function sys_SetDisplayMode (w, h, bpp: Integer; fullscreen, maximized: Boolean): Boolean; begin - result := true + Result := True end; (* --------- Input --------- *) - function sys_HandleInput (): Boolean; + function sys_HandleEvents (): Boolean; begin - result := false + Result := False end; procedure sys_RequestQuit; -- 2.11.4.GIT