From 7ed288bed4dd67b37b67ab51c3e0872359253406 Mon Sep 17 00:00:00 2001 From: Roberto Aloi Date: Thu, 1 Jun 2023 09:09:08 +0200 Subject: [PATCH] [DAP] Ensure breakpoints are purged on setBreakpoints (#1434) --- apps/els_dap/src/els_dap_general_provider.erl | 17 +++++ apps/els_dap/src/els_dap_rpc.erl | 5 ++ .../test/els_dap_general_provider_SUITE.erl | 72 ++++++++++++++++++- elvis.config | 1 + 4 files changed, 94 insertions(+), 1 deletion(-) diff --git a/apps/els_dap/src/els_dap_general_provider.erl b/apps/els_dap/src/els_dap_general_provider.erl index 3483999b9..dea535781 100644 --- a/apps/els_dap/src/els_dap_general_provider.erl +++ b/apps/els_dap/src/els_dap_general_provider.erl @@ -198,6 +198,11 @@ handle_request( ensure_connected(ProjectNode, Timeout), {Module, LineBreaks} = els_dap_breakpoints:build_source_breakpoints(Params), + %% Due to a bug in the OTP debugger and interpreter, removing the + %% breakpoints via 'int:no_break(Module)' is not enough. + %% See: https://github.com/erlang/otp/issues/7336 + force_delete_breakpoints(ProjectNode, Module, Breakpoints0), + {IsModuleAvailable, Message} = maybe_interpret_and_clear_module(ProjectNode, Module), Breakpoints1 = @@ -1125,3 +1130,15 @@ maybe_interpret_and_clear_module(ProjectNode, Module) -> ), {false, Msg} end. + +-spec force_delete_breakpoints(node(), module(), els_dap_breakpoints:breakpoints()) -> ok. +force_delete_breakpoints(ProjectNode, Module, Breakpoints) -> + case Breakpoints of + #{Module := #{line := Lines}} -> + [ + els_dap_rpc:delete_break(ProjectNode, Module, Line) + || Line <- maps:keys(Lines) + ]; + _ -> + ok + end. diff --git a/apps/els_dap/src/els_dap_rpc.erl b/apps/els_dap/src/els_dap_rpc.erl index e12057ee0..1742f6326 100644 --- a/apps/els_dap/src/els_dap_rpc.erl +++ b/apps/els_dap/src/els_dap_rpc.erl @@ -10,6 +10,7 @@ break_in/4, clear/1, continue/2, + delete_break/3, eval/3, file/3, get_meta/2, @@ -66,6 +67,10 @@ clear(Node) -> continue(Node, Pid) -> rpc:call(Node, int, continue, [Pid]). +-spec delete_break(node(), module(), non_neg_integer()) -> any(). +delete_break(Node, Module, Line) -> + rpc:call(Node, int, delete_break, [Module, Line]). + -spec eval(node(), string(), [any()]) -> any(). eval(Node, Input, Bindings) -> {ok, Tokens, _} = erl_scan:string(unicode:characters_to_list(Input) ++ "."), diff --git a/apps/els_dap/test/els_dap_general_provider_SUITE.erl b/apps/els_dap/test/els_dap_general_provider_SUITE.erl index 80851a13c..a84ffbd34 100644 --- a/apps/els_dap/test/els_dap_general_provider_SUITE.erl +++ b/apps/els_dap/test/els_dap_general_provider_SUITE.erl @@ -34,7 +34,8 @@ log_points_with_hit/1, log_points_with_hit1/1, log_points_with_cond_and_hit/1, - log_points_empty_cond/1 + log_points_empty_cond/1, + remove_breakpoint/1 ]). %%============================================================================== @@ -165,6 +166,26 @@ wait_for_break(NodeName, WantModule, WantLine) -> end, els_dap_test_utils:wait_for_fun(Checker, 200, 20). +-spec wait_for_exit(binary(), module()) -> boolean(). +wait_for_exit(NodeName, WantModule) -> + Node = binary_to_atom(NodeName, utf8), + Checker = + fun() -> + Snapshots = rpc:call(Node, int, snapshot, []), + lists:any( + fun + ({_, {Module, _, _}, exit, normal}) when + Module =:= WantModule + -> + true; + (_) -> + false + end, + Snapshots + ) + end, + els_dap_test_utils:wait_for_fun(Checker, 200, 20). + %%============================================================================== %% Testcases %%============================================================================== @@ -669,6 +690,55 @@ log_points_base(Config, LogLine, Params, BreakLine, NumCalls) -> ), ok. +-spec remove_breakpoint(config()) -> ok. +remove_breakpoint(Config) -> + Provider = els_dap_general_provider, + DataDir = ?config(data_dir, Config), + Node = ?config(node, Config), + BreakLine = 9, + Params = #{}, + + %% Initialize + els_dap_provider:handle_request(Provider, request_initialize(#{})), + els_dap_provider:handle_request( + Provider, + request_launch(DataDir, Node, els_dap_test_module, dummy, [unused]) + ), + els_test_utils:wait_until_mock_called(els_dap_server, send_event), + %% Set Breakpoint + meck:reset([els_dap_server]), + els_dap_provider:handle_request( + Provider, + request_set_breakpoints( + path_to_test_module(DataDir, els_dap_test_module), + [{BreakLine, Params}] + ) + ), + %% Spawn a process on the target node which will hit the + %% breakpoint multiple times + spawn(binary_to_atom(Node), fun() -> els_dap_test_module:entry(10) end), + %% Wait until we hit the breakpoint for the first time + ?assertEqual(ok, wait_for_break(Node, els_dap_test_module, BreakLine)), + %% Retrieve the list of active threads + #{<<"threads">> := [#{<<"id">> := ThreadId}]} = + els_dap_provider:handle_request( + Provider, + request_threads() + ), + %% Reset the breakpoint + els_dap_provider:handle_request( + Provider, + request_set_breakpoints( + path_to_test_module(DataDir, els_dap_test_module), + [] + ) + ), + %% Continue thread execution + els_dap_provider:handle_request(Provider, request_continue(ThreadId)), + %% Check for process termination + ?assertEqual(ok, wait_for_exit(Node, els_dap_test_module)), + ok. + %%============================================================================== %% Requests %%============================================================================== diff --git a/elvis.config b/elvis.config index aa077b578..528395ab1 100644 --- a/elvis.config +++ b/elvis.config @@ -18,6 +18,7 @@ els_client, els_completion_SUITE, els_dap_general_provider_SUITE, + els_dap_rpc, els_definition_SUITE, els_diagnostics_SUITE, els_document_highlight_SUITE,