Skip to content

Commit 8108e63

Browse files
flyingheadflyinghead
committed
rcheevos: race condition in event handlers and async tasks
The data passed to event handlers (achievements in particular) must be copied if used by an async task. It might be deleted before the async task runs. Fixes MINIDUMP-3ZR MINIDUMP-4XJ MINIDUMP-4TB MINIDUMP-4CA MINIDUMP-38H
1 parent b0403ed commit 8108e63

File tree

1 file changed

+75
-64
lines changed

1 file changed

+75
-64
lines changed

Diff for: ‎core/achievements/achievements.cpp

+75-64
Original file line numberDiff line numberDiff line change
@@ -365,16 +365,16 @@ void Achievements::term()
365365
void Achievements::authenticationSuccess(const rc_client_user_t *user)
366366
{
367367
NOTICE_LOG(COMMON, "RA Login successful");
368-
asyncTask([this, user]() {
369-
char url[512];
370-
int rc = rc_client_user_get_image_url(user, url, sizeof(url));
371-
if (rc == RC_OK)
372-
{
373-
std::string image = getOrDownloadImage(url);
368+
std::string url(512, '\0');
369+
int rc = rc_client_user_get_image_url(user, url.data(), url.size());
370+
if (rc == RC_OK)
371+
{
372+
asyncTask([this, url]() {
373+
std::string image = getOrDownloadImage(url.c_str());
374374
std::string text = "User " + config::AchievementsUserName.get() + " authenticated";
375375
notifier.notify(Notification::Login, image, text);
376-
}
377-
});
376+
});
377+
}
378378
loggedOn = true;
379379
if (!settings.content.fileName.empty()) // TODO better test?
380380
loadGame();
@@ -600,46 +600,48 @@ void Achievements::handleUnlockEvent(const rc_client_event_t *event)
600600
assert(cheevo != nullptr);
601601

602602
INFO_LOG(COMMON, "RA: Achievement %s (%u) for game %s unlocked", cheevo->title, cheevo->id, settings.content.title.c_str());
603-
asyncTask([this, cheevo]() {
604-
char url[512];
605-
int rc = rc_client_achievement_get_image_url(cheevo, cheevo->state, url, sizeof(url));
606-
if (rc == RC_OK)
607-
{
608-
std::string image = getOrDownloadImage(url);
609-
std::string text = "Achievement " + std::string(cheevo->title) + " unlocked!";
610-
notifier.notify(Notification::Login, image, text, cheevo->description);
611-
}
612-
});
603+
std::string title(cheevo->title);
604+
std::string description(cheevo->description);
605+
std::string url(512, '\0');
606+
int rc = rc_client_achievement_get_image_url(cheevo, cheevo->state, url.data(), url.size());
607+
if (rc == RC_OK)
608+
{
609+
asyncTask([this, url, title, description]() {
610+
std::string image = getOrDownloadImage(url.c_str());
611+
std::string text = "Achievement " + title + " unlocked!";
612+
notifier.notify(Notification::Login, image, text, description);
613+
});
614+
}
613615
}
614616

615617
void Achievements::handleAchievementChallengeIndicatorShowEvent(const rc_client_event_t *event)
616618
{
617619
const rc_client_achievement_t* cheevo = event->achievement;
618620
INFO_LOG(COMMON, "RA: Challenge: %s", cheevo->title);
619-
asyncTask([this, cheevo]() {
620-
char url[128];
621-
int rc = rc_client_achievement_get_image_url(cheevo, RC_CLIENT_ACHIEVEMENT_STATE_UNLOCKED, url, sizeof(url));
622-
if (rc == RC_OK)
623-
{
624-
std::string image = getOrDownloadImage(url);
621+
std::string url(512, '\0');
622+
int rc = rc_client_achievement_get_image_url(cheevo, RC_CLIENT_ACHIEVEMENT_STATE_UNLOCKED, url.data(), url.size());
623+
if (rc == RC_OK)
624+
{
625+
asyncTask([this, url]() {
626+
std::string image = getOrDownloadImage(url.c_str());
625627
notifier.showChallenge(image);
626-
}
627-
});
628+
});
629+
}
628630
}
629631

630632
void Achievements::handleAchievementChallengeIndicatorHideEvent(const rc_client_event_t *event)
631633
{
632634
const rc_client_achievement_t* cheevo = event->achievement;
633635
INFO_LOG(COMMON, "RA: Challenge hidden: %s", cheevo->title);
634-
asyncTask([this, cheevo]() {
635-
char url[128];
636-
int rc = rc_client_achievement_get_image_url(cheevo, RC_CLIENT_ACHIEVEMENT_STATE_UNLOCKED, url, sizeof(url));
637-
if (rc == RC_OK)
638-
{
639-
std::string image = getOrDownloadImage(url);
636+
std::string url(512, '\0');
637+
int rc = rc_client_achievement_get_image_url(cheevo, RC_CLIENT_ACHIEVEMENT_STATE_UNLOCKED, url.data(), url.size());
638+
if (rc == RC_OK)
639+
{
640+
asyncTask([this, url]() {
641+
std::string image = getOrDownloadImage(url.c_str());
640642
notifier.hideChallenge(image);
641-
}
642-
});
643+
});
644+
}
643645
}
644646

