mirror of
https://github.com/hrydgard/ppsspp.git
synced 2026-10-14 23:10:33 +02:00
Achievements: Fix leaderboard screen use-after-free on close
rc_client frees a request's async handle right after calling back. The error path didn't clear pendingAsyncCall_, so closing the screen after a failed fetch aborted a freed handle. Switching between Top players and Around me also overwrote the handle without aborting the old request, which could then call back into a closed screen, or leak an entry list. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
c8b32dda0a
commit
e628fe3cb2
1 file changed
+13
-5
@@ -258,20 +258,28 @@ RetroAchievementsLeaderboardScreen::RetroAchievementsLeaderboardScreen(const Pat
|
||||
|
||||
void RetroAchievementsLeaderboardScreen::FetchEntries() {
|
||||
auto callback = [](int result, const char *error_message, rc_client_leaderboard_entry_list_t *list, rc_client_t *client, void *userdata) {
|
||||
RetroAchievementsLeaderboardScreen *thiz = (RetroAchievementsLeaderboardScreen *)userdata;
|
||||
// rc_client frees the handle after this returns, so it must not be aborted later.
|
||||
thiz->pendingAsyncCall_ = nullptr;
|
||||
if (result != RC_OK) {
|
||||
g_OSD.Show(OSDType::MESSAGE_ERROR, error_message, 10.0f);
|
||||
return;
|
||||
}
|
||||
|
||||
RetroAchievementsLeaderboardScreen *thiz = (RetroAchievementsLeaderboardScreen *)userdata;
|
||||
if (thiz->pendingEntryList_) {
|
||||
rc_client_destroy_leaderboard_entry_list(thiz->pendingEntryList_);
|
||||
}
|
||||
thiz->pendingEntryList_ = list;
|
||||
thiz->pendingAsyncCall_ = nullptr;
|
||||
};
|
||||
|
||||
// An aborted request doesn't call back, so only the latest request can reach the callback.
|
||||
if (pendingAsyncCall_) {
|
||||
rc_client_abort_async(Achievements::GetClient(), pendingAsyncCall_);
|
||||
pendingAsyncCall_ = nullptr;
|
||||
}
|
||||
|
||||
// Store the handle so the destructor can abort it if we're closed before the
|
||||
// response arrives - without this, the callback above would run later against
|
||||
// a freed `this` (its pendingAsyncCall_ null-check in the destructor was
|
||||
// otherwise always a no-op since this was never assigned).
|
||||
// a freed `this`.
|
||||
if (nearMe_) {
|
||||
pendingAsyncCall_ = rc_client_begin_fetch_leaderboard_entries_around_user(Achievements::GetClient(), leaderboardID_, 10, callback, this);
|
||||
} else {
|
||||
|
||||
Reference in new issue
Block a user