Merge pull request #9698 from hrydgard/managedtexture-reliability

General crash and hang fixes

Common/Timer.cpp

@@ -209,11 +209,11 @@ void Timer::GetTimeFormatted(char formattedTime[13])
 #ifdef _WIN32
 	struct timeb tp;
 	(void)::ftime(&tp);
-	sprintf(formattedTime, "%s:%03i", tmp, tp.millitm);
+	snprintf(formattedTime, 13, "%s:%03i", tmp, tp.millitm);
 #else
 	struct timeval t;
 	(void)gettimeofday(&t, NULL);
-	sprintf(formattedTime, "%s:%03d", tmp, (int)(t.tv_usec / 1000));
+	snprintf(formattedTime, 13, "%s:%03d", tmp, (int)(t.tv_usec / 1000));
 #endif
 }
 

Core/HLE/sceDisplay.cpp

@@ -670,10 +670,14 @@ void hleEnterVblank(u64 userdata, int cyclesLate) {
 		// Let the user know if we're running slow, so they know to adjust settings.
 		// Sometimes users just think the sound emulation is broken.
 		static bool hasNotifiedSlow = false;
-		if (!g_Config.bHideSlowWarnings && !hasNotifiedSlow && PSP_CoreParameter().fpsLimit == FPS_LIMIT_NORMAL && !g_Config.bSoftwareRendering && IsRunningSlow()) {
+		if (!g_Config.bHideSlowWarnings && !hasNotifiedSlow && PSP_CoreParameter().fpsLimit == FPS_LIMIT_NORMAL && IsRunningSlow()) {
 #ifndef _DEBUG
 			I18NCategory *err = GetI18NCategory("Error");
-			host->NotifyUserMessage(err->T("Running slow: try frameskip, sound is choppy when slow"), 6.0f, 0xFF30D0D0);
+			if (g_Config.bSoftwareRendering) {
+				host->NotifyUserMessage(err->T("Running slow: Try turning off Software Rendering"), 6.0f, 0xFF30D0D0);
+			} else {
+				host->NotifyUserMessage(err->T("Running slow: try frameskip, sound is choppy when slow"), 6.0f, 0xFF30D0D0);
+			}
 #endif
 			hasNotifiedSlow = true;
 		}

Core/Util/GameManager.cpp

@@ -115,13 +115,10 @@ void GameManager::Update() {
 				curDownload_.reset();
 				return;
 			}
-			// Install the game!
-			InstallGame(zipName);
-			// Doesn't matter if the install succeeds or not, we delete the temp file to not squander space.
-			// TODO: Handle disk full?
-			File::Delete(zipName.c_str());
+			// Game downloaded to temporary file - install it!
+			InstallGameOnThread(zipName, true);
 		} else {
-			ERROR_LOG(HLE, "Expected HTTP status code 200, got status code %i. Install cancelled.", curDownload_->ResultCode());
+			ERROR_LOG(HLE, "Expected HTTP status code 200, got status code %i. Install cancelled, deleting partia lfile.", curDownload_->ResultCode());
 			File::Delete(zipName.c_str());
 		}
 		curDownload_.reset();
@@ -134,17 +131,16 @@ bool GameManager::InstallGame(std::string zipfile, bool deleteAfter) {
 		return false;
 	}
 
-	I18NCategory *sy = GetI18NCategory("System");
-	installInProgress_ = true;
-
-	std::string pspGame = GetSysDirectory(DIRECTORY_GAME);
-	INFO_LOG(HLE, "Installing %s into %s", zipfile.c_str(), pspGame.c_str());
-
 	if (!File::Exists(zipfile)) {
 		ERROR_LOG(HLE, "ZIP file %s doesn't exist", zipfile.c_str());
 		return false;
 	}
 
+	I18NCategory *sy = GetI18NCategory("System");
+	installInProgress_ = true;
+
+	std::string pspGame = GetSysDirectory(DIRECTORY_GAME);
+	INFO_LOG(HLE, "Installing %s into %s", zipfile.c_str(), pspGame.c_str());
 	int error;
 #ifdef _WIN32
 	struct zip *z = zip_open(ConvertUTF8ToWString(zipfile).c_str(), 0, &error);
@@ -192,6 +188,8 @@ bool GameManager::InstallGame(std::string zipfile, bool deleteAfter) {
 		installInProgress_ = false;
 		installError_ = sy->T("Not a PSP game");
 		InstallDone();
+		if (deleteAfter)
+			File::Delete(zipfile);
 		return false;
 	}
 
@@ -312,7 +310,6 @@ bool GameManager::InstallGameOnThread(std::string zipFile, bool deleteAfter) {
 	if (installInProgress_) {
 		return false;
 	}
-
 	installThread_.reset(new std::thread(std::bind(&GameManager::InstallGame, this, zipFile, deleteAfter)));
 	installThread_->detach();
 	return true;

GPU/D3D11/DrawEngineD3D11.h

@@ -126,7 +126,6 @@ class DrawEngineD3D11 : public DrawEngineCommon {
 	}
 	void InitDeviceObjects();
 	void DestroyDeviceObjects();
-	void GLLost() {};
 
 	void BeginFrame();
 

GPU/Directx9/DrawEngineDX9.h

@@ -125,7 +125,6 @@ class DrawEngineDX9 : public DrawEngineCommon {
 	}
 	void InitDeviceObjects();
 	void DestroyDeviceObjects();
-	void GLLost() {};
 
 	void ClearTrackedVertexArrays() override;
 	void DecimateTrackedVertexArrays();

UI/BackgroundAudio.cpp

@@ -228,7 +228,7 @@ int PlayBackgroundAudio() {
 		if (!g_gameInfoCache)
 			return 0;  // race condition?
 
-		GameInfo *gameInfo = g_gameInfoCache->GetInfo(NULL, bgGamePath, GAMEINFO_WANTSND);
+		std::shared_ptr<GameInfo> gameInfo = g_gameInfoCache->GetInfo(NULL, bgGamePath, GAMEINFO_WANTSND);
 		if (!gameInfo)
 			return 0;
 

UI/CwCheatScreen.cpp

@@ -49,7 +49,7 @@ CwCheatScreen::CwCheatScreen(std::string gamePath)
 }
 
 void CwCheatScreen::CreateCodeList() {
-	GameInfo *info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
+	std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(NULL, gamePath_, 0);
 	if (info && info->paramSFOLoaded) {
 		gameTitle = info->paramSFO.GetValueString("DISC_ID");
 	}

UI/EmuScreen.cpp

@@ -146,7 +146,7 @@ void EmuScreen::bootGame(const std::string &filename) {
 	SetBackgroundAudioGame("");
 
 	//pre-emptive loading of game specific config if possible, to get all the settings
-	GameInfo *info = g_gameInfoCache->GetInfo(NULL, filename, 0);
+	std::shared_ptr<GameInfo> info = g_gameInfoCache->GetInfo(nullptr, filename, 0);
 	if (info && !info->id.empty()) {
 		g_Config.loadGameConfig(info->id);
 	}
@@ -210,7 +210,6 @@ void EmuScreen::bootGame(const std::string &filename) {
 	coreParam.pixelWidth = pixel_xres;
 	coreParam.pixelHeight = pixel_yres;
 
-
 	std::string error_string;
 	if (!PSP_InitStart(coreParam, &error_string)) {
 		bootPending_ = false;

UI/GameInfoCache.cpp

@@ -49,9 +49,11 @@ GameInfo::GameInfo() : fileType(IdentifiedFileType::UNKNOWN) {
 }
 
 GameInfo::~GameInfo() {
-	delete icon.texture;
-	delete pic0.texture;
-	delete pic1.texture;
+	std::lock_guard<std::mutex> guard(lock);
+	sndDataLoaded = false;
+	icon.Clear();
+	pic0.Clear();
+	pic1.Clear();
 	delete fileLoader;
 }
 
@@ -356,7 +358,7 @@ static bool ReadVFSToString(const char *filename, std::string *contents, std::mu
 
 class GameInfoWorkItem : public PrioritizedWorkQueueItem {
 public:
-	GameInfoWorkItem(const std::string &gamePath, GameInfo *info)
+	GameInfoWorkItem(const std::string &gamePath, std::shared_ptr<GameInfo> &info)
 		: gamePath_(gamePath), info_(info) {
 	}
 
@@ -615,7 +617,7 @@ class GameInfoWorkItem : public PrioritizedWorkQueueItem {
 
 private:
 	std::string gamePath_;
-	GameInfo *info_;
+	std::shared_ptr<GameInfo> info_;
 	DISALLOW_COPY_AND_ASSIGN(GameInfoWorkItem);
 };
 
@@ -646,20 +648,6 @@ void GameInfoCache::Clear() {
 		gameInfoWQ_->Flush();
 		gameInfoWQ_->WaitUntilDone();
 	}
-	for (auto iter = info_.begin(); iter != info_.end(); iter++) {
-		{
-			std::lock_guard<std::mutex> lock(iter->second->lock);
-			iter->second->pic0.Clear();
-			iter->second->pic1.Clear();
-			iter->second->icon.Clear();
-
-			if (!iter->second->sndFileData.empty()) {
-				iter->second->sndFileData.clear();
-				iter->second->sndDataLoaded = false;
-			}
-		}
-		delete iter->second;
-	}
 	info_.clear();
 }
 
@@ -668,7 +656,6 @@ void GameInfoCache::FlushBGs() {
 		std::lock_guard<std::mutex> lock(iter->second->lock);
 		iter->second->pic0.Clear();
 		iter->second->pic1.Clear();
-
 		if (!iter->second->sndFileData.empty()) {
 			iter->second->sndFileData.clear();
 			iter->second->sndDataLoaded = false;
@@ -689,7 +676,7 @@ void GameInfoCache::PurgeType(IdentifiedFileType fileType) {
 	}
 }
 
-void GameInfoCache::WaitUntilDone(GameInfo *info) {
+void GameInfoCache::WaitUntilDone(std::shared_ptr<GameInfo> &info) {
 	while (info->IsPending()) {
 		if (gameInfoWQ_->WaitUntilDone(false)) {
 			// A true return means everything finished, so bail out.
@@ -703,8 +690,8 @@ void GameInfoCache::WaitUntilDone(GameInfo *info) {
 
 
 // Runs on the main thread.
-GameInfo *GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags) {
-	GameInfo *info = nullptr;
+std::shared_ptr<GameInfo> GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags) {
+	std::shared_ptr<GameInfo> info;
 
 	auto iter = info_.find(gamePath);
 	if (iter != info_.end()) {
@@ -727,7 +714,7 @@ GameInfo *GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gam
 	}
 
 	if (!info) {
-		info = new GameInfo();
+		info = std::make_shared<GameInfo>();
 	}
 
 	if (info->IsWorking()) {
@@ -745,22 +732,24 @@ GameInfo *GameInfoCache::GetInfo(Draw::DrawContext *draw, const std::string &gam
 	GameInfoWorkItem *item = new GameInfoWorkItem(gamePath, info);
 	gameInfoWQ_->Add(item);
 
-	info_[gamePath] = info;
+	// Don't re-insert if we already have it.
+	if (info_.find(gamePath) == info_.end())
+		info_[gamePath] = std::shared_ptr<GameInfo>(info);
 	return info;
 }
 
-void GameInfoCache::SetupTexture(GameInfo *info, Draw::DrawContext *thin3d, GameInfoTex &icon) {
+void GameInfoCache::SetupTexture(std::shared_ptr<GameInfo> &info, Draw::DrawContext *thin3d, GameInfoTex &tex) {
 	using namespace Draw;
-	if (icon.data.size()) {
-		if (!icon.texture) {
-			icon.texture = CreateTextureFromFileData(thin3d, (const uint8_t *)icon.data.data(), (int)icon.data.size(), ImageFileType::DETECT);
-			if (icon.texture) {
-				icon.timeLoaded = time_now_d();
+	if (tex.data.size()) {
+		if (!tex.texture) {
+			tex.texture = CreateTextureFromFileData(thin3d, (const uint8_t *)tex.data.data(), (int)tex.data.size(), ImageFileType::DETECT);
+			if (tex.texture) {
+				tex.timeLoaded = time_now_d();
 			}
 		}
 		if ((info->wantFlags & GAMEINFO_WANTBGDATA) == 0) {
-			icon.data.clear();
-			icon.dataLoaded = false;
+			tex.data.clear();
+			tex.dataLoaded = false;
 		}
 	}
 }

UI/GameInfoCache.h

@@ -60,8 +60,14 @@ class FileLoader;
 enum class IdentifiedFileType;
 
 struct GameInfoTex {
+	GameInfoTex() {}
+	~GameInfoTex() {
+		if (texture) {
+			ELOG("LEAKED GameInfoTex");
+		}
+	}
 	std::string data;
-	ManagedTexture *texture = nullptr;
+	std::unique_ptr<ManagedTexture> texture;
 	// The time at which the Icon and the BG were loaded.
 	// Can be useful to fade them in smoothly once they appear.
 	double timeLoaded = 0.0;
@@ -72,11 +78,10 @@ struct GameInfoTex {
 			data.clear();
 			dataLoaded = false;
 		}
-		if (texture) {
-			delete texture;
-			texture = nullptr;
-		}
+		texture.reset(nullptr);
 	}
+private:
+	DISALLOW_COPY_AND_ASSIGN(GameInfoTex);
 };
 
 class GameInfo {
@@ -145,6 +150,9 @@ class GameInfo {
 
 	FileLoader *fileLoader = nullptr;
 	std::string filePath_;
+
+private:
+	DISALLOW_COPY_AND_ASSIGN(GameInfo);
 };
 
 class GameInfoCache {
@@ -160,20 +168,21 @@ class GameInfoCache {
 	// but filled in later asynchronously in the background. So keep calling this,
 	// redrawing the UI often. Only set flags to GAMEINFO_WANTBG or WANTSND if you really want them 
 	// because they're big. bgTextures and sound may be discarded over time as well.
-	GameInfo *GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags);
+	std::shared_ptr<GameInfo> GetInfo(Draw::DrawContext *draw, const std::string &gamePath, int wantFlags);
 	void FlushBGs();  // Gets rid of all BG textures. Also gets rid of bg sounds.
 
 	PrioritizedWorkQueue *WorkQueue() { return gameInfoWQ_; }
 
-	void WaitUntilDone(GameInfo *info);
+	void WaitUntilDone(std::shared_ptr<GameInfo> &info);
 
 private:
 	void Init();
 	void Shutdown();
-	void SetupTexture(GameInfo *info, Draw::DrawContext *draw, GameInfoTex &icon);
+	void SetupTexture(std::shared_ptr<GameInfo> &info, Draw::DrawContext *draw, GameInfoTex &tex);
 
-	// Maps ISO path to info.
-	std::map<std::string, GameInfo *> info_;
+	// Maps ISO path to info. Need to use shared_ptr as we can return these pointers - 
+	// and if they get destructed while being in use, that's bad.
+	std::map<std::string, std::shared_ptr<GameInfo> > info_;
 
 	// Work queue and management
 	PrioritizedWorkQueue *gameInfoWQ_;