-
Notifications
You must be signed in to change notification settings - Fork 2.3k
More crash fixes #17399
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
More crash fixes #17399
Changes from all commits
214d1d2
0e2fb13
c9b7c81
c80671d
d56e27a
5724bbd
1d053d2
a132b72
3148a8a
7ddcf62
6a51b6f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -62,6 +62,7 @@ class GPUCommonHW : public GPUCommon { | |
|
||
void Execute_TexFlush(u32 op, u32 diff); | ||
|
||
// TODO: Have these return an error code if they jump to a bad address. If bad, stop the FastRunLoop. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Or just set -[Unknown] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh yeah, that's probably the slickest solution. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually it seems we already effectively do that in many cases through I am going to remove Execute_JumpFast and Execute_CallFast. I don't believe they are worth it. |
||
typedef void (GPUCommonHW::*CmdFunc)(u32 op, u32 diff); | ||
|
||
void FastRunLoop(DisplayList &list) override; | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -197,9 +197,12 @@ bool GameInfo::LoadFromPath(const Path &gamePath) { | |
std::lock_guard<std::mutex> guard(lock); | ||
// No need to rebuild if we already have it loaded. | ||
if (filePath_ != gamePath) { | ||
fileLoader.reset(ConstructFileLoader(gamePath)); | ||
if (!fileLoader) | ||
return false; | ||
{ | ||
std::lock_guard<std::mutex> guard(loaderLock); | ||
fileLoader.reset(ConstructFileLoader(gamePath)); | ||
if (!fileLoader) | ||
return false; | ||
} | ||
filePath_ = gamePath; | ||
|
||
// This is a fallback title, while we're loading / if unable to load. | ||
|
@@ -215,13 +218,18 @@ std::shared_ptr<FileLoader> GameInfo::GetFileLoader() { | |
// because Priority() calls GetFileLoader()... gnarly. | ||
return fileLoader; | ||
} | ||
|
||
std::lock_guard<std::mutex> guard(loaderLock); | ||
if (!fileLoader) { | ||
fileLoader.reset(ConstructFileLoader(filePath_)); | ||
FileLoader *loader = ConstructFileLoader(filePath_); | ||
fileLoader.reset(loader); | ||
return fileLoader; | ||
} | ||
return fileLoader; | ||
} | ||
|
||
void GameInfo::DisposeFileLoader() { | ||
std::lock_guard<std::mutex> guard(loaderLock); | ||
fileLoader.reset(); | ||
} | ||
|
||
|
@@ -365,7 +373,7 @@ class GameInfoWorkItem : public Task { | |
} | ||
|
||
// In case of a remote file, check if it actually exists before locking. | ||
if (!info_->GetFileLoader()->Exists()) { | ||
if (!info_->GetFileLoader() || !info_->GetFileLoader()->Exists()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hm. I think we now schedule these on multiple threads. There might be a race condition now where -[Unknown] There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, that's a good catch, very likely what happens.. |
||
return; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, if anything I'd be worried about a lifetime issue, i.e. the std::string passed to
PopupMultiChoiceDynamic
could be dead and this could be a use-after-free if the callback somehow happens late.-[Unknown]