From 77748f29f93312582a71cf3e66e14c1f5511339d Mon Sep 17 00:00:00 2001 From: Misa Date: Mon, 15 Feb 2021 16:42:58 -0800 Subject: [PATCH] Separate musicReadBlob into mmmmmm_blob and pppppp_blob musicReadBlob was used for both MMMMMM and PPPPPP soundtracks. This causes a memory leak if you have mmmmmm.vvv installed, because the pointers holding each allocated block of MMMMMM would be lost when PPPPPP got loaded. Valgrind complains about this memory leak. This is in contrast to 2.2 and previous behavior, where musicReadBlob was only a temporary object instead of being held in musicclass. However, this wasn't really a memory leak (moreso something that just didn't get cleaned up when closing the game), but it did get turned into a leak when per-level assets mounting and unmounting got introduced in 2.3 (loading a level with custom assets after starting the game with an mmmmmm.vvv, or exiting out of a level that had an mmmmmm.vvv, would cause the game to leak memory). Leo recognized this, and moved musicReadBlob onto musicclass in a separate 2.3 PR, but either he didn't think about what was happening here too closely, or he didn't use Valgrind, because he forgot about the memory leak caused by re-using the same binaryBlob for PPPPPP and MMMMMM. So instead, just use two different binaryBlob objects for MMMMMM and PPPPPP. That way, no memory leaks happen. --- desktop_version/src/Music.cpp | 21 +++++++++++---------- desktop_version/src/Music.h | 3 ++- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/desktop_version/src/Music.cpp b/desktop_version/src/Music.cpp index bbebce99..640c3728 100644 --- a/desktop_version/src/Music.cpp +++ b/desktop_version/src/Music.cpp @@ -74,11 +74,11 @@ void musicclass::init() num_mmmmmm_tracks = 0; num_pppppp_tracks = 0; - if (!musicReadBlob.unPackBinary("mmmmmm.vvv")) + if (!mmmmmm_blob.unPackBinary("mmmmmm.vvv")) { mmmmmm = false; usingmmmmmm=false; - bool ohCrap = musicReadBlob.unPackBinary("vvvvvvmusic.vvv"); + bool ohCrap = pppppp_blob.unPackBinary("vvvvvvmusic.vvv"); SDL_assert(ohCrap && "Music not found!"); } else @@ -102,38 +102,38 @@ void musicclass::init() } \ } - TRACK_NAMES(musicReadBlob) + TRACK_NAMES(mmmmmm_blob) num_mmmmmm_tracks += musicTracks.size(); - const std::vector extra = musicReadBlob.getExtra(); + const std::vector extra = mmmmmm_blob.getExtra(); for (size_t i = 0; i < extra.size(); i++) { const int& index_ = extra[i]; - rw = SDL_RWFromMem(musicReadBlob.getAddress(index_), musicReadBlob.getSize(index_)); + rw = SDL_RWFromMem(mmmmmm_blob.getAddress(index_), mmmmmm_blob.getSize(index_)); musicTracks.push_back(MusicTrack( rw )); num_mmmmmm_tracks++; } - bool ohCrap = musicReadBlob.unPackBinary("vvvvvvmusic.vvv"); + bool ohCrap = pppppp_blob.unPackBinary("vvvvvvmusic.vvv"); SDL_assert(ohCrap && "Music not found!"); } int index; SDL_RWops *rw; - TRACK_NAMES(musicReadBlob) + TRACK_NAMES(pppppp_blob) #undef FOREACH_TRACK num_pppppp_tracks += musicTracks.size() - num_mmmmmm_tracks; - const std::vector extra = musicReadBlob.getExtra(); + const std::vector extra = pppppp_blob.getExtra(); for (size_t i = 0; i < extra.size(); i++) { const int& index_ = extra[i]; - rw = SDL_RWFromMem(musicReadBlob.getAddress(index_), musicReadBlob.getSize(index_)); + rw = SDL_RWFromMem(pppppp_blob.getAddress(index_), pppppp_blob.getSize(index_)); musicTracks.push_back(MusicTrack( rw )); num_pppppp_tracks++; @@ -165,7 +165,8 @@ void musicclass::destroy() } musicTracks.clear(); - musicReadBlob.clear(); + pppppp_blob.clear(); + mmmmmm_blob.clear(); } void musicclass::play(int t, const double position_sec /*= 0.0*/, const int fadein_ms /*= 3000*/) diff --git a/desktop_version/src/Music.h b/desktop_version/src/Music.h index 0da6af6f..8a5e1668 100644 --- a/desktop_version/src/Music.h +++ b/desktop_version/src/Music.h @@ -51,7 +51,8 @@ public: bool mmmmmm; bool usingmmmmmm; - binaryBlob musicReadBlob; + binaryBlob pppppp_blob; + binaryBlob mmmmmm_blob; int num_pppppp_tracks; int num_mmmmmm_tracks;