Skip to content

Commit

Permalink
fix infinite loop if reset occurs just prior to a ping while progress…
Browse files Browse the repository at this point in the history
… indicator is on screen (#354)
  • Loading branch information
Jamiras authored Jul 27, 2024
1 parent cea3209 commit c1b30ea
Show file tree
Hide file tree
Showing 2 changed files with 121 additions and 2 deletions.
9 changes: 7 additions & 2 deletions src/rc_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -3570,9 +3570,11 @@ static void rc_client_award_achievement(rc_client_t* client, rc_client_achieveme
callback_data->client = client;
callback_data->id = achievement->public_.id;
callback_data->hardcore = client->state.hardcore;
callback_data->game_hash = client->game->public_.hash;
callback_data->unlock_time = achievement->public_.unlock_time;

if (client->game) /* may be NULL if this gets called while unloading the game (from another thread - events are raised outside the lock) */
callback_data->game_hash = client->game->public_.hash;

RC_CLIENT_LOG_INFO_FORMATTED(client, "Awarding achievement %u: %s", achievement->public_.id, achievement->public_.title);
rc_client_award_achievement_server_call(callback_data);
}
Expand Down Expand Up @@ -4797,6 +4799,8 @@ static void rc_client_progress_tracker_timer_elapsed(rc_client_scheduled_callbac

static void rc_client_do_frame_update_progress_tracker(rc_client_t* client, rc_client_game_info_t* game)
{
/* ASSERT: this should only be called if the mutex is held */

if (!game->progress_tracker.hide_callback) {
game->progress_tracker.hide_callback = (rc_client_scheduled_callback_data_t*)
rc_buffer_alloc(&game->buffer, sizeof(rc_client_scheduled_callback_data_t));
Expand Down Expand Up @@ -5181,6 +5185,7 @@ void rc_client_idle(rc_client_t* client)
else {
/* remove the callback from the queue while we process it. callback can requeue if desired */
client->state.scheduled_callbacks = scheduled_callback->next;
scheduled_callback->next = NULL;
}
}
rc_mutex_unlock(&client->state.mutex);
Expand Down Expand Up @@ -5256,7 +5261,7 @@ static void rc_client_reschedule_callback(rc_client_t* client,
continue;
}

if (!next || when < next->when) {
if (!next || (when < next->when && when != 0)) {
/* insert here */
callback->next = next;
*last = callback;
Expand Down
114 changes: 114 additions & 0 deletions test/test_rc_client.c
Original file line number Diff line number Diff line change
Expand Up @@ -6339,6 +6339,58 @@ static void test_do_frame_achievement_measured_progress_event(void)
rc_client_destroy(g_client);
}

static void test_do_frame_achievement_measured_progress_reshown(void)
{
uint8_t memory[64];
memset(memory, 0, sizeof(memory));

g_client = mock_client_game_loaded(patchdata_exhaustive, no_unlocks);
ASSERT_PTR_NOT_NULL(g_client->game);
mock_memory(memory, sizeof(memory));

event_count = 0;
rc_client_do_frame(g_client);
ASSERT_NUM_EQUALS(event_count, 0);

memory[0x06] = 3; /* 3/6 */
rc_client_do_frame(g_client);
ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_SHOW, 6));
event_count = 0;

/* should be two callbacks queued - hiding the progress indicator, and the rich presence update */
ASSERT_PTR_EQUALS(g_client->state.scheduled_callbacks, g_client->game->progress_tracker.hide_callback);
ASSERT_PTR_NOT_NULL(g_client->state.scheduled_callbacks->next);
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next->next);

/* advance time to hide the progress indicator */
g_now = g_client->game->progress_tracker.hide_callback->when;
rc_client_do_frame(g_client);
ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_HIDE, 0));
event_count = 0;

/* only the rich presence update should be scheduled */
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next);

/* advance time to just before the rich presence update */
g_now = g_client->state.scheduled_callbacks->when - 10;

