diff --git a/intl/msg_hash_us.h b/intl/msg_hash_us.h index d9fd906c42..7eb7156911 100644 --- a/intl/msg_hash_us.h +++ b/intl/msg_hash_us.h @@ -34408,6 +34408,10 @@ MSG_HASH( MSG_CORE_RESTORATION_DISABLED, "Core restoration disabled - core is locked: " ) +MSG_HASH( + MSG_CORE_BACKUP_IN_PROGRESS, + "A backup or restore of this core is already running: " + ) MSG_HASH( MSG_CORE_INSTALLATION_DISABLED, "Core installation disabled - core is locked: " diff --git a/menu/cbs/menu_cbs_ok.c b/menu/cbs/menu_cbs_ok.c index 53a916a5b9..e75940e284 100644 --- a/menu/cbs/menu_cbs_ok.c +++ b/menu/cbs/menu_cbs_ok.c @@ -6340,6 +6340,7 @@ static int action_ok_sideload_core(const char *path, { const char *menu_path = NULL; bool core_loaded = false; + bool pushed; menu_handle_t *menu = menu_state_get_ptr()->driver_data; settings_t *settings = config_get_ptr(); const char *dir_libretro = settings->paths.directory_libretro; @@ -6356,16 +6357,19 @@ static int action_ok_sideload_core(const char *path, char backup_path[PATH_MAX_LENGTH]; fill_pathname_join_special( backup_path, menu_path, path, sizeof(backup_path)); - task_push_core_restore(backup_path, dir_libretro, &core_loaded); + pushed = task_push_core_restore(backup_path, dir_libretro, &core_loaded); } else - task_push_core_restore(path, dir_libretro, &core_loaded); + pushed = task_push_core_restore(path, dir_libretro, &core_loaded); /* Flush stack * > Since the 'sideload core' option is present * in several locations, can't flush to a predefined - * level - just go to the top */ - menu_entries_flush_stack(NULL, 0); + * level - just go to the top + * > Only for a sideload that started: one refused leaves the + * user where they were, with the reason on screen */ + if (pushed) + menu_entries_flush_stack(NULL, 0); return 0; } diff --git a/msg_hash.h b/msg_hash.h index 2dfe934703..d111c15976 100644 --- a/msg_hash.h +++ b/msg_hash.h @@ -27907,6 +27907,7 @@ enum msg_hash_enums MSG_CORE_RESTORATION_FAILED, MSG_CORE_INSTALLATION_FAILED, MSG_CORE_RESTORATION_DISABLED, + MSG_CORE_BACKUP_IN_PROGRESS, MSG_CORE_INSTALLATION_DISABLED, MENU_LABEL(DISK_INDEX), diff --git a/samples/tasks/core_backup/core_backup_io_test.c b/samples/tasks/core_backup/core_backup_io_test.c index 16180bd3dd..43e32f1e23 100644 --- a/samples/tasks/core_backup/core_backup_io_test.c +++ b/samples/tasks/core_backup/core_backup_io_test.c @@ -244,13 +244,18 @@ bool retroarch_ctl(enum rarch_ctl_state state, void *data) return false; } +/* What was last put on screen */ +static int g_osd_calls; +static char g_osd_last[256]; void runloop_msg_queue_push(const char *msg, size_t len, unsigned prio, unsigned duration, bool flush, char *title, enum message_queue_icon icon, enum message_queue_category category) { - (void)msg; (void)len; (void)prio; (void)duration; (void)flush; + (void)len; (void)prio; (void)duration; (void)flush; (void)title; (void)icon; (void)category; + g_osd_calls++; + strlcpy(g_osd_last, msg ? msg : "", sizeof(g_osd_last)); } core_info_t *core_info_find_stub(void) { return NULL; } @@ -1151,9 +1156,10 @@ static void test_restore_through_symlink(void) * finder reading freed memory for the whole stretch between the * worker finishing the task and the main thread retiring it. The * lane opens that stretch deliberately - threaded queue, no - * task_queue_check() - and pushes the duplicate into it; ASan is the - * assertion for the read, and the push must be refused while the - * first backup is still findable and accepted once it is retired. */ + * task_queue_check() - and pushes a second backup into it; ASan is + * the assertion for the read. A finished backup has done its work, + * and the second must be taken: it was refused without a word, so + * Backup pressed again just after one completed did nothing. */ static bool finished_task_finder(retro_task_t *task, void *user_data) { (void)user_data; @@ -1221,10 +1227,10 @@ static void test_finished_task_stays_findable(void) retro_sleep(1); CHECK(i < 10000, "backup task did not finish"); - CHECK(!task_push_core_backup(g_core_path, "Test Core", 0, + CHECK(task_push_core_backup(g_core_path, "Test Core", 0, CORE_BACKUP_MODE_MANUAL, 0, g_assets_dir, true, - NULL, NULL), - "duplicate backup accepted while the first is still findable"); + NULL, NULL) != NULL, + "a backup refused because the finished one was not yet retired"); CHECK(g_done_calls == 0, "completion callback ran before retirement"); @@ -1245,6 +1251,41 @@ static void test_finished_task_stays_findable(void) task_queue_unset_threaded(); } +/* A backup of a core whose backup has not run yet: refused, as two at + * once are not allowed, and the user is told why instead of the entry + * doing nothing. */ +static void test_running_backup_refused_and_told(void) +{ + uint8_t *payload; + size_t core_size = 64 * 1024; + + printf(" a backup of a core being backed up is refused, and said\n"); + if (!(payload = make_payload(core_size))) + return; + if (!write_file_bytes(g_core_path, payload, core_size)) + { + free(payload); + return; + } + free(payload); + + g_clock_now = 1000000; + g_clock_step = 0; + task_queue_init(false, NULL); + CHECK(task_push_core_backup(g_core_path, "Test Core", 0, + CORE_BACKUP_MODE_MANUAL, 0, g_assets_dir, true, + NULL, NULL) != NULL, "the first backup was not pushed"); + g_osd_calls = 0; + g_osd_last[0] = '\0'; + CHECK(!task_push_core_backup(g_core_path, "Test Core", 0, + CORE_BACKUP_MODE_MANUAL, 0, g_assets_dir, true, + NULL, NULL), "a second backup of the same core was taken"); + CHECK(g_osd_calls == 1 && *g_osd_last, + "the refused backup said nothing on screen"); + task_queue_wait(NULL, NULL); + task_queue_deinit(); +} + int main(int argc, char **argv) { (void)argc; (void)argv; @@ -1296,6 +1337,7 @@ int main(int argc, char **argv) printf("\n[task lifetime]\n"); test_finished_task_stays_findable(); + test_running_backup_refused_and_told(); printf("\n%s (%d check%s, %d failure%s)\n", failures ? "FAILED" : "PASSED", diff --git a/tasks/task_core_backup.c b/tasks/task_core_backup.c index d554750db1..e883f2fce7 100644 --- a/tasks/task_core_backup.c +++ b/tasks/task_core_backup.c @@ -297,6 +297,13 @@ static bool task_core_backup_finder(retro_task_t *task, void *user_data) && (task->handler != task_core_restore_handler)) return false; + /* A finished backup or restore has done its file work, though it + * stays in the queue until its callback has run: a second one of + * the same core pushed meanwhile was refused without a word. Passed + * over before its handle is read, which may already be gone. */ + if (task_get_flags(task) & RETRO_TASK_FLG_FINISHED) + return false; + backup_handle = (core_backup_handle_t*)task->state; if (!backup_handle) return false; @@ -314,6 +321,29 @@ static bool task_core_backup_finder(retro_task_t *task, void *user_data) return string_is_equal(core_filename_a, core_filename_b); } +/* Concurrent backup/restore tasks for the same core are not allowed: + * true when one is running, and the user is told rather than the + * Backup or Restore entry doing nothing. */ +static bool task_core_backup_busy(const char *core_path) +{ + task_finder_data_t find_data; + find_data.func = task_core_backup_finder; + find_data.userdata = (void*)core_path; + + if (task_queue_find(&find_data)) + { + char msg[128]; + size_t _len = strlcpy(msg, + msg_hash_to_str(MSG_CORE_BACKUP_IN_PROGRESS), sizeof(msg)); + strlcpy(msg + _len, path_basename(core_path), sizeof(msg) - _len); + RARCH_LOG("[Core Backup] %s\n", msg); + runloop_msg_queue_push(msg, strlen(msg), 1, 100, true, NULL, + MESSAGE_QUEUE_ICON_DEFAULT, MESSAGE_QUEUE_CATEGORY_INFO); + return true; + } + return false; +} + /***************/ /* Core Backup */ /***************/ @@ -843,7 +873,6 @@ static void *task_push_core_backup_internal( retro_task_callback_t cb, void *user_data) { size_t _len; - task_finder_data_t find_data; const char *core_name = NULL; retro_task_t *task = NULL; core_backup_handle_t *backup_handle = NULL; @@ -856,10 +885,7 @@ static void *task_push_core_backup_internal( /* Concurrent backup/restore tasks for the same core * are not allowed */ - find_data.func = task_core_backup_finder; - find_data.userdata = (void*)core_path; - - if (task_queue_find(&find_data)) + if (task_core_backup_busy(core_path)) goto error; /* Get core name */ @@ -1481,7 +1507,6 @@ bool task_push_core_restore(const char *backup_path, const char *dir_libretro, bool *core_loaded) { size_t _len; - task_finder_data_t find_data; enum core_backup_type backup_type; core_info_t *core_info = NULL; const char *core_name = NULL; @@ -1567,10 +1592,7 @@ bool task_push_core_restore(const char *backup_path, const char *dir_libretro, /* Concurrent backup/restore tasks for the same core * are not allowed */ - find_data.func = task_core_backup_finder; - find_data.userdata = (void*)core_path; - - if (task_queue_find(&find_data)) + if (task_core_backup_busy(core_path)) goto error; /* Configure handle */