From f1e00ccf0e29ba5f1202bae6ced6a404086b1dba Mon Sep 17 00:00:00 2001 From: Martin Michelsen Date: Mon, 1 Jan 2024 00:05:19 -0800 Subject: [PATCH] fix crashes in some non-server actions --- src/Main.cc | 22 +++++++++++----------- src/ServerShell.cc | 2 +- src/ServerState.cc | 16 ++++++++++++---- src/ServerState.hh | 7 +++++-- src/StepGraph.cc | 21 ++++++++++++++++----- src/StepGraph.hh | 5 +++-- 6 files changed, 48 insertions(+), 25 deletions(-) diff --git a/src/Main.cc b/src/Main.cc index 477d855c..c92af79e 100644 --- a/src/Main.cc +++ b/src/Main.cc @@ -1278,7 +1278,7 @@ Action a_print_word_select_table( option is given, prints the token table sorted by canonical name.\n", +[](Arguments& args) { ServerState s; - s.load_objects("word_select_table"); + s.load_objects_and_upstream_dependents("word_select_table"); Version v; try { v = get_cli_version(args); @@ -1335,7 +1335,7 @@ Action a_convert_rare_item_set( auto version = get_cli_version(args); ServerState s; - s.load_objects("item_name_indexes"); + s.load_objects_and_upstream_dependents("item_name_indexes"); string input_filename = args.get(1, false); if (input_filename.empty() || (input_filename == "-")) { @@ -1390,7 +1390,7 @@ Action a_describe_item( auto version = get_cli_version(args); ServerState s; - s.load_objects("item_name_indexes"); + s.load_objects_and_upstream_dependents("item_name_indexes"); auto name_index = s.item_name_index(version); ItemData item = name_index->parse_item_description(description); @@ -1450,7 +1450,7 @@ Action a_describe_item( Action a_name_all_items( "name-all-items", nullptr, +[](Arguments&) { ServerState s; - s.load_objects("item_name_indexes"); + s.load_objects_and_upstream_dependents("item_name_indexes"); set all_primary_identifiers; for (const auto& index : s.item_name_indexes) { @@ -1494,7 +1494,7 @@ Action a_show_ep3_cards( bool one_line = args.get("one-line"); ServerState s; - s.load_objects("ep3_data"); + s.load_objects_and_upstream_dependents("ep3_data"); unique_ptr text_english; try { @@ -1541,8 +1541,8 @@ Action a_generate_ep3_cards_html( size_t num_threads = args.get("threads", 0); ServerState s; - s.load_objects("ep3_data"); - s.load_objects("text_index"); + s.load_objects_and_upstream_dependents("ep3_data"); + s.load_objects_and_upstream_dependents("text_index"); shared_ptr text_english; try { @@ -1678,7 +1678,7 @@ Action a_show_ep3_maps( config_log.info("Collecting Episode 3 data"); ServerState s; - s.load_objects("ep3_data"); + s.load_objects_and_upstream_dependents("ep3_data"); auto map_ids = s.ep3_map_index->all_numbers(); log_info("%zu maps", map_ids.size()); @@ -1702,7 +1702,7 @@ Action a_show_battle_params( in a human-readable format.\n", +[](Arguments&) { ServerState s; - s.load_objects("battle_params"); + s.load_objects_and_upstream_dependents("battle_params"); fprintf(stdout, "Episode 1 multi\n"); s.battle_params->get_table(false, Episode::EP1).print(stdout); @@ -1837,7 +1837,7 @@ Action a_diff_dol_files( Action a_replay_ep3_battle_commands( "replay-ep3-battle-commands", nullptr, +[](Arguments& args) { ServerState s; - s.load_objects("ep3_data"); + s.load_objects_and_upstream_dependents("ep3_data"); auto random_crypt = make_shared(args.get("seed", 0, Arguments::IntFormat::HEX)); Episode3::Server::Options options = { @@ -1886,7 +1886,7 @@ Action a_run_server_replay_log( shared_ptr base(event_base_new(), event_base_free); auto state = make_shared(base, config_filename, is_replay); - state->load_objects("all"); + state->load_objects_and_downstream_dependents("all"); shared_ptr dns_server; if (state->dns_server_port && !is_replay) { diff --git a/src/ServerShell.cc b/src/ServerShell.cc index 75f4d147..0463120f 100644 --- a/src/ServerShell.cc +++ b/src/ServerShell.cc @@ -299,7 +299,7 @@ Proxy session commands:\n\ } } } - this->state->load_objects(types); + this->state->load_objects_and_downstream_dependents(types); } else if (command_name == "add-license") { auto l = this->state->license_index->create_license(); diff --git a/src/ServerState.cc b/src/ServerState.cc index bc1ef562..f04bdfaf 100644 --- a/src/ServerState.cc +++ b/src/ServerState.cc @@ -44,12 +44,20 @@ ServerState::ServerState(shared_ptr base, const string& confi this->create_load_step_graph(); } -void ServerState::load_objects(const std::string& what) { - this->load_step_graph.run(what); +void ServerState::load_objects_and_downstream_dependents(const std::string& what) { + this->load_step_graph.run(what, false); } -void ServerState::load_objects(const std::vector& what) { - this->load_step_graph.run(what); +void ServerState::load_objects_and_downstream_dependents(const std::vector& what) { + this->load_step_graph.run(what, false); +} + +void ServerState::load_objects_and_upstream_dependents(const std::string& what) { + this->load_step_graph.run(what, true); +} + +void ServerState::load_objects_and_upstream_dependents(const std::vector& what) { + this->load_step_graph.run(what, true); } void ServerState::add_client_to_available_lobby(shared_ptr c) { diff --git a/src/ServerState.hh b/src/ServerState.hh index c14b6715..4ba192fe 100644 --- a/src/ServerState.hh +++ b/src/ServerState.hh @@ -237,8 +237,11 @@ struct ServerState : public std::enable_shared_from_this { ServerState(ServerState&&) = delete; ServerState& operator=(const ServerState&) = delete; ServerState& operator=(ServerState&&) = delete; - void load_objects(const std::string& what); - void load_objects(const std::vector& what); + + void load_objects_and_downstream_dependents(const std::string& what); + void load_objects_and_downstream_dependents(const std::vector& what); + void load_objects_and_upstream_dependents(const std::string& what); + void load_objects_and_upstream_dependents(const std::vector& what); void add_client_to_available_lobby(std::shared_ptr c); void remove_client_from_lobby(std::shared_ptr c); diff --git a/src/StepGraph.cc b/src/StepGraph.cc index 275473b4..a78bfcc2 100644 --- a/src/StepGraph.cc +++ b/src/StepGraph.cc @@ -10,15 +10,16 @@ void StepGraph::add_step(const string& name, const vector& depends_on_na for (const auto& depends_on_name : depends_on_names) { auto upstream_step = this->steps.at(depends_on_name); upstream_step->downstream_dependencies.emplace_back(new_step); + new_step->upstream_dependencies.emplace_back(upstream_step); } } -void StepGraph::run(const string& start_step_name) { +void StepGraph::run(const string& start_step_name, bool run_upstreams) { vector start_step_names({start_step_name}); - this->run(start_step_names); + this->run(start_step_names, run_upstreams); } -void StepGraph::run(const vector& start_step_names) { +void StepGraph::run(const vector& start_step_names, bool run_upstreams) { // Collect all steps to run deque> steps_to_visit; try { @@ -33,8 +34,18 @@ void StepGraph::run(const vector& start_step_names) { auto step = std::move(steps_to_visit.front()); steps_to_visit.pop_front(); if (steps_to_run.emplace(step).second) { - for (const auto& downstream_step : step->downstream_dependencies) { - steps_to_visit.emplace_back(downstream_step); + if (run_upstreams) { + for (const auto& w_other_step : step->upstream_dependencies) { + auto other_step = w_other_step.lock(); + if (!other_step) { + throw runtime_error("upstream step is deleted"); + } + steps_to_visit.emplace_back(other_step); + } + } else { + for (const auto& other_step : step->downstream_dependencies) { + steps_to_visit.emplace_back(other_step); + } } } } diff --git a/src/StepGraph.hh b/src/StepGraph.hh index 6ae1daad..9bb00fba 100644 --- a/src/StepGraph.hh +++ b/src/StepGraph.hh @@ -12,6 +12,7 @@ struct StepGraph { struct Step { std::vector> downstream_dependencies; + std::vector> upstream_dependencies; std::function execute; uint64_t last_run_id = 0; }; @@ -22,6 +23,6 @@ struct StepGraph { StepGraph() = default; void add_step(const std::string& name, const std::vector& depends_on_names, std::function&& execute); - void run(const std::string& start_step); - void run(const std::vector& start_steps); + void run(const std::string& start_step, bool run_upstreams); + void run(const std::vector& start_steps, bool run_upstreams); };