/* reschedule the progress indicator */
memory[0x06] = 4; /* 4/6 */
rc_client_do_frame(g_client);
ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_SHOW, 6));
event_count = 0;

/* should be two callbacks queued - rich presence update, then hiding the progress indicator */
ASSERT_PTR_NOT_NULL(g_client->state.scheduled_callbacks);
ASSERT_PTR_EQUALS(g_client->state.scheduled_callbacks->next, g_client->game->progress_tracker.hide_callback);
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next->next);

rc_client_destroy(g_client);
}

static void test_do_frame_achievement_challenge_indicator(void)
{
rc_client_event_t* event;
Expand Down Expand Up @@ -8248,6 +8300,66 @@ static void test_reset_hides_widgets(void)
rc_client_destroy(g_client);
}

static void test_reset_detaches_hide_progress_indicator_event(void)
{
uint8_t memory[64];
memset(memory, 0, sizeof(memory));

g_client = mock_client_game_loaded(patchdata_exhaustive, no_unlocks);
ASSERT_NUM_EQUALS(rc_client_get_hardcore_enabled(g_client), 1);
mock_memory(memory, sizeof(memory));

rc_client_do_frame(g_client);

memory[0x06] = 3; /* progress indicator for achievement 6 */
event_count = 0;
rc_client_do_frame(g_client);

ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_SHOW, 6));
event_count = 0;

/* should be two callbacks queued - hiding the progress indicator, and the rich presence update */
ASSERT_PTR_EQUALS(g_client->state.scheduled_callbacks, g_client->game->progress_tracker.hide_callback);
ASSERT_PTR_NOT_NULL(g_client->state.scheduled_callbacks->next);
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next->next);

/* advance time to hide the progress indicator */
g_now = g_client->game->progress_tracker.hide_callback->when;
rc_client_do_frame(g_client);
ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_HIDE, 0));
event_count = 0;

/* only the rich presence update should be scheduled */
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next);

/* advance time to just before the rich presence update */
g_now = g_client->state.scheduled_callbacks->when - 10;

/* reschedule the progress indicator */
memory[0x06] = 4; /* 4/6 */
rc_client_do_frame(g_client);
ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_SHOW, 6));
event_count = 0;

/* should be two callbacks queued - rich presence update, then hiding the progress indicator */
ASSERT_PTR_NOT_NULL(g_client->state.scheduled_callbacks);
ASSERT_PTR_EQUALS(g_client->state.scheduled_callbacks->next, g_client->game->progress_tracker.hide_callback);
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next->next);

rc_client_reset(g_client);

ASSERT_NUM_EQUALS(event_count, 1);
ASSERT_PTR_NOT_NULL(find_event(RC_CLIENT_EVENT_ACHIEVEMENT_PROGRESS_INDICATOR_HIDE, 0));

/* only the rich presence update should be scheduled */
ASSERT_PTR_NULL(g_client->state.scheduled_callbacks->next);

rc_client_destroy(g_client);
}

/* ----- pause ----- */

static void test_can_pause(void)
Expand Down Expand Up @@ -9293,6 +9405,7 @@ void test_client(void) {
TEST(test_do_frame_achievement_trigger_rarity);
TEST(test_do_frame_achievement_measured);
TEST(test_do_frame_achievement_measured_progress_event);
TEST(test_do_frame_achievement_measured_progress_reshown);
TEST(test_do_frame_achievement_challenge_indicator);
TEST(test_do_frame_achievement_challenge_indicator_primed_while_reset);
TEST(test_do_frame_achievement_challenge_indicator_primed_while_reset_next);
Expand Down Expand Up @@ -9323,6 +9436,7 @@ void test_client(void) {

/* reset */
TEST(test_reset_hides_widgets);
TEST(test_reset_detaches_hide_progress_indicator_event);

/* pause */
TEST(test_can_pause);
Expand Down

0 comments on commit c1b30ea

Please sign in to comment.