Merge pull request #11842 from hrydgard/win32-handle-leak-fix

Win32 handle leak fix
hrydgard · Feb 25, 2019 · 64dd425 · 64dd425
2 parents e3d7ca6 + cb25354
commit 64dd425
Show file tree

Hide file tree

Showing 7 changed files with 25 additions and 13 deletions.
diff --git a/Common/FileUtil.cpp b/Common/FileUtil.cpp
@@ -133,6 +133,7 @@ std::string ResolvePath(const std::string &path) {
 			int result = getFinalPathNameByHandleW(hFile, buf, BUF_SIZE - 1, FILE_NAME_NORMALIZED | VOLUME_NAME_DOS);
 			if (result >= BUF_SIZE || result == 0)
 				wcscpy_s(buf, BUF_SIZE - 1, input.c_str());
+			CloseHandle(hFile);
 		}
 	} else {
 		wchar_t *longBuf = new wchar_t[BUF_SIZE];

diff --git a/Core/FileLoaders/LocalFileLoader.cpp b/Core/FileLoaders/LocalFileLoader.cpp
@@ -16,6 +16,7 @@
 // https://github.com/hrydgard/ppsspp and http://www.ppsspp.org/.
 
 #include <cstdio>
+
 #include "ppsspp_config.h"
 #include "util/text/utf8.h"
 #include "file/file_util.h"
@@ -30,7 +31,10 @@
 
 LocalFileLoader::LocalFileLoader(const std::string &filename)
 	: filesize_(0), filename_(filename) {
-
+	if (filename.empty()) {
+		ERROR_LOG(FILESYS, "LocalFileLoader can't load empty filenames");
+		return;
+	}
 #ifndef _WIN32
 
 	fd_ = open(filename.c_str(), O_RDONLY | O_CLOEXEC);
@@ -47,7 +51,7 @@ LocalFileLoader::LocalFileLoader(const std::string &filename)
 	lseek(fd_, 0, SEEK_SET);
 #endif
 
-#else // !_WIN32
+#else // _WIN32
 
 	const DWORD access = GENERIC_READ, share = FILE_SHARE_READ, mode = OPEN_EXISTING, flags = FILE_ATTRIBUTE_NORMAL;
 #if PPSSPP_PLATFORM(UWP)
@@ -60,14 +64,15 @@ LocalFileLoader::LocalFileLoader(const std::string &filename)
 	}
 	LARGE_INTEGER end_offset;
 	const LARGE_INTEGER zero = { 0 };
-	if(SetFilePointerEx(handle_, zero, &end_offset, FILE_END) == 0) {
+	if (SetFilePointerEx(handle_, zero, &end_offset, FILE_END) == 0) {
+		// Couldn't seek in the file. Close it and give up? This should never happen.
+		CloseHandle(handle_);
+		handle_ = INVALID_HANDLE_VALUE;
 		return;
 	}
 	filesize_ = end_offset.QuadPart;
 	SetFilePointerEx(handle_, zero, nullptr, FILE_BEGIN);
-
-#endif // !_WIN32
-
+#endif // _WIN32
 }
 
 LocalFileLoader::~LocalFileLoader() {

diff --git a/Core/FileSystems/DirectoryFileSystem.cpp b/Core/FileSystems/DirectoryFileSystem.cpp
@@ -154,6 +154,10 @@ DirectoryFileSystem::DirectoryFileSystem(IHandleAllocator *_hAlloc, std::string
 	hAlloc = _hAlloc;
 }
 
+DirectoryFileSystem::~DirectoryFileSystem() {
+	CloseAll();
+}
+
 std::string DirectoryFileHandle::GetLocalPath(std::string& basePath, std::string localpath)
 {
 	if (localpath.empty())
@@ -445,16 +449,12 @@ void DirectoryFileHandle::Close()
 
 void DirectoryFileSystem::CloseAll() {
 	for (auto iter = entries.begin(); iter != entries.end(); ++iter) {
+		INFO_LOG(FILESYS, "DirectoryFileSystem::CloseAll(): Force closing %d (%s)", (int)iter->first, iter->second.guestFilename.c_str());
 		iter->second.hFile.Close();
 	}
-
 	entries.clear();
 }
 
-DirectoryFileSystem::~DirectoryFileSystem() {
-	CloseAll();
-}
-
 std::string DirectoryFileSystem::GetLocalPath(std::string localpath) {
 	if (localpath.empty())
 		return basePath;

diff --git a/UI/GameInfoCache.cpp b/UI/GameInfoCache.cpp
@@ -229,6 +229,11 @@ bool GameInfo::LoadFromPath(const std::string &gamePath) {
 }
 
 std::shared_ptr<FileLoader> GameInfo::GetFileLoader() {
+	if (filePath_.empty()) {
+		// Happens when workqueue tries to figure out priorities in PrioritizedWorkQueue::Pop(),
+		// because priority() calls GetFileLoader()... gnarly.
+		return fileLoader;
+	}
 	if (!fileLoader) {
 		fileLoader.reset(ConstructFileLoader(filePath_));
 	}

diff --git a/UI/GameInfoCache.h b/UI/GameInfoCache.h
@@ -146,6 +146,7 @@ class GameInfo {
 	// Note: this can change while loading, use GetTitle().
 	std::string title;
 
+	// TODO: Get rid of this shared_ptr and managae lifetime better instead.
 	std::shared_ptr<FileLoader> fileLoader;
 	std::string filePath_;
 

diff --git a/Windows/GPU/WindowsVulkanContext.cpp b/Windows/GPU/WindowsVulkanContext.cpp
@@ -128,8 +128,8 @@ bool WindowsVulkanContext::Init(HINSTANCE hInst, HWND hWnd, std::string *error_m
 			int bits = VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT
 				| VK_DEBUG_UTILS_MESSAGE_SEVERITY_WARNING_BIT_EXT
 				| VK_DEBUG_UTILS_MESSAGE_SEVERITY_INFO_BIT_EXT
-				| VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT
 				| VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT;
+				// We're intentionally skipping VK_DEBUG_UTILS_MESSAGE_SEVERITY_VERBOSE_BIT_EXT
 			g_Vulkan->InitDebugUtilsCallback(&VulkanDebugUtilsCallback, bits, &g_LogOptions);
 		} else {
 			int bits = VK_DEBUG_REPORT_ERROR_BIT_EXT | VK_DEBUG_REPORT_WARNING_BIT_EXT | VK_DEBUG_REPORT_PERFORMANCE_WARNING_BIT_EXT;

diff --git a/ext/native/file/path.h b/ext/native/file/path.h
@@ -25,7 +25,7 @@ class PathBrowser {
 		else
 			return "";
 	}
-	std::string GetFriendlyPath() {
+	std::string GetFriendlyPath() const {
 		std::string str = GetPath();
 #if defined(__ANDROID__)
 		// Do nothing