Fix async gui race condition

If apps and achievements are allowed to be retrieved in parallel,
we need to use different data for each of them. Otherwise they
use each other's data and corrupt each other.

fixes #46
This commit is contained in:
William Pierce 2019-12-28 18:06:39 -05:00
parent cbc95839b1
commit 579eaa3d72
2 changed files with 57 additions and 52 deletions

View File

@ -21,87 +21,90 @@ AsyncGuiLoader::AsyncGuiLoader(MainPickerWindow* window)
bool bool
AsyncGuiLoader::load_achievements_idle() AsyncGuiLoader::load_achievements_idle()
{ {
if (m_idle_data.state == ACH_STATE_STARTED) { if (m_achievement_idle_data.state == ACH_STATE_STARTED) {
g_perfmon->log("Starting achievement retrieval"); g_perfmon->log("Starting achievement retrieval");
m_achievements_future = std::async(std::launch::async, []{g_steam->refresh_achievements();}); m_achievements_future = std::async(std::launch::async, []{g_steam->refresh_achievements();});
m_idle_data.state = ACH_STATE_WAITING_FOR_ACHIEVEMENTS; m_achievement_idle_data.state = ACH_STATE_WAITING_FOR_ACHIEVEMENTS;
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == ACH_STATE_WAITING_FOR_ACHIEVEMENTS) { if (m_achievement_idle_data.state == ACH_STATE_WAITING_FOR_ACHIEVEMENTS) {
if (m_achievements_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) { if (m_achievements_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) {
g_perfmon->log("Done retrieving achievements"); g_perfmon->log("Done retrieving achievements");
// Fire off the schema parsing now. // Fire off the schema parsing now.
// TODO: figure out if all the icons are already there and skip parsing schema // TODO: figure out if all the icons are already there and skip parsing schema
m_schema_parser_future = std::async(std::launch::async, [this]{return m_schema_parser.load_user_game_stats_schema();}); m_schema_parser_future = std::async(std::launch::async, [this]{return m_schema_parser.load_user_game_stats_schema();});
m_idle_data.state = ACH_STATE_LOADING_GUI; m_achievement_idle_data.state = ACH_STATE_LOADING_GUI;
} }
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == ACH_STATE_LOADING_GUI) { if (m_achievement_idle_data.state == ACH_STATE_LOADING_GUI) {
if (m_idle_data.current_item == g_steam->get_achievements().size()) { if (m_achievement_idle_data.current_item == g_steam->get_achievements().size()) {
g_perfmon->log("Done adding achievements to GUI"); g_perfmon->log("Done adding achievements to GUI");
m_window->confirm_achievement_list(); m_window->confirm_achievement_list();
if ( g_steam->get_achievements().size() > 1000 ) { if ( g_steam->get_achievements().size() > 1000 ) {
// The game is an achievement printer of some kind, downloading the icons // The game is an achievement printer of some kind, downloading the icons
// leads to serious performance issues and takes a lot of cache // leads to serious performance issues and takes a lot of cache
std::cerr << "App has excessive achievements, skipping icon downloads" << std::endl;
g_perfmon->log("Achievements retrieved, no icons (Achievement farming app)."); g_perfmon->log("Achievements retrieved, no icons (Achievement farming app).");
m_achievement_idle_data.state = ACH_STATE_FINISHED;
m_window->show_no_achievements_found_placeholder();
m_achievement_refresh_lock.unlock(); m_achievement_refresh_lock.unlock();
return G_SOURCE_REMOVE; return G_SOURCE_REMOVE;
} }
m_idle_data.state = ACH_STATE_WAITING_FOR_SCHEMA_PARSER; m_achievement_idle_data.state = ACH_STATE_WAITING_FOR_SCHEMA_PARSER;
m_idle_data.current_item = 0; m_achievement_idle_data.current_item = 0;
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
auto achievement = g_steam->get_achievements()[m_idle_data.current_item]; auto achievement = g_steam->get_achievements()[m_achievement_idle_data.current_item];
m_window->add_to_achievement_list(achievement); m_window->add_to_achievement_list(achievement);
m_idle_data.current_item++; m_achievement_idle_data.current_item++;
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == ACH_STATE_WAITING_FOR_SCHEMA_PARSER) { if (m_achievement_idle_data.state == ACH_STATE_WAITING_FOR_SCHEMA_PARSER) {
if (m_schema_parser_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) { if (m_schema_parser_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) {
g_perfmon->log("Done parsing schema to find achievement icon download names"); g_perfmon->log("Done parsing schema to find achievement icon download names");
if (!m_schema_parser_future.get()) { if (!m_schema_parser_future.get()) {
std::cerr << "Schema parsing failed, skipping icon downloads" << std::endl; std::cerr << "Schema parsing failed, skipping icon downloads" << std::endl;
m_idle_data.state = ACH_STATE_FINISHED; m_achievement_idle_data.state = ACH_STATE_FINISHED;
g_perfmon->log("Achievements retrieved, no icons."); g_perfmon->log("Achievements retrieved, no icons.");
m_window->show_no_achievements_found_placeholder(); m_window->show_no_achievements_found_placeholder();
m_achievement_refresh_lock.unlock(); m_achievement_refresh_lock.unlock();
return G_SOURCE_REMOVE; return G_SOURCE_REMOVE;
} }
m_idle_data.state = ACH_STATE_DOWNLOADING_ICONS; m_achievement_idle_data.state = ACH_STATE_DOWNLOADING_ICONS;
} }
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == ACH_STATE_DOWNLOADING_ICONS) { if (m_achievement_idle_data.state == ACH_STATE_DOWNLOADING_ICONS) {
// this could hang if we failed to parse all the icon download names // this could hang if we failed to parse all the icon download names
bool done_starting_downloads = (m_idle_data.current_item == g_steam->get_achievements().size()); bool done_starting_downloads = (m_achievement_idle_data.current_item == g_steam->get_achievements().size());
if (done_starting_downloads && (m_achievement_icon_download_futures.size() == 0)) { if (done_starting_downloads && (m_achievement_icon_download_futures.size() == 0)) {
m_idle_data.state = ACH_STATE_FINISHED; m_achievement_idle_data.state = ACH_STATE_FINISHED;
g_perfmon->log("Achievements retrieved with icons."); g_perfmon->log("Achievements retrieved with icons.");
m_window->show_no_achievements_found_placeholder(); m_window->show_no_achievements_found_placeholder();
m_achievement_refresh_lock.unlock(); m_achievement_refresh_lock.unlock();
// See top of the file // See top of the file
// VALGRIND_MONITOR_COMMAND("detailed_snapshot"); // VALGRIND_MONITOR_COMMAND("detailed_snapshot");
return G_SOURCE_REMOVE; return G_SOURCE_REMOVE;
} }
if ( !done_starting_downloads && (m_concurrent_icon_downloads < MAX_CONCURRENT_ICON_DOWNLOADS)) { if ( !done_starting_downloads && (m_concurrent_icon_downloads < MAX_CONCURRENT_ICON_DOWNLOADS)) {
// Fire off a new download thread // Fire off a new download thread
std::string id = g_steam->get_achievements()[m_idle_data.current_item].id; std::string id = g_steam->get_achievements()[m_achievement_idle_data.current_item].id;
std::string icon_download_name = m_schema_parser.get_icon_download_names()[id]; std::string icon_download_name = m_schema_parser.get_icon_download_names()[id];
// Assuming it returns empty string on failing to lookup // Assuming it returns empty string on failing to lookup
if (icon_download_name.empty()) { if (icon_download_name.empty()) {
std::cerr << "Failed to lookup achievement icon name: " << id << std::endl; std::cerr << "Failed to lookup achievement icon name: " << id << std::endl;
@ -111,7 +114,7 @@ AsyncGuiLoader::load_achievements_idle()
m_concurrent_icon_downloads++; m_concurrent_icon_downloads++;
} }
m_idle_data.current_item++; m_achievement_idle_data.current_item++;
// continue on to service a thread if it's finished // continue on to service a thread if it's finished
} }
@ -137,8 +140,8 @@ AsyncGuiLoader::load_achievements_idle()
void void
AsyncGuiLoader::populate_achievements() { AsyncGuiLoader::populate_achievements() {
if (m_achievement_refresh_lock.try_lock()) { if (m_achievement_refresh_lock.try_lock()) {
m_idle_data.current_item = 0; m_achievement_idle_data.current_item = 0;
m_idle_data.state = ACH_STATE_STARTED; m_achievement_idle_data.state = ACH_STATE_STARTED;
m_concurrent_icon_downloads = 0; m_concurrent_icon_downloads = 0;
m_window->reset_achievements_list(); m_window->reset_achievements_list();
m_window->show_fetch_achievements_placeholder(); m_window->show_fetch_achievements_placeholder();
@ -157,12 +160,12 @@ AsyncGuiLoader::populate_achievements() {
/* /*
* The actual loading function * The actual loading function
* *
* We CANNOT update the GUI from any thread but the main thread because * We CANNOT update the GUI from any thread but the main thread because
* it is explicitly deprecated in gtk... * it is explicitly deprecated in gtk...
* (e.g. https://developer.gnome.org/gdk3/stable/gdk3-Threads.html#gdk-threads-init) * (e.g. https://developer.gnome.org/gdk3/stable/gdk3-Threads.html#gdk-threads-init)
* See the AsyncGuiLoader.h for the FSM rationale. * See the AsyncGuiLoader.h for the FSM rationale.
* *
* For anything that isn't the GUI, we can fire off worker threads, and doing * For anything that isn't the GUI, we can fire off worker threads, and doing
* so is simpler than splitting out the worker threads into the main GUI loop. * so is simpler than splitting out the worker threads into the main GUI loop.
* Additionally, the worker threads depend on calling functions which may * Additionally, the worker threads depend on calling functions which may
@ -172,48 +175,48 @@ AsyncGuiLoader::populate_achievements() {
bool bool
AsyncGuiLoader::load_apps_idle() AsyncGuiLoader::load_apps_idle()
{ {
if (m_idle_data.state == APPS_STATE_STARTED) { if (m_app_idle_data.state == APPS_STATE_STARTED) {
m_window->reset_game_list(); m_window->reset_game_list();
g_perfmon->log("Starting library parsing."); g_perfmon->log("Starting library parsing.");
m_owned_apps_future = std::async(std::launch::async, []{g_steam->refresh_owned_apps();}); m_owned_apps_future = std::async(std::launch::async, []{g_steam->refresh_owned_apps();});
m_idle_data.state = APPS_STATE_WAITING_FOR_OWNED_APPS; m_app_idle_data.state = APPS_STATE_WAITING_FOR_OWNED_APPS;
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == APPS_STATE_WAITING_FOR_OWNED_APPS) { if (m_app_idle_data.state == APPS_STATE_WAITING_FOR_OWNED_APPS) {
if (m_owned_apps_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) { if (m_owned_apps_future.wait_for(std::chrono::seconds(0)) == std::future_status::ready) {
g_perfmon->log("Done retrieving and filtering owned apps"); g_perfmon->log("Done retrieving and filtering owned apps");
m_idle_data.state = APPS_STATE_LOADING_GUI; m_app_idle_data.state = APPS_STATE_LOADING_GUI;
} }
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == APPS_STATE_LOADING_GUI) { if (m_app_idle_data.state == APPS_STATE_LOADING_GUI) {
if (m_idle_data.current_item == g_steam->get_subscribed_apps().size()) { if (m_app_idle_data.current_item == g_steam->get_subscribed_apps().size()) {
g_perfmon->log("Done adding apps to GUI"); g_perfmon->log("Done adding apps to GUI");
m_window->confirm_game_list(); m_window->confirm_game_list();
m_idle_data.state = APPS_STATE_DOWNLOADING_ICONS; m_app_idle_data.state = APPS_STATE_DOWNLOADING_ICONS;
m_idle_data.current_item = 0; m_app_idle_data.current_item = 0;
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
Game_t app = g_steam->get_subscribed_apps()[m_idle_data.current_item]; Game_t app = g_steam->get_subscribed_apps()[m_app_idle_data.current_item];
m_window->add_to_game_list(app); m_window->add_to_game_list(app);
m_idle_data.current_item++; m_app_idle_data.current_item++;
return G_SOURCE_CONTINUE; return G_SOURCE_CONTINUE;
} }
if (m_idle_data.state == APPS_STATE_DOWNLOADING_ICONS) { if (m_app_idle_data.state == APPS_STATE_DOWNLOADING_ICONS) {
// This must occur after the main gui game_list is // This must occur after the main gui game_list is
// complete, otherwise we might have concurrent // complete, otherwise we might have concurrent
// access and modification of the GUI's game_list // access and modification of the GUI's game_list
bool done_starting_downloads = (m_idle_data.current_item == g_steam->get_subscribed_apps().size()); bool done_starting_downloads = (m_app_idle_data.current_item == g_steam->get_subscribed_apps().size());
// Make sure we're done starting all downloads and finshed with outstanding downloads // Make sure we're done starting all downloads and finshed with outstanding downloads
if (done_starting_downloads && (m_app_icon_download_futures.size() == 0)) { if (done_starting_downloads && (m_app_icon_download_futures.size() == 0)) {
g_perfmon->log("Done downloading app icons"); g_perfmon->log("Done downloading app icons");
m_idle_data.state = APPS_STATE_FINISHED; m_app_idle_data.state = APPS_STATE_FINISHED;
m_window->show_no_games_found_placeholder(); m_window->show_no_games_found_placeholder();
m_game_refresh_lock.unlock(); m_game_refresh_lock.unlock();
return G_SOURCE_REMOVE; return G_SOURCE_REMOVE;
@ -232,10 +235,10 @@ AsyncGuiLoader::load_apps_idle()
// so only 1 thread will ever be here at a time anyway. // so only 1 thread will ever be here at a time anyway.
if ( !done_starting_downloads && (m_concurrent_icon_downloads < MAX_CONCURRENT_ICON_DOWNLOADS)) { if ( !done_starting_downloads && (m_concurrent_icon_downloads < MAX_CONCURRENT_ICON_DOWNLOADS)) {
// Fire off a new download thread // Fire off a new download thread
Game_t app = g_steam->get_subscribed_apps()[m_idle_data.current_item]; Game_t app = g_steam->get_subscribed_apps()[m_app_idle_data.current_item];
m_app_icon_download_futures.insert(std::make_pair(app.app_id, std::async(std::launch::async, g_steam->refresh_app_icon, app.app_id))); m_app_icon_download_futures.insert(std::make_pair(app.app_id, std::async(std::launch::async, g_steam->refresh_app_icon, app.app_id)));
m_concurrent_icon_downloads++; m_concurrent_icon_downloads++;
m_idle_data.current_item++; m_app_idle_data.current_item++;
// continue on to service a thread if it's finished // continue on to service a thread if it's finished
} }
@ -262,11 +265,11 @@ AsyncGuiLoader::load_apps_idle()
} }
// => load_apps_idle // => load_apps_idle
void void
AsyncGuiLoader::populate_apps() { AsyncGuiLoader::populate_apps() {
if (m_game_refresh_lock.try_lock()) { if (m_game_refresh_lock.try_lock()) {
m_idle_data.current_item = 0; m_app_idle_data.current_item = 0;
m_idle_data.state = APPS_STATE_STARTED; m_app_idle_data.state = APPS_STATE_STARTED;
m_concurrent_icon_downloads = 0; m_concurrent_icon_downloads = 0;
m_window->show_fetch_games_placeholder(); m_window->show_fetch_games_placeholder();
@ -277,4 +280,4 @@ AsyncGuiLoader::populate_apps() {
std::cerr << "Not refreshing games because a refresh is already in progress" << std::endl; std::cerr << "Not refreshing games because a refresh is already in progress" << std::endl;
} }
} }
// => on_refresh_games_button_clicked // => on_refresh_games_button_clicked

View File

@ -39,20 +39,20 @@ typedef struct
unsigned state; unsigned state;
/** /**
* other information is just pulled from the global * other information is pulled from the
* or other singleton info for now * AsyncGuiLoader object
*/ */
/* the currently loaded item */ /* the currently loaded item */
unsigned current_item; unsigned current_item;
} IdleData; } IdleData;
class AsyncGuiLoader class AsyncGuiLoader
{ {
public: public:
AsyncGuiLoader(MainPickerWindow* window); AsyncGuiLoader(MainPickerWindow* window);
/** /**
* When the user wants to refresh the game list. * When the user wants to refresh the game list.
* This is also called when the main window just got spawned. * This is also called when the main window just got spawned.
* - Clear the game list (will show the loading widget) * - Clear the game list (will show the loading widget)
@ -62,10 +62,10 @@ public:
* to display the app logos. * to display the app logos.
* - Draw the result. * - Draw the result.
*/ */
void void
populate_apps(); populate_apps();
void void
populate_achievements(); populate_achievements();
private: private:
bool load_achievements_idle(); bool load_achievements_idle();
@ -77,7 +77,7 @@ private:
std::future<void> m_owned_apps_future; std::future<void> m_owned_apps_future;
std::future<void> m_achievements_future; std::future<void> m_achievements_future;
std::future<bool> m_schema_parser_future; std::future<bool> m_schema_parser_future;
/** /**
* Mutex to prevent on_refresh_games_button_clicked from being reentrant * Mutex to prevent on_refresh_games_button_clicked from being reentrant
* and allowing multiple idle threads to corrupt the main window. * and allowing multiple idle threads to corrupt the main window.
@ -85,7 +85,9 @@ private:
std::mutex m_game_refresh_lock; std::mutex m_game_refresh_lock;
std::mutex m_achievement_refresh_lock; std::mutex m_achievement_refresh_lock;
IdleData m_idle_data; IdleData m_app_idle_data;
IdleData m_achievement_idle_data;
UserGameStatsSchemaParser m_schema_parser; UserGameStatsSchemaParser m_schema_parser;
MainPickerWindow* m_window; MainPickerWindow* m_window;
}; };