From 0ea94b873b9b42b248095afeb2e8d88ec385dbfb Mon Sep 17 00:00:00 2001 From: slavek-kucera <53339291+slavek-kucera@users.noreply.github.com> Date: Fri, 17 Feb 2023 10:02:43 +0100 Subject: [PATCH] refactor: Remove the internal thread from the macro debugger --- language_server/src/blocking_queue.h | 42 +++- language_server/src/dap/dap_feature.cpp | 2 + language_server/src/dap/dap_feature.h | 2 + language_server/src/dap/dap_server.cpp | 7 +- language_server/src/dap/dap_server.h | 3 + language_server/src/dap/dap_session.cpp | 28 ++- language_server/src/json_queue_channel.h | 1 + language_server/test/dap/dap_feature_test.cpp | 65 +++--- parser_library/include/debugger.h | 2 + parser_library/src/analyzer.cpp | 9 + parser_library/src/analyzer.h | 6 + .../src/debugging/debug_lib_provider.cpp | 39 ++-- .../src/debugging/debug_lib_provider.h | 14 +- parser_library/src/debugging/debugger.cpp | 198 ++++++++---------- .../src/processing/processing_manager.cpp | 28 +++ .../src/processing/processing_manager.h | 6 + .../statement_analyzers/lsp_analyzer.cpp | 4 +- .../statement_analyzers/lsp_analyzer.h | 2 +- .../statement_analyzers/statement_analyzer.h | 2 +- .../debugging/debug_event_consumer_s_mock.cpp | 6 +- .../debugging/debug_event_consumer_s_mock.h | 16 +- .../debugging/debug_lib_provider_test.cpp | 26 ++- .../test/debugging/debugger_test.cpp | 38 ++-- .../processing/occurence_collector_test.cpp | 7 +- utils/include/utils/CMakeLists.txt | 1 + utils/include/utils/task.h | 149 +++++++++++++ utils/test/CMakeLists.txt | 1 + utils/test/task_test.cpp | 151 +++++++++++++ 28 files changed, 620 insertions(+), 235 deletions(-) create mode 100644 utils/include/utils/task.h create mode 100644 utils/test/task_test.cpp diff --git a/language_server/src/blocking_queue.h b/language_server/src/blocking_queue.h index 1a5e0a961..045a11420 100644 --- a/language_server/src/blocking_queue.h +++ b/language_server/src/blocking_queue.h @@ -30,42 +30,55 @@ enum class blocking_queue_termination_policy : bool }; template + blocking_queue_termination_policy termination_policy = blocking_queue_termination_policy::drop_elements, + bool one_reader = true> class blocking_queue { + static constexpr unsigned char terminated_flag = 0x01; + static constexpr unsigned char has_elements_flag = 0x02; + std::mutex mutex; std::condition_variable cond_var; std::deque queue; - bool terminated = false; + std::atomic state = 0; public: - void push(T&& t) + bool push(T&& t) { std::unique_lock g(mutex); - if (terminated) - return; + + if (terminated()) + return false; const bool notify = queue.size() == 0; queue.push_back(std::move(t)); + state.fetch_or(has_elements_flag, std::memory_order_relaxed); g.unlock(); if (notify) cond_var.notify_one(); + + return true; } - void push(const T& t) + + bool push(const T& t) { std::unique_lock g(mutex); - if (terminated) - return; + + if (terminated()) + return false; const bool notify = queue.size() == 0; queue.push_back(t); + state.fetch_or(has_elements_flag, std::memory_order_relaxed); g.unlock(); if (notify) cond_var.notify_one(); + + return true; } std::optional pop() @@ -74,13 +87,15 @@ class blocking_queue constexpr const auto process = blocking_queue_termination_policy::process_elements; std::unique_lock g(mutex); - cond_var.wait(g, [this] { return queue.size() || terminated; }); + cond_var.wait(g, [this] { return queue.size() || terminated(); }); - if ((termination_policy == drop && terminated) || (termination_policy == process && queue.size() == 0)) + if ((termination_policy == drop && terminated()) || (termination_policy == process && queue.size() == 0)) return std::nullopt; std::optional result = std::move(queue.front()); queue.pop_front(); + if (queue.empty()) + state.fetch_and(static_cast(~has_elements_flag), std::memory_order_relaxed); return result; } @@ -88,11 +103,16 @@ class blocking_queue void terminate() { std::unique_lock g(mutex); - terminated = true; + state.fetch_or(terminated_flag, std::memory_order_relaxed); g.unlock(); cond_var.notify_one(); } + + bool terminated() const { return state.load(std::memory_order_relaxed) & terminated_flag; } + bool empty() const { return !(state.load(std::memory_order_relaxed) & has_elements_flag); } + + bool will_block() const requires one_reader { return state.load(std::memory_order_relaxed) == 0; } }; } // namespace hlasm_plugin::language_server diff --git a/language_server/src/dap/dap_feature.cpp b/language_server/src/dap/dap_feature.cpp index 2fafdbb9d..2331d3705 100644 --- a/language_server/src/dap/dap_feature.cpp +++ b/language_server/src/dap/dap_feature.cpp @@ -356,4 +356,6 @@ void dap_feature::on_pause(const nlohmann::json& request_seq, const nlohmann::js response_->respond(request_seq, "pause", nlohmann::json()); } +bool dap_feature::idle_handler() { return debugger && debugger->analysis_step(); } + } // namespace hlasm_plugin::language_server::dap diff --git a/language_server/src/dap/dap_feature.h b/language_server/src/dap/dap_feature.h index a70056a21..fe056ecdf 100644 --- a/language_server/src/dap/dap_feature.h +++ b/language_server/src/dap/dap_feature.h @@ -65,6 +65,8 @@ class dap_feature : public feature, public hlasm_plugin::parser_library::debuggi void on_continue(const nlohmann::json& request_seq, const nlohmann::json& args); void on_pause(const nlohmann::json& request_seq, const nlohmann::json& args); + bool idle_handler(); + private: // Inherited via feature void register_methods(std::map& methods) override; diff --git a/language_server/src/dap/dap_server.cpp b/language_server/src/dap/dap_server.cpp index 223d15915..b8f0017bf 100644 --- a/language_server/src/dap/dap_server.cpp +++ b/language_server/src/dap/dap_server.cpp @@ -24,7 +24,9 @@ namespace hlasm_plugin::language_server::dap { server::server(parser_library::workspace_manager& ws_mngr, telemetry_sink* telemetry_reporter) : language_server::server(ws_mngr, telemetry_reporter) { - features_.push_back(std::make_unique(ws_mngr_, *this, this)); + auto dap_f = std::make_unique(ws_mngr_, *this, this); + m_dap_feature = dap_f.get(); + features_.push_back(std::move(dap_f)); register_feature_methods(); } @@ -105,4 +107,7 @@ void server::disconnected() exit_notification_received_ = true; } + +bool server::idle_handler() { return m_dap_feature->idle_handler(); } + } // namespace hlasm_plugin::language_server::dap diff --git a/language_server/src/dap/dap_server.h b/language_server/src/dap/dap_server.h index 3481cb0db..c9434999c 100644 --- a/language_server/src/dap/dap_server.h +++ b/language_server/src/dap/dap_server.h @@ -46,8 +46,11 @@ class server final : public hlasm_plugin::language_server::server, public dap_di void message_received(const nlohmann::json& message) override; + bool idle_handler(); + private: std::atomic last_seq_ = 0; + dap_feature* m_dap_feature = nullptr; void register_methods(); diff --git a/language_server/src/dap/dap_session.cpp b/language_server/src/dap/dap_session.cpp index 1f1667e8a..16a3b8f6c 100644 --- a/language_server/src/dap/dap_session.cpp +++ b/language_server/src/dap/dap_session.cpp @@ -26,13 +26,31 @@ void session::thread_routine() { try { - std::atomic cancel = false; + json_channel_adapter channel(msg_unwrapper, msg_wrapper); + struct smp_t final : send_message_provider + { + json_channel_adapter& channel; + void reply(const nlohmann::json& result) override { channel.write(result); } + explicit smp_t(json_channel_adapter& channel) + : channel(channel) + {} + } smp(channel); + scope_exit indicate_end([this]() { running = false; }); - request_manager req_mgr(&cancel); - scope_exit end_request_manager([&req_mgr]() { req_mgr.end_worker(); }); + dap::server server(*ws_mngr, telemetry_reporter); - dispatcher dispatcher(json_channel_adapter(msg_unwrapper, msg_wrapper), server, req_mgr); - dispatcher.run_server_loop(); + server.set_send_message_provider(&smp); + + while (!server.is_exit_notification_received()) + { + if (queue.will_read_block() && server.idle_handler()) + continue; + + auto msg = channel.read(); + if (!msg.has_value()) + break; + server.message_received(msg.value()); + } } catch (const std::exception& ex) { diff --git a/language_server/src/json_queue_channel.h b/language_server/src/json_queue_channel.h index d9d0315bc..43f5b4eae 100644 --- a/language_server/src/json_queue_channel.h +++ b/language_server/src/json_queue_channel.h @@ -32,6 +32,7 @@ class json_queue_channel final : public json_channel void write(nlohmann::json&&) override; void terminate(); + bool will_read_block() const { return queue.will_block(); } }; } // namespace hlasm_plugin::language_server diff --git a/language_server/test/dap/dap_feature_test.cpp b/language_server/test/dap/dap_feature_test.cpp index 4678aacb8..969b1a738 100644 --- a/language_server/test/dap/dap_feature_test.cpp +++ b/language_server/test/dap/dap_feature_test.cpp @@ -76,36 +76,8 @@ struct response_provider_mock : public response_provider std::vector responses; std::vector notifs; - std::atomic stopped = false; - std::atomic exited = false; - - void wait_for_stopped() - { - size_t i = 0; - while (!stopped) - { - if (i > 50) - throw std::runtime_error("Wait for stopped timeout."); - ++i; - - std::this_thread::sleep_for(100ms); - } - stopped = false; - } - - void wait_for_exited() - { - size_t i = 0; - while (!exited) - { - if (i > 50) - throw std::runtime_error("Wait for exited timeout."); - ++i; - - std::this_thread::sleep_for(100ms); - } - exited = false; - } + bool stopped = false; + bool exited = false; void reset() @@ -150,8 +122,27 @@ struct feature_launch_test : public testing::Test void wait_for_stopped() { - resp_provider.wait_for_stopped(); + for (int i = 0; !resp_provider.stopped; ++i) + { + if (i >= 1000000) + throw std::runtime_error("Wait for stopped timeout."); + + feature.idle_handler(); + } EXPECT_EQ(resp_provider.notifs.size(), 1U); + resp_provider.stopped = false; + } + + void wait_for_exited() + { + for (int i = 0; !resp_provider.exited; ++i) + { + if (i >= 1000000) + throw std::runtime_error("Wait for exited timeout."); + + feature.idle_handler(); + } + resp_provider.exited = false; } std::map methods; @@ -271,7 +262,7 @@ TEST_F(feature_launch_test, step) ASSERT_EQ(r2.args["stackFrames"].size(), 1U); feature.on_continue("47"_json, nlohmann::json()); - resp_provider.wait_for_exited(); + wait_for_exited(); feature.on_disconnect("48"_json, {}); } @@ -337,11 +328,13 @@ TEST_F(feature_launch_test, variables) check_simple_stack_trace("2"_json, 0); feature.on_step_in("3"_json, nlohmann::json()); - resp_provider.wait_for_stopped(); + wait_for_stopped(); + resp_provider.reset(); feature.on_step_in("3"_json, nlohmann::json()); - resp_provider.wait_for_stopped(); + wait_for_stopped(); + resp_provider.reset(); feature.on_step_in("3"_json, nlohmann::json()); - resp_provider.wait_for_stopped(); + wait_for_stopped(); resp_provider.reset(); feature.on_stack_trace("1"_json, nlohmann::json()); @@ -429,7 +422,7 @@ TEST_F(feature_launch_test, pause) feature.on_pause("1"_json, {}); - resp_provider.wait_for_stopped(); + wait_for_stopped(); std::vector expected_resp = { { "0"_json, "launch", nlohmann::json() }, { "1"_json, "pause", nlohmann::json() } }; EXPECT_EQ(resp_provider.responses, expected_resp); diff --git a/parser_library/include/debugger.h b/parser_library/include/debugger.h index 267ef23a8..bc1e6bd43 100644 --- a/parser_library/include/debugger.h +++ b/parser_library/include/debugger.h @@ -95,6 +95,8 @@ class debugger stack_frames_t stack_frames() const; scopes_t scopes(frame_id_t frame_id) const; variables_t variables(var_reference_t var_ref) const; + + bool analysis_step(); }; } // namespace hlasm_plugin::parser_library::debugging diff --git a/parser_library/src/analyzer.cpp b/parser_library/src/analyzer.cpp index e5b919f9c..984f7d190 100644 --- a/parser_library/src/analyzer.cpp +++ b/parser_library/src/analyzer.cpp @@ -18,6 +18,7 @@ #include "lsp/lsp_context.h" #include "processing/opencode_provider.h" #include "processing/preprocessor.h" +#include "utils/task.h" using namespace hlasm_plugin::parser_library; using namespace hlasm_plugin::parser_library::lexing; @@ -148,6 +149,14 @@ void analyzer::analyze() bool analyzer::analyze_step() { return mngr_.step() || (src_proc_.finish(), false); } + +hlasm_plugin::utils::task analyzer::co_analyze() & +{ + co_await mngr_.co_step(); + + src_proc_.finish(); +} + void analyzer::collect_diags() const { collect_diags_from_child(mngr_); diff --git a/parser_library/src/analyzer.h b/parser_library/src/analyzer.h index 215a74092..7f93f123f 100644 --- a/parser_library/src/analyzer.h +++ b/parser_library/src/analyzer.h @@ -39,6 +39,11 @@ #include "virtual_file_monitor.h" #include "workspaces/parse_lib_provider.h" + +namespace hlasm_plugin::utils { +struct task; +} // namespace hlasm_plugin::utils + namespace hlasm_plugin::parser_library::parsing { class hlasmparser_multiline; } // namespace hlasm_plugin::parser_library::parsing @@ -154,6 +159,7 @@ class analyzer : public diagnosable_ctx void analyze(); bool analyze_step(); + utils::task co_analyze() &; void collect_diags() const override; const performance_metrics& get_metrics() const; diff --git a/parser_library/src/debugging/debug_lib_provider.cpp b/parser_library/src/debugging/debug_lib_provider.cpp index 42a0597f1..fb35f86b2 100644 --- a/parser_library/src/debugging/debug_lib_provider.cpp +++ b/parser_library/src/debugging/debug_lib_provider.cpp @@ -15,6 +15,7 @@ #include "debug_lib_provider.h" #include "analyzer.h" +#include "utils/task.h" #include "workspaces/file_manager.h" #include "workspaces/library.h" #include "workspaces/workspace.h" @@ -23,10 +24,10 @@ namespace hlasm_plugin::parser_library::debugging { debug_lib_provider::debug_lib_provider(std::vector> libraries, workspaces::file_manager& fm, - std::atomic* cancel) + std::vector& analyzers) : m_libraries(std::move(libraries)) , m_file_manager(fm) - , m_cancel(cancel) + , m_analyzers(analyzers) {} void debug_lib_provider::parse_library( @@ -43,25 +44,21 @@ void debug_lib_provider::parse_library( break; const auto& [location, content] = *m_files.try_emplace(std::move(url), std::move(content_o).value()).first; - analyzer a(content, - analyzer_options { - location, - this, - std::move(ctx), - data, - collect_highlighting_info::no, - }); - - do - { - if (m_cancel && m_cancel->load(std::memory_order_relaxed)) - { - callback(false); - return; - } - } while (a.analyze_step()); - - callback(true); + + constexpr auto dep_task = + [](std::string content, analyzer_options opts, std::function callback) -> utils::task { + analyzer a(content, std::move(opts)); + + co_await a.co_analyze(); + + if (callback) + callback(true); + }; + + m_analyzers.emplace_back(dep_task(content, + analyzer_options(location, this, std::move(ctx), data, collect_highlighting_info::no), + std::move(callback))); + return; } callback(false); diff --git a/parser_library/src/debugging/debug_lib_provider.h b/parser_library/src/debugging/debug_lib_provider.h index 975edee99..3717b4d03 100644 --- a/parser_library/src/debugging/debug_lib_provider.h +++ b/parser_library/src/debugging/debug_lib_provider.h @@ -15,7 +15,7 @@ #ifndef HLASMPLUGIN_PARSERLIBRARY_DEBUG_LIB_PROVIDER_H #define HLASMPLUGIN_PARSERLIBRARY_DEBUG_LIB_PROVIDER_H -#include +#include #include #include #include @@ -27,6 +27,14 @@ #include "utils/resource_location.h" #include "workspaces/parse_lib_provider.h" +namespace hlasm_plugin::utils { +struct task; +} // namespace hlasm_plugin::utils + +namespace hlasm_plugin::parser_library { +class analyzer; +} // namespace hlasm_plugin::parser_library + namespace hlasm_plugin::parser_library::workspaces { class file_manager; class library; @@ -43,12 +51,12 @@ class debug_lib_provider final : public workspaces::parse_lib_provider m_files; std::vector> m_libraries; workspaces::file_manager& m_file_manager; - std::atomic* m_cancel; + std::vector& m_analyzers; public: debug_lib_provider(std::vector> libraries, workspaces::file_manager& fm, - std::atomic* cancel); + std::vector& analyzers); void parse_library(std::string_view library, analyzing_context ctx, diff --git a/parser_library/src/debugging/debugger.cpp b/parser_library/src/debugging/debugger.cpp index 23edb83b8..706542e32 100644 --- a/parser_library/src/debugging/debugger.cpp +++ b/parser_library/src/debugging/debugger.cpp @@ -32,6 +32,7 @@ #include "macro_param_variable.h" #include "ordinary_symbol_variable.h" #include "set_symbol_variable.h" +#include "utils/task.h" #include "variable.h" #include "virtual_file_monitor.h" #include "workspaces/file_manager.h" @@ -76,32 +77,19 @@ std::size_t breakpoints_t::size() const { return pimpl->m_breakpoints.size(); } // interface. class debugger::impl final : public processing::statement_analyzer { - mutable std::mutex control_mtx; - mutable std::mutex variable_mtx_; - mutable std::mutex breakpoints_mutex_; - - std::thread thread_; - - // these are used in conditional variable to stop execution - // of analyzer and wait for user input - std::condition_variable con_var; - std::atomic continue_ = false; - - std::atomic debug_ended_ = false; + bool debug_ended_ = false; + bool continue_ = false; // Specifies whether the debugger stops on the next statement call. - std::atomic stop_on_next_stmt_ = false; - std::atomic stop_on_stack_changes_ = false; + bool stop_on_next_stmt_ = false; + bool stop_on_stack_changes_ = false; std::pair stop_on_stack_condition_; // Range of statement that is about to be processed by analyzer. range next_stmt_range_; // True, if disconnect request was received - std::atomic disconnected_ = false; - - // Cancellation token for parsing - std::atomic cancel_ = false; + bool disconnected_ = false; // Provides a way to inform outer world about debugger events debug_event_consumer* event_ = nullptr; @@ -127,6 +115,8 @@ class debugger::impl final : public processing::statement_analyzer return next_var_ref_++; } + std::vector analyzers; + public: impl() = default; @@ -141,85 +131,97 @@ class debugger::impl final : public processing::statement_analyzer return false; } opencode_source_uri_ = open_code_location.get_uri(); + continue_ = true; stop_on_next_stmt_ = stop_on_entry; + stop_on_stack_changes_ = false; + + const auto main_analyzer = [](std::string open_code_text, + debug_lib_provider debug_provider, + workspaces::file_manager_vfm vfm, + asm_option asm_opts, + std::vector pp_opts, + utils::resource::resource_location open_code_location, + impl* self) -> utils::task { + analyzer a(open_code_text, + analyzer_options { + std::move(open_code_location), + &debug_provider, + std::move(asm_opts), + std::move(pp_opts), + &vfm, + }); - struct debugger_thread_data - { - utils::resource::resource_location open_code_location; - std::string open_code_text; - debug_lib_provider debug_provider; - asm_option asm_opts; - std::vector pp_opts; - workspaces::file_manager_vfm vfm; - analyzer_options opts; + a.register_stmt_analyzer(self); + + self->ctx_ = a.context().hlasm_ctx.get(); + + co_await a.co_analyze(); }; - auto data = std::make_unique(debugger_thread_data { - open_code_location, - std::move(open_code_text).value(), - debug_lib_provider(workspace.get_libraries(open_code_location), workspace.get_file_manager(), &cancel_), + auto& fm = workspace.get_file_manager(); + analyzers.clear(); + analyzers.emplace_back(main_analyzer(std::move(open_code_text).value(), + debug_lib_provider(workspace.get_libraries(open_code_location), fm, analyzers), + workspaces::file_manager_vfm(fm, utils::resource::resource_location(workspace.uri())), workspace.get_asm_options(open_code_location), workspace.get_preprocessor_options(open_code_location), - workspaces::file_manager_vfm( - workspace.get_file_manager(), utils::resource::resource_location(workspace.uri())), - }); + open_code_location, + this)); - thread_ = std::thread([this, data = std::move(data)]() { - std::lock_guard guard(variable_mtx_); // Lock the mutex while analyzer is running, unlock once - // it is stopped and waiting in the statement method + return true; + } - analyzer a(data->open_code_text, - analyzer_options { - std::move(data->open_code_location), - &data->debug_provider, - std::move(data->asm_opts), - std::move(data->pp_opts), - &data->vfm, - }); + bool step() + { + if (analyzers.empty() || debug_ended_) + return false; - a.register_stmt_analyzer(this); + if (!continue_) + return false; - ctx_ = a.context().hlasm_ctx.get(); + if (const auto& a = analyzers.back(); !a.done()) + { + a(); + return true; + } - do - { - if (cancel_.load(std::memory_order_relaxed)) - break; - } while (a.analyze_step()); + analyzers.pop_back(); - if (!disconnected_ && event_) - event_->exited(0); - debug_ended_ = true; - }); + if (!analyzers.empty()) + return true; - return true; + if (!disconnected_ && event_) + event_->exited(0); + debug_ended_ = true; + + return false; } void set_event_consumer(debug_event_consumer* event) { event_ = event; } - void analyze(const context::hlasm_statement& statement, + bool analyze(const context::hlasm_statement& statement, processing::statement_provider_kind, processing::processing_kind proc_kind, bool evaluated_model) override { if (disconnected_) - return; + return false; if (evaluated_model) - return; // we already stopped on the model itself + return false; // we already stopped on the model itself // Continue only for ordinary processing kind (i.e. when the statement is executed, skip // lookahead and copy/macro definitions) if (proc_kind != processing::processing_kind::ORDINARY) - return; + return false; const auto* resolved_stmt = statement.access_resolved(); if (!resolved_stmt) - return; + return false; // Continue only for non-empty statements if (resolved_stmt->opcode_ref().value.empty()) - return; + return false; range stmt_range = resolved_stmt->stmt_range_ref(); @@ -249,11 +251,9 @@ class debugger::impl final : public processing::statement_analyzer stack_frames_.clear(); scopes_.clear(); proc_stack_ = std::move(stack); - variable_mtx_.unlock(); - std::unique_lock lck(control_mtx); if (disconnected_) - return; + return false; stop_on_next_stmt_ = false; stop_on_stack_changes_ = false; next_stmt_range_ = stmt_range; @@ -262,73 +262,48 @@ class debugger::impl final : public processing::statement_analyzer continue_ = false; if (event_) event_->stopped("entry", ""); - - con_var.wait(lck, [&] { return !!continue_; }); - - variable_mtx_.lock(); } + return !continue_; } // User controls of debugging. void next() { - { - std::lock_guard lck(control_mtx); - stop_on_stack_changes_ = true; - continue_ = true; - } - con_var.notify_all(); + stop_on_stack_changes_ = true; + continue_ = true; } void step_in() { - { - std::lock_guard lck(control_mtx); - stop_on_next_stmt_ = true; - continue_ = true; - } - con_var.notify_all(); + stop_on_next_stmt_ = true; + continue_ = true; } void step_out() { + if (!stop_on_stack_condition_.first.empty()) { - std::lock_guard lck(control_mtx); - if (!stop_on_stack_condition_.first.empty()) - { - stop_on_stack_changes_ = true; - stop_on_stack_condition_ = std::make_pair( - stop_on_stack_condition_.first.parent(), stop_on_stack_condition_.first.frame().resource_loc); - } - else - stop_on_next_stmt_ = false; // step out in the opencode is equivalent to continue - continue_ = true; + stop_on_stack_changes_ = true; + stop_on_stack_condition_ = std::make_pair( + stop_on_stack_condition_.first.parent(), stop_on_stack_condition_.first.frame().resource_loc); } - con_var.notify_all(); + else + stop_on_next_stmt_ = false; // step out in the opencode is equivalent to continue + continue_ = true; } void disconnect() { // set cancellation token and wake up the thread, // so it peacefully finishes, we then wait for it and join - { - std::lock_guard lck(control_mtx); - - disconnected_ = true; - continue_ = true; - cancel_ = true; - } - con_var.notify_all(); - - if (thread_.joinable()) - thread_.join(); + disconnected_ = true; + continue_ = true; } void continue_debug() { stop_on_next_stmt_ = false; continue_ = true; - con_var.notify_all(); } void pause() { stop_on_next_stmt_ = true; } @@ -336,7 +311,6 @@ class debugger::impl final : public processing::statement_analyzer // Retrieval of current context. const std::vector& stack_frames() { - std::lock_guard guard(variable_mtx_); stack_frames_.clear(); if (debug_ended_) return stack_frames_; @@ -370,8 +344,6 @@ class debugger::impl final : public processing::statement_analyzer const std::vector& scopes(frame_id_t frame_id) { - std::lock_guard guard(variable_mtx_); - scopes_.clear(); if (debug_ended_) return scopes_; @@ -436,7 +408,6 @@ class debugger::impl final : public processing::statement_analyzer { static const hlasm_plugin::parser_library::debugging::variable_store empty_variables; - std::lock_guard guard(variable_mtx_); if (debug_ended_) return empty_variables; auto it = variables_.find(var_ref); @@ -455,23 +426,17 @@ class debugger::impl final : public processing::statement_analyzer void breakpoints(const utils::resource::resource_location& source, std::vector bps) { - std::lock_guard g(breakpoints_mutex_); breakpoints_[source] = std::move(bps); } [[nodiscard]] std::vector breakpoints(const utils::resource::resource_location& source) const { - std::lock_guard g(breakpoints_mutex_); if (auto it = breakpoints_.find(source); it != breakpoints_.end()) return it->second; return {}; } - ~impl() - { - if (thread_.joinable()) - disconnect(); - } + ~impl() { disconnect(); } }; debugger::debugger() @@ -505,6 +470,7 @@ void debugger::step_out() { pimpl->step_out(); } void debugger::disconnect() { pimpl->disconnect(); } void debugger::continue_debug() { pimpl->continue_debug(); } void debugger::pause() { pimpl->pause(); } +bool debugger::analysis_step() { return pimpl->step(); } void debugger::breakpoints(sequence source, sequence bps) diff --git a/parser_library/src/processing/processing_manager.cpp b/parser_library/src/processing/processing_manager.cpp index 6d52a204b..f4a61d955 100644 --- a/parser_library/src/processing/processing_manager.cpp +++ b/parser_library/src/processing/processing_manager.cpp @@ -28,6 +28,7 @@ #include "statement_processors/ordinary_processor.h" #include "statement_providers/copy_statement_provider.h" #include "statement_providers/macro_statement_provider.h" +#include "utils/task.h" namespace hlasm_plugin::parser_library::processing { @@ -124,6 +125,33 @@ bool processing_manager::step() return true; } +utils::task processing_manager::co_step() +{ + while (!procs_.empty()) + { + statement_processor& proc = *procs_.back(); + statement_provider& prov = find_provider(); + + if ((prov.finished() && proc.terminal_condition(prov.kind)) || proc.finished()) + { + finish_processor(); + continue; + } + + if (auto stmt = prov.get_next(proc)) + { + update_metrics(proc.kind, prov.kind, hlasm_ctx_.metrics); + for (auto& a : stms_analyzers_) + if (a->analyze(*stmt, prov.kind, proc.kind, false)) + co_await utils::task::suspend(); + + proc.process_statement(std::move(stmt)); + } + + co_await utils::task::suspend(); + } +} + void processing_manager::register_stmt_analyzer(statement_analyzer* stmt_analyzer) { stms_analyzers_.push_back(stmt_analyzer); diff --git a/parser_library/src/processing/processing_manager.h b/parser_library/src/processing/processing_manager.h index 7f4296594..cf1ff1308 100644 --- a/parser_library/src/processing/processing_manager.h +++ b/parser_library/src/processing/processing_manager.h @@ -31,6 +31,10 @@ #include "statement_fields_parser.h" #include "workspaces/parse_lib_provider.h" +namespace hlasm_plugin::utils { +struct task; +} // namespace hlasm_plugin::utils + namespace hlasm_plugin::parser_library::processing { // main class for processing of the opencode @@ -52,6 +56,8 @@ class processing_manager final : public processing_state_listener, public branch // method that starts the processing loop bool step(); + utils::task co_step(); + void register_stmt_analyzer(statement_analyzer* stmt_analyzer); void run_anayzers(const context::hlasm_statement& statement, bool evaluated_model) const; diff --git a/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp b/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp index 2f93508b1..b821a7e7f 100644 --- a/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp +++ b/parser_library/src/processing/statement_analyzers/lsp_analyzer.cpp @@ -69,7 +69,7 @@ lsp_analyzer::lsp_analyzer(context::hlasm_context& hlasm_ctx, lsp::lsp_context& , file_text_(file_text) {} -void lsp_analyzer::analyze(const context::hlasm_statement& statement, +bool lsp_analyzer::analyze(const context::hlasm_statement& statement, statement_provider_kind prov_kind, processing_kind proc_kind, bool evaluated_model) @@ -115,6 +115,8 @@ void lsp_analyzer::analyze(const context::hlasm_statement& statement, } assign_statement_occurences(hlasm_ctx_.current_statement_location().resource_loc); + + return false; } namespace { diff --git a/parser_library/src/processing/statement_analyzers/lsp_analyzer.h b/parser_library/src/processing/statement_analyzers/lsp_analyzer.h index 8a3e22e08..811e7dbd6 100644 --- a/parser_library/src/processing/statement_analyzers/lsp_analyzer.h +++ b/parser_library/src/processing/statement_analyzers/lsp_analyzer.h @@ -87,7 +87,7 @@ class lsp_analyzer : public statement_analyzer public: lsp_analyzer(context::hlasm_context& hlasm_ctx, lsp::lsp_context& lsp_ctx, std::string_view file_text); - void analyze(const context::hlasm_statement& statement, + bool analyze(const context::hlasm_statement& statement, statement_provider_kind prov_kind, processing_kind proc_kind, bool evaluated_model) override; diff --git a/parser_library/src/processing/statement_analyzers/statement_analyzer.h b/parser_library/src/processing/statement_analyzers/statement_analyzer.h index 25fcba8e1..e0777a62d 100644 --- a/parser_library/src/processing/statement_analyzers/statement_analyzer.h +++ b/parser_library/src/processing/statement_analyzers/statement_analyzer.h @@ -30,7 +30,7 @@ using analyzer_ptr = std::unique_ptr; class statement_analyzer { public: - virtual void analyze(const context::hlasm_statement& statement, + virtual bool analyze(const context::hlasm_statement& statement, statement_provider_kind prov_kind, processing_kind proc_kind, bool evaluated_model) = 0; diff --git a/parser_library/test/debugging/debug_event_consumer_s_mock.cpp b/parser_library/test/debugging/debug_event_consumer_s_mock.cpp index 9b0b318a4..1c862a88f 100644 --- a/parser_library/test/debugging/debug_event_consumer_s_mock.cpp +++ b/parser_library/test/debugging/debug_event_consumer_s_mock.cpp @@ -26,22 +26,20 @@ void debug_event_consumer_s_mock::stopped( (void)reason; (void)addtl_info; ++stop_count; - while (stopped_) - std::this_thread::sleep_for(100ms); stopped_ = true; } void debug_event_consumer_s_mock::wait_for_stopped() { while (!stopped_) - std::this_thread::sleep_for(100ms); + d.analysis_step(); stopped_ = false; } void debug_event_consumer_s_mock::wait_for_exited() { while (!exited_) - std::this_thread::sleep_for(100ms); + d.analysis_step(); stopped_ = false; } diff --git a/parser_library/test/debugging/debug_event_consumer_s_mock.h b/parser_library/test/debugging/debug_event_consumer_s_mock.h index 117e28add..cf4ba87d4 100644 --- a/parser_library/test/debugging/debug_event_consumer_s_mock.h +++ b/parser_library/test/debugging/debug_event_consumer_s_mock.h @@ -15,17 +15,23 @@ #ifndef HLASMPLUGIN_PARSERLIBRARY_TEST_DEBUG_EVENT_CONSUMER_S_MOCK_H #define HLASMPLUGIN_PARSERLIBRARY_TEST_DEBUG_EVENT_CONSUMER_S_MOCK_H -#include - #include "debugger.h" class debug_event_consumer_s_mock : public hlasm_plugin::parser_library::debugging::debug_event_consumer { - std::atomic stopped_ = false; - std::atomic exited_ = false; - std::atomic stop_count = 0; + bool stopped_ = false; + bool exited_ = false; + size_t stop_count = 0; + + hlasm_plugin::parser_library::debugging::debugger& d; public: + debug_event_consumer_s_mock(hlasm_plugin::parser_library::debugging::debugger& d) + : d(d) + { + d.set_event_consumer(this); + } + void stopped(hlasm_plugin::parser_library::sequence reason, hlasm_plugin::parser_library::sequence addtl_info) override; diff --git a/parser_library/test/debugging/debug_lib_provider_test.cpp b/parser_library/test/debugging/debug_lib_provider_test.cpp index 9931d2446..891cf8250 100644 --- a/parser_library/test/debugging/debug_lib_provider_test.cpp +++ b/parser_library/test/debugging/debug_lib_provider_test.cpp @@ -23,6 +23,7 @@ #include "analyzer.h" #include "debugging/debug_lib_provider.h" #include "utils/resource_location.h" +#include "utils/task.h" using namespace ::testing; using namespace hlasm_plugin::parser_library; @@ -37,7 +38,28 @@ class debug_lib_provider_test : public Test protected: std::shared_ptr> mock_lib = std::make_shared>(); NiceMock fm_mock; - debug_lib_provider lib = debug_lib_provider({ mock_lib }, fm_mock, nullptr); + std::vector nested_analyzers; + debug_lib_provider lib = debug_lib_provider({ mock_lib }, fm_mock, nested_analyzers); + + void analyze(analyzer& a) + { + while (true) + { + if (!nested_analyzers.empty()) + { + auto& na = nested_analyzers.back(); + if (na.done()) + { + nested_analyzers.pop_back(); + continue; + } + na(); + continue; + } + if (!a.analyze_step()) + break; + } + } }; } // namespace @@ -51,7 +73,7 @@ TEST_F(debug_lib_provider_test, parse_library) std::string input = " COPY AAA"; analyzer a(input, analyzer_options(&lib)); - a.analyze(); + analyze(a); a.collect_diags(); EXPECT_TRUE(matches_message_codes(a.diags(), { "MNOTE" })); diff --git a/parser_library/test/debugging/debugger_test.cpp b/parser_library/test/debugging/debugger_test.cpp index 26f387593..76d56f257 100644 --- a/parser_library/test/debugging/debugger_test.cpp +++ b/parser_library/test/debugging/debugger_test.cpp @@ -85,9 +85,8 @@ TEST(debugger, stopped_on_entry) shared_json global_settings = make_empty_shared_json(); workspace ws(file_manager, config, global_settings); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string file_name = "test_workspace\\test"; resource_location file_loc(file_name); @@ -123,9 +122,8 @@ TEST(debugger, disconnect) shared_json global_settings = make_empty_shared_json(); workspace ws(file_manager, config, global_settings); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string file_name = "test_workspace\\test"; resource_location file_loc(file_name); @@ -467,9 +465,8 @@ TEST(debugger, test) file_manager.did_open_file(copy2_file_loc, 0, copy2_source); workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -559,9 +556,8 @@ TEST(debugger, sysstmt) file_manager_impl file_manager; workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -621,9 +617,8 @@ A MAC_IN () file_manager.did_open_file(copy1_file_loc, 0, copy1_source); workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -787,9 +782,8 @@ TEST(debugger, positional_parameters) file_manager_impl file_manager; workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -908,9 +902,8 @@ TEST(debugger, arrays) file_manager_impl file_manager; workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -964,9 +957,8 @@ B EQU A file_manager_impl file_manager; workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -1011,9 +1003,8 @@ TEST(debugger, ainsert) file_manager_impl file_manager; workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -1076,9 +1067,8 @@ TEST(debugger, concurrent_next_and_file_change) file_manager.did_open_file(copy1_file_loc, 0, copy1_source); workspace_mock lib_provider(file_manager); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string filename = "ws\\test"; resource_location file_loc(filename); @@ -1089,16 +1079,13 @@ TEST(debugger, concurrent_next_and_file_change) std::vector chs; chs.emplace_back(new_string.c_str(), new_string.size()); d.next(); - std::this_thread::sleep_for(std::chrono::milliseconds(100)); std::thread t([&file_manager, ©1_file_loc, &chs]() { file_manager.did_change_file(copy1_file_loc, 0, chs.data(), chs.size()); }); + t.join(); m.wait_for_stopped(); - d.disconnect(); - - t.join(); } TEST(debugger, pimpl_moves) @@ -1129,9 +1116,8 @@ TEST(debugger, invalid_file) shared_json global_settings = make_empty_shared_json(); workspace ws(file_manager, config, global_settings); - debug_event_consumer_s_mock m; debugger d; - d.set_event_consumer(&m); + debug_event_consumer_s_mock m(d); std::string file_name = "test_workspace\\test"; resource_location file_loc(file_name); diff --git a/parser_library/test/processing/occurence_collector_test.cpp b/parser_library/test/processing/occurence_collector_test.cpp index 65874344e..c758006be 100644 --- a/parser_library/test/processing/occurence_collector_test.cpp +++ b/parser_library/test/processing/occurence_collector_test.cpp @@ -35,16 +35,19 @@ struct operand_occurence_analyzer_mock : public processing::statement_analyzer a.analyze(); } - void analyze(const context::hlasm_statement& statement, + bool analyze(const context::hlasm_statement& statement, processing::statement_provider_kind, processing::processing_kind, bool evaluated_model) override { const auto* res_stmt = statement.access_resolved(); - ASSERT_TRUE(res_stmt); + EXPECT_TRUE(res_stmt); + assert(res_stmt); processing::occurence_collector collector(occ_kind, *a.context().hlasm_ctx, st, evaluated_model); const auto& operands = res_stmt->operands_ref().value; std::for_each(operands.begin(), operands.end(), [&](const auto& op) { op->apply(collector); }); + + return false; } context::id_index get_id(const std::string& s) { return a.context().hlasm_ctx->ids().add(s); } diff --git a/utils/include/utils/CMakeLists.txt b/utils/include/utils/CMakeLists.txt index d7b736470..2bf3c61a3 100644 --- a/utils/include/utils/CMakeLists.txt +++ b/utils/include/utils/CMakeLists.txt @@ -24,6 +24,7 @@ target_sources(hlasm_utils PUBLIC resource_location.h similar.h string_operations.h + task.h text_matchers.h time.h truth_table.h diff --git a/utils/include/utils/task.h b/utils/include/utils/task.h new file mode 100644 index 000000000..bfcf039af --- /dev/null +++ b/utils/include/utils/task.h @@ -0,0 +1,149 @@ +/* + * Copyright (c) 2023 Broadcom. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Broadcom, Inc. - initial API and implementation + */ + +#ifndef HLASMPLUGIN_UTILS_TASK_H +#define HLASMPLUGIN_UTILS_TASK_H + +#include +#include +#include +#include + +namespace hlasm_plugin::utils { + +struct task +{ + class awaiter; + struct promise_type + { + task get_return_object() { return task(std::coroutine_handle::from_promise(*this)); } + std::suspend_always initial_suspend() const noexcept { return {}; } + std::suspend_always final_suspend() noexcept + { + detach(); + return {}; + } + void return_void() const noexcept {} + void unhandled_exception() + { + if (!active || pending_exception) + throw; + pending_exception = std::current_exception(); + } + + std::coroutine_handle next_step = std::coroutine_handle::from_promise(*this); + awaiter* active = nullptr; + std::coroutine_handle top_waiter = std::coroutine_handle::from_promise(*this); + std::exception_ptr pending_exception; + + void attach(std::coroutine_handle current_top_waiter, awaiter* a) + { + current_top_waiter.promise().next_step = std::coroutine_handle::from_promise(*this); + active = a; + top_waiter = std::move(current_top_waiter); + } + + void detach() noexcept + { + if (active) + { + active->to_resume.promise().pending_exception = std::exchange(pending_exception, {}); + top_waiter.promise().next_step = active->to_resume; + + next_step = std::coroutine_handle::from_promise(*this); + active = nullptr; + top_waiter = std::coroutine_handle::from_promise(*this); + } + } + }; + + class awaiter + { + promise_type& self; + std::coroutine_handle to_resume {}; + + friend struct promise_type; + + public: + constexpr bool await_ready() const noexcept { return false; } + bool await_suspend(std::coroutine_handle h) noexcept + { + self.attach(h.promise().top_waiter, this); + to_resume = std::move(h); + return true; + } + void await_resume() const + { + if (to_resume.promise().pending_exception) + std::rethrow_exception(std::exchange(to_resume.promise().pending_exception, {})); + } + + explicit awaiter(promise_type& self) noexcept + : self(self) + {} + }; + + explicit task(std::coroutine_handle handle) + : m_handle(std::move(handle)) + {} + task(task&& t) noexcept + : m_handle(std::exchange(t.m_handle, {})) + {} + task& operator=(task&& t) noexcept + { + task tmp(std::move(t)); + std::swap(m_handle, tmp.m_handle); + + return *this; + } + ~task() + { + if (m_handle) + { + // pending exception will be dropped - should we do something about it? + m_handle.promise().detach(); + m_handle.destroy(); + } + } + + bool done() const noexcept + { + assert(m_handle); + return m_handle.done(); + } + + void operator()() const + { + assert(m_handle); + m_handle.promise().next_step(); + } + + auto operator co_await() const&& { return awaiter(m_handle.promise()); } + + static std::suspend_always suspend() { return {}; } + + std::exception_ptr pending_exception(bool clear = false) const + { + assert(m_handle); + auto& excp = m_handle.promise().next_step.promise().pending_exception; + return clear ? std::exchange(excp, {}) : excp; + } + +private: + std::coroutine_handle m_handle; +}; + +} // namespace hlasm_plugin::utils + +#endif diff --git a/utils/test/CMakeLists.txt b/utils/test/CMakeLists.txt index 6d845908a..b6b47ebd7 100644 --- a/utils/test/CMakeLists.txt +++ b/utils/test/CMakeLists.txt @@ -21,6 +21,7 @@ target_sources(hlasm_utils_test PRIVATE resource_location_test.cpp unicode_text_test.cpp time_test.cpp + task_test.cpp ) target_link_libraries(hlasm_utils_test hlasm_utils) diff --git a/utils/test/task_test.cpp b/utils/test/task_test.cpp new file mode 100644 index 000000000..42a2aea37 --- /dev/null +++ b/utils/test/task_test.cpp @@ -0,0 +1,151 @@ +/* + * Copyright (c) 2023 Broadcom. + * The term "Broadcom" refers to Broadcom Inc. and/or its subsidiaries. + * + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Broadcom, Inc. - initial API and implementation + */ + + +#include "gtest/gtest.h" + +#include "utils/task.h" + +using namespace hlasm_plugin::utils; + + +TEST(task, basics) +{ + struct test_data + { + int f = 0; + int fail = 0; + int g = 0; + int h = 0; + + int excp = 0; + }; + static constexpr auto f = [](test_data& data) -> task { + data.f++; + co_return; + }; + static constexpr auto fail = [](test_data& data) -> task { + data.fail++; + static int i = 0; + throw &i; + co_return; + }; + static constexpr auto g = [](test_data& data) -> task { + data.g++; + co_await f(data); + try + { + co_await fail(data); + } + catch (int*) + { + data.excp++; + } + co_await f(data); // await suspend + }; + + static constexpr auto h = [](test_data& data) -> task { + data.h++; + co_await g(data); + co_await f(data); + co_await g(data); + }; + + test_data data; + + for (auto x = h(data); !x.done();) + x(); + + EXPECT_EQ(data.f, 5); + EXPECT_EQ(data.fail, 2); + EXPECT_EQ(data.g, 2); + EXPECT_EQ(data.h, 1); + EXPECT_EQ(data.excp, 2); +} + +TEST(task, excp_propagation) +{ + bool excp = false; + + static constexpr auto fail = []() -> task { + throw 0; + co_return; + }; + static constexpr auto inner = []() -> task { co_await fail(); }; + static constexpr auto outer = []() -> task { co_await inner(); }; + static constexpr auto main = [](bool& e) -> task { + try + { + co_await outer(); + } + catch (int) + { + e = true; + } + }; + + for (auto x = main(excp); !x.done();) + x(); + + EXPECT_TRUE(excp); +} + +TEST(task, excp_inspection) +{ + bool about_to_throw = false; + + static constexpr auto fail = [](bool& e) -> task { + e = true; + co_await task::suspend(); + throw 0; + }; + static constexpr auto main = [](bool& e) -> task { co_await fail(e); }; + + auto x = main(about_to_throw); + while (!x.done() && !about_to_throw) + x(); + + EXPECT_FALSE(x.done()); + EXPECT_FALSE(x.pending_exception()); + + x(); + + EXPECT_TRUE(x.pending_exception()); + + x.pending_exception(true); + + EXPECT_FALSE(x.pending_exception()); + + EXPECT_NO_THROW(([&x]() { + while (!x.done()) + x(); + }())); + + EXPECT_TRUE(x.done()); + EXPECT_FALSE(x.pending_exception()); +} + +TEST(task, direct_throw) +{ + static constexpr auto fail = []() -> task { + throw 0; + co_return; + }; + + auto x = fail(); + + EXPECT_FALSE(x.done()); + EXPECT_ANY_THROW(x()); + EXPECT_TRUE(x.done()); +}