645647
void Achievements::handleLeaderboardStarted(const rc_client_event_t *event)
@@ -685,18 +687,21 @@ void Achievements::handleUpdateLeaderboardTracker(const rc_client_event_t *event
685687
void Achievements::handleGameCompleted(const rc_client_event_t *event)
686688
{
687689
const rc_client_game_t* game = rc_client_get_game_info(rc_client);
688-
asyncTask([this, game]() {
690+
std::string text1 = (rc_client_get_hardcore_enabled(rc_client) ? "Mastered " : "Completed ") + std::string(game->title);
691+
rc_client_user_game_summary_t summary;
692+
rc_client_get_user_game_summary(rc_client, &summary);
693+
std::stringstream ss;
694+
ss << summary.num_unlocked_achievements << " achievements, " << summary.points_unlocked << " points";
695+
std::string text2(ss.str());
696+
std::string text3 = rc_client_get_user_info(rc_client)->display_name;
697+
std::string url(512, '\0');
698+
if (rc_client_game_get_image_url(game, url.data(), url.size()) != RC_OK)
699+
url.clear();
700+
asyncTask([this, url, text1, text2, text3]() {
689701
std::string image;
690-
char url[128];
691-
if (rc_client_game_get_image_url(game, url, sizeof(url)) == RC_OK)
692-
image = getOrDownloadImage(url);
693-
std::string text1 = (rc_client_get_hardcore_enabled(rc_client) ? "Mastered " : "Completed ") + std::string(game->title);
694-
rc_client_user_game_summary_t summary;
695-
rc_client_get_user_game_summary(rc_client, &summary);
696-
std::stringstream ss;
697-
ss << summary.num_unlocked_achievements << " achievements, " << summary.points_unlocked << " points";
698-
std::string text3 = rc_client_get_user_info(rc_client)->display_name;
699-
notifier.notify(Notification::Mastery, image, text1, ss.str(), text3);
702+
if (!url.empty())
703+
image = getOrDownloadImage(url.c_str());
704+
notifier.notify(Notification::Mastery, image, text1, text2, text3);
700705
});
701706
}
702707

@@ -711,12 +716,15 @@ void Achievements::handleHideAchievementProgress(const rc_client_event_t *event)
711716
void Achievements::handleUpdateAchievementProgress(const rc_client_event_t *event)
712717
{
713718
const rc_client_achievement_t* cheevo = event->achievement;
714-
asyncTask([this, cheevo]() {
715-
char url[128];
719+
std::string url(512, '\0');
720+
if (rc_client_achievement_get_image_url(cheevo, RC_CLIENT_ACHIEVEMENT_STATE_ACTIVE, url.data(), url.size()) != RC_OK)
721+
url.clear();
722+
std::string progress(cheevo->measured_progress);
723+
asyncTask([this, url, progress]() {
716724
std::string image;
717-
if (rc_client_achievement_get_image_url(cheevo, RC_CLIENT_ACHIEVEMENT_STATE_ACTIVE, url, sizeof(url)) == RC_OK)
718-
image = getOrDownloadImage(url);
719-
notifier.notify(Notification::Progress, image, cheevo->measured_progress);
725+
if (!url.empty())
726+
image = getOrDownloadImage(url.c_str());
727+
notifier.notify(Notification::Progress, image, progress);
720728
});
721729
}
722730

@@ -919,21 +927,24 @@ void Achievements::gameLoaded(int result, const char *errorMessage)
919927
settings.raHardcoreMode = false;
920928
else
921929
settings.raHardcoreMode = (bool)rc_client_get_hardcore_enabled(rc_client);
922-
asyncTask([this, info]() {
930+
std::string url(512, '\0');
931+
if (rc_client_game_get_image_url(info, url.data(), url.size()) != RC_OK)
932+
url.clear();
933+
std::string text1(info->title);
934+
rc_client_user_game_summary_t summary;
935+
rc_client_get_user_game_summary(rc_client, &summary);
936+
std::string text2;
937+
if (summary.num_core_achievements > 0)
938+
text2 = "You have " + std::to_string(summary.num_unlocked_achievements)
939+
+ " of " + std::to_string(summary.num_core_achievements) + " achievements unlocked.";
940+
else
941+
text2 = "This game has no achievements.";
942+
asyncTask([this, url, text1, text2]() {
923943
std::string image;
924-
char url[512];
925-
if (rc_client_game_get_image_url(info, url, sizeof(url)) == RC_OK)
926-
image = getOrDownloadImage(url);
927-
rc_client_user_game_summary_t summary;
928-
rc_client_get_user_game_summary(rc_client, &summary);
929-
std::string text;
930-
if (summary.num_core_achievements > 0)
931-
text = "You have " + std::to_string(summary.num_unlocked_achievements)
932-
+ " of " + std::to_string(summary.num_core_achievements) + " achievements unlocked.";
933-
else
934-
text = "This game has no achievements.";
935-
std::string text2 = settings.raHardcoreMode ? "Hardcore Mode" : "";
936-
notifier.notify(Notification::Login, image, info->title, text, text2);
944+
if (!url.empty())
945+
image = getOrDownloadImage(url.c_str());
946+
std::string text3 = settings.raHardcoreMode ? "Hardcore Mode" : "";
947+
notifier.notify(Notification::Login, image, text1, text2, text3);
937948
});
938949
}
939950

0 commit comments

Comments
 (0)