Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Win32 handle leak fix #11842

Merged
merged 3 commits into from
Feb 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Common/FileUtil.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Arg, oops. Thanks.

-[Unknown]

}
} else {
wchar_t *longBuf = new wchar_t[BUF_SIZE];
Expand Down
17 changes: 11 additions & 6 deletions Core/FileLoaders/LocalFileLoader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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);
Expand All @@ -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)
Expand All @@ -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() {
Expand Down
10 changes: 5 additions & 5 deletions Core/FileSystems/DirectoryFileSystem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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;
Expand Down
5 changes: 5 additions & 0 deletions UI/GameInfoCache.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just remember that for some loaders, e.g. ones that involve memory caching or http access, the cost of tossing this in the bin and getting a new one a couple times is much higher. Ideally we shouldn't recreate just because that's easiest.

-[Unknown]

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, put this comment in the wrong place somehow - was meant for the lifetime comment.

-[Unknown]

}
if (!fileLoader) {
fileLoader.reset(ConstructFileLoader(filePath_));
}
Expand Down
1 change: 1 addition & 0 deletions UI/GameInfoCache.h
Original file line number Diff line number Diff line change
Expand Up @@ -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_;

Expand Down
2 changes: 1 addition & 1 deletion Windows/GPU/WindowsVulkanContext.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion ext/native/file/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class PathBrowser {
else
return "";
}
std::string GetFriendlyPath() {
std::string GetFriendlyPath() const {
std::string str = GetPath();
#if defined(__ANDROID__)
// Do nothing
Expand Down