From 7269ade98656e80aaf08720839f92e8095fbbb94 Mon Sep 17 00:00:00 2001
From: Gleb Mazovetskiy <glex.spb@gmail.com>
Date: Tue, 23 Mar 2021 00:30:35 +0000
Subject: [PATCH] Fix memory leak in single-player difficulty selection

Single-player difficulty selection was implemented in a very hacky way.

Documents what's going on there and fixes a memory leak.

Memory leaks that this fixes looked like this:

```
Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7f435b789f17 in operator new(unsigned long) (/lib/x86_64-linux-gnu/libasan.so.6+0xb1f17)
    #1 0x5567766c62b8 in dvl::selgame_GameSelection_Select(int) ../SourceX/DiabloUI/selgame.cpp:163
    #2 0x5567766d8ee8 in dvl::selhero_Load_Select(int) ../SourceX/DiabloUI/selhero.cpp:464
    #3 0x5567766d6ed1 in dvl::selhero_List_Select(int) ../SourceX/DiabloUI/selhero.cpp:325
    #4 0x556776692683 in dvl::UiFocusNavigationSelect() ../SourceX/DiabloUI/diabloui.cpp:396
    #5 0x55677669158e in HandleMenuAction ../SourceX/DiabloUI/diabloui.cpp:223
    #6 0x5567766917b9 in dvl::UiFocusNavigation(SDL_Event*) ../SourceX/DiabloUI/diabloui.cpp:277
    #7 0x556776695dff in dvl::UiPollAndRender() ../SourceX/DiabloUI/diabloui.cpp:626
    #8 0x5567766d94ef in UiSelHeroDialog ../SourceX/DiabloUI/selhero.cpp:512
    #9 0x5567766d997f in dvl::UiSelHeroSingDialog(int (*)(int (*)(dvl::_uiheroinfo*)), int (*)(dvl::_uiheroinfo*), int (*)(dvl::_uiheroinfo*), void (*)(unsigned int, dvl::_uidefaultstats*), int*, char (*) [16], int*) ../SourceX/DiabloUI/selhero.cpp:547
    #10 0x556776a44f45 in mainmenu_select_hero_dialog ../Source/mainmenu.cpp:98
    #11 0x5567765f9f15 in SNetInitializeProvider ../SourceX/storm/storm_net.cpp:102
    #12 0x556776c996b9 in multi_init_single ../Source/multi.cpp:826
    #13 0x556776c98b1e in NetInit ../Source/multi.cpp:770
    #14 0x5567767d0c0b in StartGame ../Source/diablo.cpp:375
    #15 0x556776a4493d in mainmenu_init_menu ../Source/mainmenu.cpp:45
    #16 0x556776a44c05 in mainmenu_single_player ../Source/mainmenu.cpp:61
    #17 0x556776a454a9 in mainmenu_loop ../Source/mainmenu.cpp:152
    #18 0x5567767d2892 in DiabloMain ../Source/diablo.cpp:602
    #19 0x5567766e3c69 in main ../SourceX/main.cpp:34
    #20 0x7f435a69ecb1 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x28cb1)
```
---
 SourceX/DiabloUI/selgame.cpp | 14 ++++++++++++++
 SourceX/DiabloUI/selhero.cpp | 15 ++++++++++++---
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/SourceX/DiabloUI/selgame.cpp b/SourceX/DiabloUI/selgame.cpp
index 7b0f64637d7..0550a3a7f50 100644
--- a/SourceX/DiabloUI/selgame.cpp
+++ b/SourceX/DiabloUI/selgame.cpp
@@ -246,7 +246,21 @@ void selgame_Diff_Select(int value)
 	nDifficulty = value;
 
 	if (!selhero_isMultiPlayer) {
+		// This is part of a dangerous hack to enable difficulty selection in single-player.
+		// FIXME: Dialogs should not refer to each other's variables.
+
+		// We're in the selhero loop instead of the selgame one.
+		// Free the selgame data and flag the end of the selhero loop.
 		selhero_endMenu = true;
+
+		// We only call FreeVectors because ArtBackground.Unload()
+		// will be called by selheroFree().
+		selgame_FreeVectors();
+
+		// We must clear the InitList because selhero's loop will perform
+		// one more iteration after this function exits.
+		UiInitList_clear();
+
 		return;
 	}
 
diff --git a/SourceX/DiabloUI/selhero.cpp b/SourceX/DiabloUI/selhero.cpp
index 964ece2422a..aa20e47f594 100644
--- a/SourceX/DiabloUI/selhero.cpp
+++ b/SourceX/DiabloUI/selhero.cpp
@@ -88,6 +88,7 @@ void selhero_Free()
 
 	selhero_FreeDlgItems();
 	selhero_FreeListItems();
+	UnloadScrollBar();
 
 	bUIElementsLoaded = false;
 }
@@ -457,7 +458,17 @@ void selhero_Load_Select(int value)
 	if (vecSelHeroDlgItems[value]->m_value == 0) {
 		selhero_result = SELHERO_CONTINUE;
 		return;
-	} else if (!selhero_isMultiPlayer) {
+	}
+
+	if (!selhero_isMultiPlayer) {
+		// This is part of a dangerous hack to enable difficulty selection in single-player.
+		// FIXME: Dialogs should not refer to each other's variables.
+
+		// We disable `selhero_endMenu` and replace the background and art
+		// and the item list with the difficulty selection ones.
+		//
+		// This means selhero's render loop will render selgame's items,
+		// which happens to work because the render loops are similar.
 		selhero_endMenu = false;
 		selhero_Free();
 		LoadBackgroundArt("ui_art\\selgame.pcx");
@@ -530,8 +541,6 @@ static void UiSelHeroDialog(
 
 	*dlgresult = selhero_result;
 	snprintf(*name, sizeof(*name), selhero_heroInfo.name);
-
-	UnloadScrollBar();
 }
 
 void UiSelHeroSingDialog(