From be7efe13ff24d23dac145c69a97e5f4a4d2c1296 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Sun, 9 Jul 2023 08:05:59 +0200 Subject: [PATCH 1/8] break in all protocol implementations --- .../elixir_ls_debugger/lib/debugger/server.ex | 66 ++++++---- .../elixir_ls_debugger/test/debugger_test.exs | 120 +++++++++++++++--- .../mix_project/lib/protocol_breakpoints.ex | 16 +++ 3 files changed, 161 insertions(+), 41 deletions(-) create mode 100644 apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex index 55beb5ee6..711be1f80 100644 --- a/apps/elixir_ls_debugger/lib/debugger/server.ex +++ b/apps/elixir_ls_debugger/lib/debugger/server.ex @@ -269,17 +269,17 @@ defmodule ElixirLS.Debugger.Server do for b <- breakpoints, do: {b["condition"], b["logMessage"], b["hitCondition"]} existing_bps = state.breakpoints[path] || [] - existing_bp_lines = for {_module, line} <- existing_bps, do: line + existing_bp_lines = for {_modules, line} <- existing_bps, do: line removed_lines = existing_bp_lines -- new_lines removed_bps = Enum.filter(existing_bps, fn {_, line} -> line in removed_lines end) - for {module, line} <- removed_bps do + for {modules, line} <- removed_bps, module <- modules do :int.delete_break(module, line) BreakpointCondition.unregister_condition(module, [line]) end result = set_breakpoints(path, new_lines |> Enum.zip(new_conditions)) - new_bps = for {:ok, module, line} <- result, do: {module, line} + new_bps = for {:ok, modules, line} <- result, do: {modules, line} state = if new_bps == [] do @@ -1151,37 +1151,49 @@ defmodule ElixirLS.Debugger.Server do defp set_breakpoints(path, lines) do if Path.extname(path) == ".erl" do module = String.to_atom(Path.basename(path, ".erl")) - for line <- lines, do: set_breakpoint(module, line) + for line <- lines, do: set_breakpoint([module], path, line) else - try do - metadata = ElixirSense.Core.Parser.parse_file(path, false, false, nil) + metadata = ElixirSense.Core.Parser.parse_file(path, false, false, nil) - for line <- lines do - env = ElixirSense.Core.Metadata.get_env(metadata, {line |> elem(0), 1}) + for line <- lines do + env = ElixirSense.Core.Metadata.get_env(metadata, {line |> elem(0), 1}) - if env.module == nil do - {:error, "Could not determine module at line"} - else - set_breakpoint(env.module, line) - end - end - rescue - error -> - for _line <- lines, do: {:error, Exception.format_exit(error)} + modules = + env.module_variants + |> Enum.filter(&(&1 != Elixir)) + + set_breakpoint(modules, path, line) end end end - defp set_breakpoint(module, {line, {condition, log_message, hit_count}}) do - case :int.ni(module) do - {:module, _} -> - :int.break(module, line) - update_break_condition(module, line, condition, log_message, hit_count) + defp set_breakpoint([], path, {line, _}) do + {:error, "Could not determine module at line #{line} in #{path}"} + end + + defp set_breakpoint(modules, path, {line, {condition, log_message, hit_count}}) do + modules_with_breakpoints = + Enum.reduce(modules, [], fn module, added -> + case :int.ni(module) do + {:module, _} -> + Output.debugger_console("breaking in #{module}") + :int.break(module, line) + update_break_condition(module, line, condition, log_message, hit_count) - {:ok, module, line} + [module | added] - _ -> - {:error, "Cannot interpret module #{inspect(module)}"} + :error -> + Output.debugger_console("Could not interpret module #{inspect(module)} in #{path}") + added + end + end) + + if modules_with_breakpoints == [] do + {:error, + "Could not interpret any of the modules #{Enum.map_join(modules, ", ", &inspect/1)} in #{path}"} + else + # return :ok if at least one breakpoint was set + {:ok, modules_with_breakpoints, line} end end @@ -1342,7 +1354,9 @@ defmodule ElixirLS.Debugger.Server do function_breakpoints = Map.get(state.function_breakpoints, frame_mfa, []) cond do - {first_frame.module, first_frame.line} in file_breakpoints -> + Enum.any?(file_breakpoints, fn {modules, line} -> + line == first_frame.line and first_frame.module in modules + end) -> "breakpoint" first_frame.line in function_breakpoints -> diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index d25511742..8cdcfd4c2 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -844,7 +844,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert :hello in :int.interpreted() assert [{{:hello, 5}, [:active, :enable, :null, _]}] = :int.all_breaks(:hello) - assert %{^abs_path => [hello: 5]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[:hello], 5}]} = :sys.get_state(server).breakpoints Server.receive_packet( server, @@ -860,7 +860,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert [{{:hello, 5}, _}, {{:hello, 6}, _}] = :int.all_breaks(:hello) - assert %{^abs_path => [{:hello, 5}, {:hello, 6}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[:hello], 5}, {[:hello], 6}]} = :sys.get_state(server).breakpoints Server.receive_packet( server, @@ -873,7 +873,7 @@ defmodule ElixirLS.Debugger.ServerTest do ) assert [{{:hello, 6}, _}] = :int.all_breaks(:hello) - assert %{^abs_path => [{:hello, 6}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[:hello], 6}]} = :sys.get_state(server).breakpoints Server.receive_packet( server, @@ -930,7 +930,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert [{{MixProject, 3}, [:active, :enable, :null, _]}] = :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints Server.receive_packet( server, @@ -953,7 +953,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert [{{MixProject.Some, 35}, _}] = :int.all_breaks(MixProject.Some) assert %{ - ^abs_path => [{MixProject, 3}, {MixProject, 7}, {MixProject.Some, 35}] + ^abs_path => [{[MixProject], 3}, {[MixProject], 7}, {[MixProject.Some], 35}] } = :sys.get_state(server).breakpoints Server.receive_packet( @@ -976,7 +976,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert [{{MixProject.Some, 35}, _}, {{MixProject.Some, 39}, _}] = :int.all_breaks(MixProject.Some) - assert %{^abs_path => [{MixProject.Some, 35}, {MixProject.Some, 39}]} = + assert %{^abs_path => [{[MixProject.Some], 35}, {[MixProject.Some], 39}]} = :sys.get_state(server).breakpoints Server.receive_packet( @@ -1039,7 +1039,7 @@ defmodule ElixirLS.Debugger.ServerTest do ] == :int.all_breaks(MixProject) - assert %{^abs_path_1 => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path_1 => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints Server.receive_packet( server, @@ -1054,7 +1054,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert :hello in :int.interpreted() assert [{{:hello, 5}, _}] = :int.all_breaks(:hello) - assert %{abs_path_1 => [{MixProject, 3}], abs_path_2 => [hello: 5]} == + assert %{abs_path_1 => [{[MixProject], 3}], abs_path_2 => [{[:hello], 5}]} == :sys.get_state(server).breakpoints Server.receive_packet( @@ -1069,7 +1069,7 @@ defmodule ElixirLS.Debugger.ServerTest do assert [] = :int.all_breaks(MixProject) assert [{{:hello, 5}, _}] = :int.all_breaks(:hello) - assert %{abs_path_2 => [hello: 5]} == :sys.get_state(server).breakpoints + assert %{abs_path_2 => [{[:hello], 5}]} == :sys.get_state(server).breakpoints Server.receive_packet( server, @@ -1130,7 +1130,7 @@ defmodule ElixirLS.Debugger.ServerTest do {{MixProject, 3}, [:active, :enable, :null, _]} ] = :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints assert BreakpointCondition.has_condition?(MixProject, [3]) @@ -1155,7 +1155,7 @@ defmodule ElixirLS.Debugger.ServerTest do ] == :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints assert BreakpointCondition.has_condition?(MixProject, [3]) @@ -1225,7 +1225,7 @@ defmodule ElixirLS.Debugger.ServerTest do {{MixProject, 3}, [:active, :enable, :null, _]} ] = :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints assert BreakpointCondition.has_condition?(MixProject, [3]) @@ -1250,7 +1250,7 @@ defmodule ElixirLS.Debugger.ServerTest do ] == :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints assert BreakpointCondition.has_condition?(MixProject, [3]) @@ -1320,7 +1320,7 @@ defmodule ElixirLS.Debugger.ServerTest do {{MixProject, 3}, [:active, :enable, :null, _]} ] = :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints assert BreakpointCondition.has_condition?(MixProject, [3]) @@ -1345,7 +1345,7 @@ defmodule ElixirLS.Debugger.ServerTest do ] == :int.all_breaks(MixProject) - assert %{^abs_path => [{MixProject, 3}]} = :sys.get_state(server).breakpoints + assert %{^abs_path => [{[MixProject], 3}]} = :sys.get_state(server).breakpoints assert BreakpointCondition.has_condition?(MixProject, [3]) @@ -1685,6 +1685,96 @@ defmodule ElixirLS.Debugger.ServerTest do end end + @tag :fixture + test "breakpoint in protocol implementation", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) + + assert_receive(response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true})) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], + "projectDir" => File.cwd!() + }) + ) + + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") + + Server.receive_packet( + server, + set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 7}]) + ) + + assert_receive( + response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) + ) + + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 7, + "name" => "Proto.List.go/1", + "source" => %{"path" => ^abs_path} + }, + _ + ] + }) + when is_integer(frame_id) + + Server.receive_packet(server, continue_req(15, thread_id)) + assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => ^thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(8, thread_id)) + + assert_receive response(_, 8, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 7, + "name" => "Proto.BitString.go/1", + "source" => %{"path" => ^abs_path} + } + ] + }) + when is_integer(frame_id) + end) + end + @tag :fixture test "server tracks running processes", %{server: server} do in_fixture(__DIR__, "mix_project", fn -> diff --git a/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex b/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex new file mode 100644 index 000000000..bd6cee19f --- /dev/null +++ b/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex @@ -0,0 +1,16 @@ +defprotocol Proto do + def go(t) +end + +defimpl Proto, for: [List, BitString] do + def go(t) do + IO.inspect(t) + end +end + +defmodule ProtocolBreakpoints do + def go1() do + Proto.go([]) + Proto.go("") + end +end From b40286761c61f5490723667fb1a3390369ee62c8 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Sun, 9 Jul 2023 08:15:02 +0200 Subject: [PATCH 2/8] test unsetting implementation breakpoint --- .../elixir_ls_debugger/test/debugger_test.exs | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index 8cdcfd4c2..240188b54 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -1722,6 +1722,17 @@ defmodule ElixirLS.Debugger.ServerTest do response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) ) + assert Proto.List in :int.interpreted() + assert Proto.BitString in :int.interpreted() + + assert [{{Proto.List, 7}, [:active, :enable, :null, _]}] = :int.all_breaks(Proto.List) + + assert [{{Proto.BitString, 7}, [:active, :enable, :null, _]}] = + :int.all_breaks(Proto.BitString) + + assert %{^abs_path => [{[Proto.BitString, Proto.List], 7}]} = + :sys.get_state(server).breakpoints + Server.receive_packet(server, request(5, "configurationDone", %{})) assert_receive(response(_, 5, "configurationDone", %{})) @@ -1772,6 +1783,17 @@ defmodule ElixirLS.Debugger.ServerTest do ] }) when is_integer(frame_id) + + Server.receive_packet( + server, + set_breakpoints_req(9, %{"path" => abs_path}, []) + ) + + assert_receive(response(_, 9, "setBreakpoints", %{"breakpoints" => []})) + + assert [] = :int.all_breaks(Proto.List) + assert [] = :int.all_breaks(Proto.BitString) + assert %{} == :sys.get_state(server).breakpoints end) end From e67e999970e41ac93a7278af49840232158ab0ce Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Sun, 9 Jul 2023 08:50:02 +0200 Subject: [PATCH 3/8] add test --- .../elixir_ls_debugger/test/debugger_test.exs | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index 240188b54..026ca42cb 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -1797,6 +1797,114 @@ defmodule ElixirLS.Debugger.ServerTest do end) end + @tag :fixture + test "breakpoint in protocol", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) + + assert_receive(response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true})) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], + "projectDir" => File.cwd!() + }) + ) + + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") + + Server.receive_packet( + server, + set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 2}]) + ) + + assert_receive( + response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) + ) + + assert Proto.List in :int.interpreted() + assert Proto.BitString in :int.interpreted() + + assert [{{Proto, 2}, [:active, :enable, :null, _]}] = :int.all_breaks(Proto) + + assert %{^abs_path => [{[Proto], 2}]} = + :sys.get_state(server).breakpoints + + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 2, + "name" => "Proto.go/1", + "source" => %{"path" => ^abs_path} + }, + _ + ] + }) + when is_integer(frame_id) + + Server.receive_packet(server, continue_req(15, thread_id)) + assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => ^thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(8, thread_id)) + + assert_receive response(_, 8, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 2, + "name" => "Proto.go/1", + "source" => %{"path" => ^abs_path} + } + ] + }) + when is_integer(frame_id) + + Server.receive_packet( + server, + set_breakpoints_req(9, %{"path" => abs_path}, []) + ) + + assert_receive(response(_, 9, "setBreakpoints", %{"breakpoints" => []})) + + assert [] = :int.all_breaks(Proto) + assert %{} == :sys.get_state(server).breakpoints + end) + end + @tag :fixture test "server tracks running processes", %{server: server} do in_fixture(__DIR__, "mix_project", fn -> From 47d6e829ab3bd4a6981f520a5c8254e105a31878 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Sun, 9 Jul 2023 08:50:34 +0200 Subject: [PATCH 4/8] implement fallback to runtime modules inspection --- .../elixir_ls_debugger/lib/debugger/server.ex | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex index 711be1f80..250e7c92b 100644 --- a/apps/elixir_ls_debugger/lib/debugger/server.ex +++ b/apps/elixir_ls_debugger/lib/debugger/server.ex @@ -1153,16 +1153,33 @@ defmodule ElixirLS.Debugger.Server do module = String.to_atom(Path.basename(path, ".erl")) for line <- lines, do: set_breakpoint([module], path, line) else + loaded_elixir_modules = + :code.all_loaded() + |> Enum.map(&elem(&1, 0)) + |> Enum.filter(fn module -> String.starts_with?(Atom.to_string(module), "Elixir.") end) + |> Enum.group_by(fn module -> + Path.expand(to_string(module.module_info(:compile)[:source])) + end) + + loaded_modules_from_path = Map.get(loaded_elixir_modules, path, []) |> dbg metadata = ElixirSense.Core.Parser.parse_file(path, false, false, nil) for line <- lines do env = ElixirSense.Core.Metadata.get_env(metadata, {line |> elem(0), 1}) + metadata_modules = Enum.filter(env.module_variants, &(&1 != Elixir)) - modules = - env.module_variants - |> Enum.filter(&(&1 != Elixir)) + modules_to_break = + if metadata_modules != [] and + Enum.all?(metadata_modules, &(&1 in loaded_modules_from_path)) do + # prefer metadata modules if valid and loaded + metadata_modules + else + # fall back to all loaded modules from file + # this may create breakpoints outside module line range + loaded_modules_from_path + end - set_breakpoint(modules, path, line) + set_breakpoint(modules_to_break, path, line) end end end From b2792a09d3001e8fa98c669911b4f0abcbda43c6 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Mon, 10 Jul 2023 07:24:18 +0200 Subject: [PATCH 5/8] more tests and logs --- .../elixir_ls_debugger/lib/debugger/server.ex | 11 +- .../elixir_ls_debugger/test/debugger_test.exs | 582 ++++++++++++------ 2 files changed, 401 insertions(+), 192 deletions(-) diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex index 250e7c92b..347d2e5ac 100644 --- a/apps/elixir_ls_debugger/lib/debugger/server.ex +++ b/apps/elixir_ls_debugger/lib/debugger/server.ex @@ -336,6 +336,11 @@ defmodule ElixirLS.Debugger.Server do case :int.ni(m) do {:module, _} -> breaks_before = :int.all_breaks(m) + + Output.debugger_console( + "Setting function breakpoint in #{inspect(m)}.#{f}/#{a}" + ) + :ok = :int.break_in(m, f, a) breaks_after = :int.all_breaks(m) lines = for {{^m, line}, _} <- breaks_after -- breaks_before, do: line @@ -1145,6 +1150,7 @@ defmodule ElixirLS.Debugger.Server do defp save_and_reload(module, beam_bin) do :ok = File.write(Path.join(@temp_beam_dir, to_string(module) <> ".beam"), beam_bin) true = :code.delete(module) + Output.debugger_console("Interpreting module #{inspect(module)}") {:module, _} = :int.ni(module) end @@ -1161,7 +1167,7 @@ defmodule ElixirLS.Debugger.Server do Path.expand(to_string(module.module_info(:compile)[:source])) end) - loaded_modules_from_path = Map.get(loaded_elixir_modules, path, []) |> dbg + loaded_modules_from_path = Map.get(loaded_elixir_modules, path, []) metadata = ElixirSense.Core.Parser.parse_file(path, false, false, nil) for line <- lines do @@ -1193,7 +1199,7 @@ defmodule ElixirLS.Debugger.Server do Enum.reduce(modules, [], fn module, added -> case :int.ni(module) do {:module, _} -> - Output.debugger_console("breaking in #{module}") + Output.debugger_console("Setting breakpoint in #{inspect(module)} #{path}:#{line}") :int.break(module, line) update_break_condition(module, line, condition, log_message, hit_count) @@ -1225,6 +1231,7 @@ defmodule ElixirLS.Debugger.Server do defp interpret_module(mod) do try do + Output.debugger_console("Interpreting module #{inspect(mod)}") {:module, _} = :int.ni(mod) catch _, _ -> diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index 026ca42cb..21c5f0f4e 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -1370,6 +1370,229 @@ defmodule ElixirLS.Debugger.ServerTest do refute BreakpointCondition.has_condition?(MixProject, [3]) end) end + + @tag :fixture + test "breakpoint in protocol", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) + + assert_receive( + response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true}) + ) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], + "projectDir" => File.cwd!() + }) + ) + + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") + + Server.receive_packet( + server, + set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 2}]) + ) + + assert_receive( + response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) + ) + + assert Proto in :int.interpreted() + + assert [{{Proto, 2}, [:active, :enable, :null, _]}] = :int.all_breaks(Proto) + + assert %{^abs_path => [{[Proto], 2}]} = + :sys.get_state(server).breakpoints + + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 2, + "name" => "Proto.go/1", + "source" => %{"path" => ^abs_path} + }, + _ + ] + }) + when is_integer(frame_id) + + Server.receive_packet(server, continue_req(15, thread_id)) + assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => ^thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(8, thread_id)) + + assert_receive response(_, 8, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 2, + "name" => "Proto.go/1", + "source" => %{"path" => ^abs_path} + } + ] + }) + when is_integer(frame_id) + + Server.receive_packet( + server, + set_breakpoints_req(9, %{"path" => abs_path}, []) + ) + + assert_receive(response(_, 9, "setBreakpoints", %{"breakpoints" => []})) + + assert [] = :int.all_breaks(Proto) + assert %{} == :sys.get_state(server).breakpoints + end) + end + + @tag :fixture + test "breakpoint in protocol implementation", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) + + assert_receive( + response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true}) + ) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], + "projectDir" => File.cwd!() + }) + ) + + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") + + Server.receive_packet( + server, + set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 7}]) + ) + + assert_receive( + response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) + ) + + assert Proto.List in :int.interpreted() + assert Proto.BitString in :int.interpreted() + + assert [{{Proto.List, 7}, [:active, :enable, :null, _]}] = :int.all_breaks(Proto.List) + + assert [{{Proto.BitString, 7}, [:active, :enable, :null, _]}] = + :int.all_breaks(Proto.BitString) + + assert %{^abs_path => [{[Proto.BitString, Proto.List], 7}]} = + :sys.get_state(server).breakpoints + + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 7, + "name" => "Proto.List.go/1", + "source" => %{"path" => ^abs_path} + }, + _ + ] + }) + when is_integer(frame_id) + + Server.receive_packet(server, continue_req(15, thread_id)) + assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "breakpoint", + "threadId" => ^thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(8, thread_id)) + + assert_receive response(_, 8, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 7, + "name" => "Proto.BitString.go/1", + "source" => %{"path" => ^abs_path} + } + ] + }) + when is_integer(frame_id) + + Server.receive_packet( + server, + set_breakpoints_req(9, %{"path" => abs_path}, []) + ) + + assert_receive(response(_, 9, "setBreakpoints", %{"breakpoints" => []})) + + assert [] = :int.all_breaks(Proto.List) + assert [] = :int.all_breaks(Proto.BitString) + assert %{} == :sys.get_state(server).breakpoints + end) + end end describe "function breakpoints" do @@ -1683,226 +1906,205 @@ defmodule ElixirLS.Debugger.ServerTest do 5000 end) end - end - - @tag :fixture - test "breakpoint in protocol implementation", %{server: server} do - in_fixture(__DIR__, "mix_project", fn -> - Server.receive_packet( - server, - initialize_req(1, %{ - "supportsVariablePaging" => true, - "supportsVariableType" => true - }) - ) - assert_receive(response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true})) - - Server.receive_packet( - server, - launch_req(2, %{ - "request" => "launch", - "type" => "mix_task", - "task" => "run", - "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], - "projectDir" => File.cwd!() - }) - ) - - assert_receive(response(_, 2, "launch", %{}), 5000) - assert_receive(event(_, "initialized", %{})) - abs_path = Path.absname("lib/protocol_breakpoints.ex") - - Server.receive_packet( - server, - set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 7}]) - ) - - assert_receive( - response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) - ) - - assert Proto.List in :int.interpreted() - assert Proto.BitString in :int.interpreted() - - assert [{{Proto.List, 7}, [:active, :enable, :null, _]}] = :int.all_breaks(Proto.List) - - assert [{{Proto.BitString, 7}, [:active, :enable, :null, _]}] = - :int.all_breaks(Proto.BitString) - - assert %{^abs_path => [{[Proto.BitString, Proto.List], 7}]} = - :sys.get_state(server).breakpoints - - Server.receive_packet(server, request(5, "configurationDone", %{})) - assert_receive(response(_, 5, "configurationDone", %{})) - - assert_receive event(_, "stopped", %{ - "allThreadsStopped" => false, - "reason" => "breakpoint", - "threadId" => thread_id - }), - 5_000 + @tag :fixture + test "function breakpoint in protocol", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) - Server.receive_packet(server, stacktrace_req(7, thread_id)) + assert_receive( + response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true}) + ) - assert_receive response(_, 7, "stackTrace", %{ - "stackFrames" => [ - %{ - "column" => 0, - "id" => frame_id, - "line" => 7, - "name" => "Proto.List.go/1", - "source" => %{"path" => ^abs_path} - }, - _ - ] - }) - when is_integer(frame_id) + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], + "projectDir" => File.cwd!() + }) + ) - Server.receive_packet(server, continue_req(15, thread_id)) - assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") - assert_receive event(_, "stopped", %{ - "allThreadsStopped" => false, - "reason" => "breakpoint", - "threadId" => ^thread_id - }), - 5_000 + Server.receive_packet( + server, + set_function_breakpoints_req(3, [%{"name" => "Proto.go/1"}]) + ) - Server.receive_packet(server, stacktrace_req(8, thread_id)) + assert_receive( + response(_, 3, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => true}]}), + 3000 + ) - assert_receive response(_, 8, "stackTrace", %{ - "stackFrames" => [ - %{ - "column" => 0, - "id" => frame_id, - "line" => 7, - "name" => "Proto.BitString.go/1", - "source" => %{"path" => ^abs_path} - } - ] - }) - when is_integer(frame_id) + assert Proto in :int.interpreted() - Server.receive_packet( - server, - set_breakpoints_req(9, %{"path" => abs_path}, []) - ) + assert [ + {{Proto, 2}, + [:active, :enable, :null, {ElixirLS.Debugger.BreakpointCondition, :check_0}]} + ] = :int.all_breaks(Proto) - assert_receive(response(_, 9, "setBreakpoints", %{"breakpoints" => []})) + assert %{{Proto, :go, 1} => [2]} = :sys.get_state(server).function_breakpoints - assert [] = :int.all_breaks(Proto.List) - assert [] = :int.all_breaks(Proto.BitString) - assert %{} == :sys.get_state(server).breakpoints - end) - end + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) - @tag :fixture - test "breakpoint in protocol", %{server: server} do - in_fixture(__DIR__, "mix_project", fn -> - Server.receive_packet( - server, - initialize_req(1, %{ - "supportsVariablePaging" => true, - "supportsVariableType" => true - }) - ) + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "function breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 2, + "name" => "Proto.go/1", + "source" => %{"path" => ^abs_path} + }, + _ + ] + }) + when is_integer(frame_id) - assert_receive(response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true})) + Server.receive_packet(server, continue_req(15, thread_id)) + assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) - Server.receive_packet( - server, - launch_req(2, %{ - "request" => "launch", - "type" => "mix_task", - "task" => "run", - "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], - "projectDir" => File.cwd!() - }) - ) + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "function breakpoint", + "threadId" => ^thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(8, thread_id)) + + assert_receive response(_, 8, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 2, + "name" => "Proto.go/1", + "source" => %{"path" => ^abs_path} + } + ] + }) + when is_integer(frame_id) - assert_receive(response(_, 2, "launch", %{}), 5000) - assert_receive(event(_, "initialized", %{})) - abs_path = Path.absname("lib/protocol_breakpoints.ex") + Server.receive_packet( + server, + set_function_breakpoints_req(9, []) + ) - Server.receive_packet( - server, - set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 2}]) - ) + assert_receive(response(_, 9, "setFunctionBreakpoints", %{"breakpoints" => []})) - assert_receive( - response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}) - ) + assert [] = :int.all_breaks(Proto) + assert %{} == :sys.get_state(server).function_breakpoints + end) + end - assert Proto.List in :int.interpreted() - assert Proto.BitString in :int.interpreted() + @tag :fixture + test "function breakpoint in protocol implementation", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) - assert [{{Proto, 2}, [:active, :enable, :null, _]}] = :int.all_breaks(Proto) + assert_receive( + response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true}) + ) - assert %{^abs_path => [{[Proto], 2}]} = - :sys.get_state(server).breakpoints + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go1()"], + "projectDir" => File.cwd!() + }) + ) - Server.receive_packet(server, request(5, "configurationDone", %{})) - assert_receive(response(_, 5, "configurationDone", %{})) + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") - assert_receive event(_, "stopped", %{ - "allThreadsStopped" => false, - "reason" => "breakpoint", - "threadId" => thread_id - }), - 5_000 + Server.receive_packet( + server, + set_function_breakpoints_req(3, [%{"name" => "Proto.List.go/1"}]) + ) - Server.receive_packet(server, stacktrace_req(7, thread_id)) + assert_receive( + response(_, 3, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => true}]}), + 3000 + ) - assert_receive response(_, 7, "stackTrace", %{ - "stackFrames" => [ - %{ - "column" => 0, - "id" => frame_id, - "line" => 2, - "name" => "Proto.go/1", - "source" => %{"path" => ^abs_path} - }, - _ - ] - }) - when is_integer(frame_id) + assert Proto.List in :int.interpreted() - Server.receive_packet(server, continue_req(15, thread_id)) - assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true}) + assert [ + {{Proto.List, 7}, + [:active, :enable, :null, {ElixirLS.Debugger.BreakpointCondition, :check_0}]} + ] = :int.all_breaks(Proto.List) - assert_receive event(_, "stopped", %{ - "allThreadsStopped" => false, - "reason" => "breakpoint", - "threadId" => ^thread_id - }), - 5_000 + assert %{{Proto.List, :go, 1} => [7]} = :sys.get_state(server).function_breakpoints - Server.receive_packet(server, stacktrace_req(8, thread_id)) + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) - assert_receive response(_, 8, "stackTrace", %{ - "stackFrames" => [ - %{ - "column" => 0, - "id" => frame_id, - "line" => 2, - "name" => "Proto.go/1", - "source" => %{"path" => ^abs_path} - } - ] - }) - when is_integer(frame_id) + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "function breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 7, + "name" => "Proto.List.go/1", + "source" => %{"path" => ^abs_path} + }, + _ + ] + }) + when is_integer(frame_id) - Server.receive_packet( - server, - set_breakpoints_req(9, %{"path" => abs_path}, []) - ) + Server.receive_packet( + server, + set_function_breakpoints_req(9, []) + ) - assert_receive(response(_, 9, "setBreakpoints", %{"breakpoints" => []})) + assert_receive(response(_, 9, "setFunctionBreakpoints", %{"breakpoints" => []})) - assert [] = :int.all_breaks(Proto) - assert %{} == :sys.get_state(server).breakpoints - end) + assert [] = :int.all_breaks(Proto.List) + assert %{} == :sys.get_state(server).function_breakpoints + end) + end end @tag :fixture From 0481fb868a2cc6f3df4cfa9a33bd6899cc1d7144 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Tue, 11 Jul 2023 06:16:33 +0200 Subject: [PATCH 6/8] cache module_info of interpreted modules call to module_info may deadlock when stopped on a breakpoint Fixes https://github.com/elixir-lsp/elixir-ls/issues/940 --- .../lib/debugger/module_info_cache.ex | 30 +++++++ .../elixir_ls_debugger/lib/debugger/server.ex | 11 ++- .../lib/debugger/stacktrace.ex | 5 +- .../elixir_ls_debugger/test/debugger_test.exs | 90 ++++++++++++++++++- .../mix_project/lib/protocol_breakpoints.ex | 29 ++++++ 5 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 apps/elixir_ls_debugger/lib/debugger/module_info_cache.ex diff --git a/apps/elixir_ls_debugger/lib/debugger/module_info_cache.ex b/apps/elixir_ls_debugger/lib/debugger/module_info_cache.ex new file mode 100644 index 000000000..48b21b89e --- /dev/null +++ b/apps/elixir_ls_debugger/lib/debugger/module_info_cache.ex @@ -0,0 +1,30 @@ +defmodule ElixirLS.Debugger.ModuleInfoCache do + @moduledoc """ + Caches module_info of interpreted modules. There are cases when module_info call + may deadlock (https://github.com/elixir-lsp/elixir-ls/issues/940) + """ + + use Agent + + def start_link(args) do + Agent.start_link(fn -> args end, name: __MODULE__) + end + + def get(module) do + Agent.get(__MODULE__, & &1[module]) + end + + def store(module) do + Agent.update(__MODULE__, fn map -> + if Map.has_key?(map, module) do + map + else + Map.put(map, module, module.module_info()) + end + end) + end + + def clear() do + Agent.update(__MODULE__, fn _map -> %{} end) + end +end diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex index 347d2e5ac..9611952be 100644 --- a/apps/elixir_ls_debugger/lib/debugger/server.ex +++ b/apps/elixir_ls_debugger/lib/debugger/server.ex @@ -23,7 +23,8 @@ defmodule ElixirLS.Debugger.Server do Variables, Utils, BreakpointCondition, - Binding + Binding, + ModuleInfoCache } alias ElixirLS.Debugger.Stacktrace.Frame @@ -84,6 +85,7 @@ defmodule ElixirLS.Debugger.Server do @impl GenServer def init(opts) do BreakpointCondition.start_link([]) + ModuleInfoCache.start_link(%{}) state = if opts[:output], do: %__MODULE__{output: opts[:output]}, else: %__MODULE__{} {:ok, state} end @@ -335,6 +337,7 @@ defmodule ElixirLS.Debugger.Server do nil -> case :int.ni(m) do {:module, _} -> + ModuleInfoCache.store(m) breaks_before = :int.all_breaks(m) Output.debugger_console( @@ -1152,6 +1155,7 @@ defmodule ElixirLS.Debugger.Server do true = :code.delete(module) Output.debugger_console("Interpreting module #{inspect(module)}") {:module, _} = :int.ni(module) + ModuleInfoCache.store(module) end defp set_breakpoints(path, lines) do @@ -1164,7 +1168,8 @@ defmodule ElixirLS.Debugger.Server do |> Enum.map(&elem(&1, 0)) |> Enum.filter(fn module -> String.starts_with?(Atom.to_string(module), "Elixir.") end) |> Enum.group_by(fn module -> - Path.expand(to_string(module.module_info(:compile)[:source])) + module_info = ModuleInfoCache.get(module) || module.module_info() + Path.expand(to_string(module_info[:compile][:source])) end) loaded_modules_from_path = Map.get(loaded_elixir_modules, path, []) @@ -1199,6 +1204,7 @@ defmodule ElixirLS.Debugger.Server do Enum.reduce(modules, [], fn module, added -> case :int.ni(module) do {:module, _} -> + ModuleInfoCache.store(module) Output.debugger_console("Setting breakpoint in #{inspect(module)} #{path}:#{line}") :int.break(module, line) update_break_condition(module, line, condition, log_message, hit_count) @@ -1233,6 +1239,7 @@ defmodule ElixirLS.Debugger.Server do try do Output.debugger_console("Interpreting module #{inspect(mod)}") {:module, _} = :int.ni(mod) + ModuleInfoCache.store(mod) catch _, _ -> Output.debugger_important( diff --git a/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex b/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex index 549a5b3da..1886d1fc0 100644 --- a/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex +++ b/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex @@ -3,6 +3,7 @@ defmodule ElixirLS.Debugger.Stacktrace do Retrieves the stack trace for a process that's paused at a breakpoint """ alias ElixirLS.Debugger.Output + alias ElixirLS.Debugger.ModuleInfoCache defmodule Frame do defstruct [:level, :file, :module, :function, :args, :line, :bindings, :messages] @@ -86,8 +87,6 @@ defmodule ElixirLS.Debugger.Stacktrace do end defp get_file(module) do - if Code.ensure_loaded?(module) do - to_string(module.module_info[:compile][:source]) - end + Path.expand(to_string(ModuleInfoCache.get(module)[:compile][:source])) end end diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index 21c5f0f4e..41c040b5b 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -4,7 +4,7 @@ defmodule ElixirLS.Debugger.ServerTest do # between the debugger's tests and the fixture project's tests. Expect to see output printed # from both. - alias ElixirLS.Debugger.{Server, Protocol, BreakpointCondition} + alias ElixirLS.Debugger.{Server, Protocol, BreakpointCondition, ModuleInfoCache} use ElixirLS.Utils.MixTest.Case, async: false use Protocol @@ -22,6 +22,7 @@ defmodule ElixirLS.Debugger.ServerTest do :int.no_break() :int.clear() BreakpointCondition.clear() + ModuleInfoCache.clear() end) {:ok, %{server: server}} @@ -2105,6 +2106,93 @@ defmodule ElixirLS.Debugger.ServerTest do assert %{} == :sys.get_state(server).function_breakpoints end) end + + @tag :fixture + test "function breakpoint in derived protocol implementation", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet( + server, + initialize_req(1, %{ + "supportsVariablePaging" => true, + "supportsVariableType" => true + }) + ) + + assert_receive( + response(_, 1, "initialize", %{"supportsConfigurationDoneRequest" => true}) + ) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "run", + "taskArgs" => ["-e", "ProtocolBreakpoints.go2()"], + "projectDir" => File.cwd!() + }) + ) + + assert_receive(response(_, 2, "launch", %{}), 5000) + assert_receive(event(_, "initialized", %{})) + abs_path = Path.absname("lib/protocol_breakpoints.ex") + + Server.receive_packet( + server, + set_function_breakpoints_req(3, [%{"name" => "DerivedProto.MyStruct.go/1"}]) + ) + + assert_receive( + response(_, 3, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => true}]}), + 3000 + ) + + assert DerivedProto.MyStruct in :int.interpreted() + + assert [ + {{DerivedProto.MyStruct, 33}, + [:active, :enable, :null, {ElixirLS.Debugger.BreakpointCondition, :check_0}]} + ] = :int.all_breaks(DerivedProto.MyStruct) + + assert %{{DerivedProto.MyStruct, :go, 1} => [33]} = + :sys.get_state(server).function_breakpoints + + Server.receive_packet(server, request(5, "configurationDone", %{})) + assert_receive(response(_, 5, "configurationDone", %{})) + + assert_receive event(_, "stopped", %{ + "allThreadsStopped" => false, + "reason" => "function breakpoint", + "threadId" => thread_id + }), + 5_000 + + Server.receive_packet(server, stacktrace_req(7, thread_id)) + + assert_receive response(_, 7, "stackTrace", %{ + "stackFrames" => [ + %{ + "column" => 0, + "id" => frame_id, + "line" => 33, + "name" => "DerivedProto.MyStruct.go/1", + "source" => %{"path" => ^abs_path} + } + ] + }) + when is_integer(frame_id) + + Server.receive_packet( + server, + set_function_breakpoints_req(9, []) + ) + + assert_receive(response(_, 9, "setFunctionBreakpoints", %{"breakpoints" => []})) + + assert [] = :int.all_breaks(DerivedProto.MyStruct) + assert %{} == :sys.get_state(server).function_breakpoints + end) + end end @tag :fixture diff --git a/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex b/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex index bd6cee19f..85a96bef0 100644 --- a/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex +++ b/apps/elixir_ls_debugger/test/fixtures/mix_project/lib/protocol_breakpoints.ex @@ -8,9 +8,38 @@ defimpl Proto, for: [List, BitString] do end end +defprotocol DerivedProto do + def go(t) +end + +defimpl DerivedProto, for: Any do + defmacro __deriving__(module, _struct, _options) do + quote do + defimpl unquote(@protocol), for: unquote(module) do + def go(term) do + IO.puts("") + end + end + end + end + + def go(term) do + raise Protocol.UndefinedError, protocol: @protocol, value: term + end +end + +defmodule MyStruct do + @derive [{DerivedProto, []}] + defstruct [:a] +end + defmodule ProtocolBreakpoints do def go1() do Proto.go([]) Proto.go("") end + + def go2() do + DerivedProto.go(%MyStruct{}) + end end From 6bc501c288060fce88294b443f6b2fde493d503a Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Tue, 11 Jul 2023 06:16:41 +0200 Subject: [PATCH 7/8] format --- .../language_server/providers/completion.ex | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/apps/language_server/lib/language_server/providers/completion.ex b/apps/language_server/lib/language_server/providers/completion.ex index 269144a29..fbcb861eb 100644 --- a/apps/language_server/lib/language_server/providers/completion.ex +++ b/apps/language_server/lib/language_server/providers/completion.ex @@ -252,20 +252,25 @@ defmodule ElixirLS.LanguageServer.Providers.Completion do _options ) do name_only = String.trim_leading(name, "@") - insert_text = case String.split(prefix, "@") do - [_ | attribute_prefix] -> if String.starts_with?(name_only, attribute_prefix) do - name_only - else - name + + insert_text = + case String.split(prefix, "@") do + [_ | attribute_prefix] -> + if String.starts_with?(name_only, attribute_prefix) do + name_only + else + name + end + + _ -> + name end - _ -> name - end %__MODULE__{ label: name, kind: :variable, detail: "module attribute", - documentation: name <> "\n" <> (if summary, do: summary, else: ""), + documentation: name <> "\n" <> if(summary, do: summary, else: ""), insert_text: insert_text, filter_text: name_only, priority: 14, From 5c1182b73897dcde24a6d5e42295189b6625ee20 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Tue, 11 Jul 2023 18:14:39 +0200 Subject: [PATCH 8/8] Handle errors in setting breakpoints Disallow setting breakpoints in Inspect protocol implementations Disallow setting breakpoints in builtin protocols and JasonV.Encoder protocol Fixes https://github.com/elixir-lsp/elixir-ls/issues/900 Fixes https://github.com/elixir-lsp/elixir-ls/issues/903 Fixes https://github.com/elixir-lsp/elixir-ls/issues/942 --- .../elixir_ls_debugger/lib/debugger/server.ex | 72 ++++++++++++--- .../elixir_ls_debugger/test/debugger_test.exs | 92 +++++++++++++++++++ 2 files changed, 151 insertions(+), 13 deletions(-) diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex index 9611952be..66d7b5f2c 100644 --- a/apps/elixir_ls_debugger/lib/debugger/server.ex +++ b/apps/elixir_ls_debugger/lib/debugger/server.ex @@ -335,7 +335,7 @@ defmodule ElixirLS.Debugger.Server do result = case current[{m, f, a}] do nil -> - case :int.ni(m) do + case interpret(m, false) do {:module, _} -> ModuleInfoCache.store(m) breaks_before = :int.all_breaks(m) @@ -344,16 +344,21 @@ defmodule ElixirLS.Debugger.Server do "Setting function breakpoint in #{inspect(m)}.#{f}/#{a}" ) - :ok = :int.break_in(m, f, a) - breaks_after = :int.all_breaks(m) - lines = for {{^m, line}, _} <- breaks_after -- breaks_before, do: line + case :int.break_in(m, f, a) do + :ok -> + breaks_after = :int.all_breaks(m) + lines = for {{^m, line}, _} <- breaks_after -- breaks_before, do: line - # pass nil as log_message - not supported on function breakpoints as of DAP 1.51 - update_break_condition(m, lines, condition, nil, hit_count) + # pass nil as log_message - not supported on function breakpoints as of DAP 1.51 + update_break_condition(m, lines, condition, nil, hit_count) - {:ok, lines} + {:ok, lines} - _ -> + {:error, :function_not_found} -> + {:error, "Function #{inspect(m)}.#{f}/#{a} not found"} + end + + :error -> {:error, "Cannot interpret module #{inspect(m)}"} end @@ -1153,8 +1158,7 @@ defmodule ElixirLS.Debugger.Server do defp save_and_reload(module, beam_bin) do :ok = File.write(Path.join(@temp_beam_dir, to_string(module) <> ".beam"), beam_bin) true = :code.delete(module) - Output.debugger_console("Interpreting module #{inspect(module)}") - {:module, _} = :int.ni(module) + {:module, _} = interpret(module) ModuleInfoCache.store(module) end @@ -1193,6 +1197,9 @@ defmodule ElixirLS.Debugger.Server do set_breakpoint(modules_to_break, path, line) end end + rescue + error -> + for _line <- lines, do: {:error, Exception.format_exit(error)} end defp set_breakpoint([], path, {line, _}) do @@ -1202,10 +1209,11 @@ defmodule ElixirLS.Debugger.Server do defp set_breakpoint(modules, path, {line, {condition, log_message, hit_count}}) do modules_with_breakpoints = Enum.reduce(modules, [], fn module, added -> - case :int.ni(module) do + case interpret(module, false) do {:module, _} -> ModuleInfoCache.store(module) Output.debugger_console("Setting breakpoint in #{inspect(module)} #{path}:#{line}") + # no need to handle errors here, it can fail only with {:error, :break_exists} :int.break(module, line) update_break_condition(module, line, condition, log_message, hit_count) @@ -1237,8 +1245,7 @@ defmodule ElixirLS.Debugger.Server do defp interpret_module(mod) do try do - Output.debugger_console("Interpreting module #{inspect(mod)}") - {:module, _} = :int.ni(mod) + {:module, _} = interpret(mod) ModuleInfoCache.store(mod) catch _, _ -> @@ -1397,4 +1404,43 @@ defmodule ElixirLS.Debugger.Server do "step" end end + + @exclude_protocols_from_interpreting [ + Enumerable, + Collectable, + List.Chars, + String.Chars, + Inspect, + IEx.Info, + JasonV.Encoder + ] + + @exclude_implementations_from_interpreting [Inspect] + + defp interpret(module, print_message? \\ true) + + defp interpret(module, _print_message?) when module in @exclude_protocols_from_interpreting do + :error + end + + defp interpret(module, print_message?) do + if Code.ensure_loaded?(module) do + module_behaviours = + module.module_info(:attributes) |> Keyword.get_values(:behaviour) |> Enum.concat() + + if Enum.any?(@exclude_implementations_from_interpreting, &(&1 in module_behaviours)) do + # debugger uses Inspect protocol and setting breakpoints in implementations leads to deadlocks + # https://github.com/elixir-lsp/elixir-ls/issues/903 + :error + else + if print_message? do + Output.debugger_console("Interpreting module #{inspect(module)}") + end + + :int.ni(module) + end + else + :error + end + end end diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index 41c040b5b..6e5ed1908 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -891,6 +891,54 @@ defmodule ElixirLS.Debugger.ServerTest do end) end + @tag :fixture + test "handles invalid request", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet(server, initialize_req(1, %{})) + assert_receive(response(_, 1, "initialize", _)) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "test", + "projectDir" => File.cwd!(), + # disable auto interpret + "debugAutoInterpretAllModules" => false + }) + ) + + assert_receive(response(_, 2, "launch", _), 3000) + assert_receive(event(_, "initialized", %{}), 5000) + + refute :hello in :int.interpreted() + abs_path = Path.absname("src/hello.erl1") + + Server.receive_packet( + server, + set_breakpoints_req(3, %{"path" => abs_path}, [%{"line" => 5}]) + ) + + assert_receive( + response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => false}]}), + 3000 + ) + + abs_path = Path.absname("lib/mix_project.ex1") + + Server.receive_packet( + server, + set_breakpoints_req(4, %{"path" => abs_path}, [%{"line" => 3}]) + ) + + assert_receive( + response(_, 4, "setBreakpoints", %{"breakpoints" => [%{"verified" => false}]}), + 3000 + ) + end) + end + @tag :fixture test "sets and unsets breakpoints in elixir modules", %{server: server} do in_fixture(__DIR__, "mix_project", fn -> @@ -1671,6 +1719,50 @@ defmodule ElixirLS.Debugger.ServerTest do end) end + test "handles invalid requests", %{server: server} do + in_fixture(__DIR__, "mix_project", fn -> + Server.receive_packet(server, initialize_req(1, %{})) + assert_receive(response(_, 1, "initialize", _)) + + Server.receive_packet( + server, + launch_req(2, %{ + "request" => "launch", + "type" => "mix_task", + "task" => "test", + "projectDir" => File.cwd!(), + # disable auto interpret + "debugAutoInterpretAllModules" => false + }) + ) + + assert_receive(response(_, 2, "launch", _), 3000) + assert_receive(event(_, "initialized", %{}), 5000) + + refute :hello in :int.interpreted() + + Server.receive_packet( + server, + set_function_breakpoints_req(3, [%{"name" => ":hello1.hello_world/0"}]) + ) + + assert_receive( + response(_, 3, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => false}]}), + 3000 + ) + + Server.receive_packet( + server, + set_function_breakpoints_req(4, [%{"name" => ":hello.hello_world1/0"}]) + ) + + assert_receive( + response(_, 4, "setFunctionBreakpoints", %{"breakpoints" => [%{"verified" => false}]}), + 3000 + ) + end) + end + test "sets, modifies and unsets conditional function breakpoints", %{server: server} do in_fixture(__DIR__, "mix_project", fn -> Server.receive_packet(server, initialize_req(1, %{}))