From f8626f631a97b9aec8ab93af620c9ad8797fb4f7 Mon Sep 17 00:00:00 2001 From: bogu Date: Thu, 14 Sep 2023 18:44:28 +0200 Subject: [PATCH 1/3] Loadfile leak fix, LoadThemeByScript improvements Load file: - Now zCFile* created by zfactory->CreateZFile is deleted after usage. - MusicFile::Ready is set to true only if the file has been opened and all bytes have been read. zCMusicTheme init optimalizations: - Simplified creating script instance of zCMusicTheme - Set theme name only if isn't DM format --- plugin/src/Gothic/CMusicSys_Bass.hpp | 66 ++++++++++++++++------------ 1 file changed, 39 insertions(+), 27 deletions(-) diff --git a/plugin/src/Gothic/CMusicSys_Bass.hpp b/plugin/src/Gothic/CMusicSys_Bass.hpp index d915c6a..28a281e 100644 --- a/plugin/src/Gothic/CMusicSys_Bass.hpp +++ b/plugin/src/Gothic/CMusicSys_Bass.hpp @@ -110,26 +110,25 @@ namespace GOTHIC_NAMESPACE return m_ActiveTheme; } - zCMusicTheme* theme = zNEW(zCMusicTheme); - if (NH::Bass::Options->CreateMainParserCMusicTheme && parser->GetSymbol(identifier) != nullptr) - { - parser->CreateInstance(identifier, (void*)(&(theme->fileName))); - } - else + zCMusicTheme* theme = new zCMusicTheme; + + if (!(NH::Bass::Options->CreateMainParserCMusicTheme && parser->CreateInstance(identifier, &theme->fileName))) { - parserMusic->CreateInstance(identifier, (void*)(&(theme->fileName))); + parserMusic->CreateInstance(identifier, &theme->fileName); } - theme->name = identifier; if (IsDirectMusicFormat(theme->fileName)) { - zDELETE(theme); + delete theme; theme = m_DirectMusic->LoadThemeByScript(id); } else { + theme->name = identifier; + zoptions->ChangeDir(DIR_MUSIC); - zFILE* file = zfactory->CreateZFile(theme->fileName); + std::unique_ptr file{ zfactory->CreateZFile(theme->fileName) }; + if (file->Exists()) { NH::Bass::MusicFile& musicFileRef = m_BassEngine->CreateMusicBuffer(theme->fileName.ToChar()); @@ -137,26 +136,39 @@ namespace GOTHIC_NAMESPACE { NH::Log::Info("CMusicSys_Bass", Union::StringUTF8("Loading music: ") + file->GetFullPath().ToChar()); - file->Open(false); - musicFileRef.Loading = true; - auto loadingStart = std::chrono::system_clock::now(); - - std::thread loadingThread([file, &musicFileRef, loadingStart]() { - size_t size = file->Size(); - musicFileRef.Buffer.resize(size); - size_t read = file->Read(musicFileRef.Buffer.data(), size); - file->Close(); + const auto error = file->Open(false); - musicFileRef.Ready = true; - musicFileRef.Loading = false; + if (error == 0) + { + musicFileRef.Loading = true; - uint64_t loadingTime = std::chrono::duration_cast(std::chrono::system_clock::now() - loadingStart).count(); + std::thread([loadingStart = std::chrono::system_clock::now(), myFile = std::move(file), &musicFileRef]() { - NH::Log::Info("CMusicSys_Bass", Union::StringUTF8::Format("%z ready, size ", file->GetFullPath()) + Union::StringUTF8(read)); - NH::Log::Debug("CMusicSys_Bass", Union::StringUTF8::Format("%z loading took %I ms", file->GetFullPath(), loadingTime)); - }); - - loadingThread.detach(); + zSTRING path = myFile->GetFullPath(); + const long size = myFile->Size(); + + musicFileRef.Buffer.resize(static_cast(size)); + const long read = myFile->Read(musicFileRef.Buffer.data(), size); + + if (read == size) + { + musicFileRef.Ready = true; + + NH::Log::Info("CMusicSys_Bass", Union::StringUTF8::Format("%z ready, size ", path) + Union::StringUTF8(read)); + } + + musicFileRef.Loading = false; + myFile->Close(); + + auto loadingTime = std::chrono::duration_cast(std::chrono::system_clock::now() - loadingStart).count(); + + NH::Log::Debug("CMusicSys_Bass", Union::StringUTF8::Format("%z loading took %I ms", path, loadingTime)); + }).detach(); + } + else + { + NH::Log::Error("CMusicSys_Bass", Union::StringUTF8("Could not open file: ") + theme->fileName.ToChar()); + } } } else From 798cae4f5e49f84b9f73ccf85fe7f142ac27cd1d Mon Sep 17 00:00:00 2001 From: bogu Date: Thu, 14 Sep 2023 22:26:50 +0200 Subject: [PATCH 2/3] Moved captured vars to std::thread constructor --- plugin/src/Gothic/CMusicSys_Bass.hpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/plugin/src/Gothic/CMusicSys_Bass.hpp b/plugin/src/Gothic/CMusicSys_Bass.hpp index 28a281e..7941535 100644 --- a/plugin/src/Gothic/CMusicSys_Bass.hpp +++ b/plugin/src/Gothic/CMusicSys_Bass.hpp @@ -142,28 +142,30 @@ namespace GOTHIC_NAMESPACE { musicFileRef.Loading = true; - std::thread([loadingStart = std::chrono::system_clock::now(), myFile = std::move(file), &musicFileRef]() { + std::thread([](std::unique_ptr myFile, NH::Bass::MusicFile* myMusicPtr) { + auto loadingStart = std::chrono::system_clock::now(); + zSTRING path = myFile->GetFullPath(); const long size = myFile->Size(); - musicFileRef.Buffer.resize(static_cast(size)); - const long read = myFile->Read(musicFileRef.Buffer.data(), size); + myMusicPtr->Buffer.resize(static_cast(size)); + const long read = myFile->Read(myMusicPtr->Buffer.data(), size); if (read == size) { - musicFileRef.Ready = true; + myMusicPtr->Ready = true; NH::Log::Info("CMusicSys_Bass", Union::StringUTF8::Format("%z ready, size ", path) + Union::StringUTF8(read)); } - - musicFileRef.Loading = false; + + myMusicPtr->Loading = false; myFile->Close(); auto loadingTime = std::chrono::duration_cast(std::chrono::system_clock::now() - loadingStart).count(); NH::Log::Debug("CMusicSys_Bass", Union::StringUTF8::Format("%z loading took %I ms", path, loadingTime)); - }).detach(); + }, std::move(file), &musicFileRef).detach(); } else { From be898d99561069e462d0d453c2b7fc1999e4d78b Mon Sep 17 00:00:00 2001 From: bogu Date: Sun, 17 Sep 2023 12:04:29 +0200 Subject: [PATCH 3/3] Moved loadingStart to lambda capture --- plugin/src/Gothic/CMusicSys_Bass.hpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/plugin/src/Gothic/CMusicSys_Bass.hpp b/plugin/src/Gothic/CMusicSys_Bass.hpp index 7941535..4c38663 100644 --- a/plugin/src/Gothic/CMusicSys_Bass.hpp +++ b/plugin/src/Gothic/CMusicSys_Bass.hpp @@ -142,9 +142,7 @@ namespace GOTHIC_NAMESPACE { musicFileRef.Loading = true; - std::thread([](std::unique_ptr myFile, NH::Bass::MusicFile* myMusicPtr) { - - auto loadingStart = std::chrono::system_clock::now(); + std::thread([loadingStart = std::chrono::system_clock::now()](std::unique_ptr myFile, NH::Bass::MusicFile* myMusicPtr) { zSTRING path = myFile->GetFullPath(); const long size = myFile->Size();