From 0e185345e174b17d87825019cc586132b33fbac4 Mon Sep 17 00:00:00 2001 From: Lukasz Samson Date: Sat, 9 Jan 2021 14:06:44 +0100 Subject: [PATCH] avoid debugger crashes when handling requests for no longer existing thread, frame and variable ids Fixes #452 --- apps/elixir_ls_debugger/lib/debugger/cli.ex | 1 + .../elixir_ls_debugger/lib/debugger/output.ex | 19 +- .../elixir_ls_debugger/lib/debugger/server.ex | 222 +++++++++++------- .../lib/debugger/stacktrace.ex | 2 +- .../elixir_ls_debugger/test/debugger_test.exs | 3 +- 5 files changed, 156 insertions(+), 91 deletions(-) diff --git a/apps/elixir_ls_debugger/lib/debugger/cli.ex b/apps/elixir_ls_debugger/lib/debugger/cli.ex index fce0527b0..201b6d672 100644 --- a/apps/elixir_ls_debugger/lib/debugger/cli.ex +++ b/apps/elixir_ls_debugger/lib/debugger/cli.ex @@ -6,6 +6,7 @@ defmodule ElixirLS.Debugger.CLI do WireProtocol.intercept_output(&Output.print/1, &Output.print_err/1) Launch.start_mix() {:ok, _} = Application.ensure_all_started(:elixir_ls_debugger, :permanent) + IO.puts("Started ElixirLS debugger v#{Launch.debugger_version()}") Launch.print_versions() Launch.limit_num_schedulers() diff --git a/apps/elixir_ls_debugger/lib/debugger/output.ex b/apps/elixir_ls_debugger/lib/debugger/output.ex index 3ad493b85..bd1481c37 100644 --- a/apps/elixir_ls_debugger/lib/debugger/output.ex +++ b/apps/elixir_ls_debugger/lib/debugger/output.ex @@ -51,16 +51,17 @@ defmodule ElixirLS.Debugger.Output do end def handle_call({:send_error_response, request_packet, message, format, variables}, _from, seq) do - res = WireProtocol.send( - error_response( - seq, - request_packet["seq"], - request_packet["command"], - message, - format, - variables + res = + WireProtocol.send( + error_response( + seq, + request_packet["seq"], + request_packet["command"], + message, + format, + variables + ) ) - ) {:reply, res, seq + 1} end diff --git a/apps/elixir_ls_debugger/lib/debugger/server.ex b/apps/elixir_ls_debugger/lib/debugger/server.ex index 46bcb213a..793648138 100644 --- a/apps/elixir_ls_debugger/lib/debugger/server.ex +++ b/apps/elixir_ls_debugger/lib/debugger/server.ex @@ -287,68 +287,89 @@ defmodule ElixirLS.Debugger.Server do defp handle_request(request(_, "stackTrace", %{"threadId" => thread_id} = args), state) do pid = state.threads[thread_id] - paused_process = state.paused_processes[pid] - total_frames = Enum.count(paused_process.stack) + case state.paused_processes[pid] do + %PausedProcess{} = paused_process -> + total_frames = Enum.count(paused_process.stack) - start_frame = - case args do - %{"startFrame" => start_frame} when is_integer(start_frame) -> start_frame - _ -> 0 - end + start_frame = + case args do + %{"startFrame" => start_frame} when is_integer(start_frame) -> start_frame + _ -> 0 + end - end_frame = - case args do - %{"levels" => levels} when is_integer(levels) and levels > 0 -> start_frame + levels - _ -> -1 - end + end_frame = + case args do + %{"levels" => levels} when is_integer(levels) and levels > 0 -> start_frame + levels + _ -> -1 + end - stack_frames = Enum.slice(paused_process.stack, start_frame..end_frame) - {state, frame_ids} = ensure_frame_ids(state, pid, stack_frames) - - stack_frames_json = - for {stack_frame, frame_id} <- List.zip([stack_frames, frame_ids]) do - %{ - "id" => frame_id, - "name" => Stacktrace.Frame.name(stack_frame), - "line" => stack_frame.line, - "column" => 0, - "source" => %{"path" => stack_frame.file} - } - end + stack_frames = Enum.slice(paused_process.stack, start_frame..end_frame) + {state, frame_ids} = ensure_frame_ids(state, pid, stack_frames) + + stack_frames_json = + for {%Frame{} = stack_frame, frame_id} <- List.zip([stack_frames, frame_ids]) do + %{ + "id" => frame_id, + "name" => Stacktrace.Frame.name(stack_frame), + "line" => stack_frame.line, + "column" => 0, + "source" => %{"path" => stack_frame.file} + } + end + + {%{"stackFrames" => stack_frames_json, "totalFrames" => total_frames}, state} - {%{"stackFrames" => stack_frames_json, "totalFrames" => total_frames}, state} + nil -> + IO.warn("unable to get stacktrace for thread_id #{thread_id}") + {%{"stackFrames" => [], "totalFrames" => 0}, state} + end end defp handle_request(request(_, "scopes", %{"frameId" => frame_id}), state) do - {pid, frame} = find_frame(state.paused_processes, frame_id) - - {state, args_id} = ensure_var_id(state, pid, frame.args) - {state, bindings_id} = ensure_var_id(state, pid, frame.bindings) - - vars_scope = %{ - "name" => "variables", - "variablesReference" => bindings_id, - "namedVariables" => Enum.count(frame.bindings), - "indexedVariables" => 0, - "expensive" => false - } - - args_scope = %{ - "name" => "arguments", - "variablesReference" => args_id, - "namedVariables" => 0, - "indexedVariables" => Enum.count(frame.args), - "expensive" => false - } + {state, scopes} = + case find_frame(state.paused_processes, frame_id) do + {pid, %Frame{} = frame} -> + {state, args_id} = ensure_var_id(state, pid, frame.args) + {state, bindings_id} = ensure_var_id(state, pid, frame.bindings) + + vars_scope = %{ + "name" => "variables", + "variablesReference" => bindings_id, + "namedVariables" => Enum.count(frame.bindings), + "indexedVariables" => 0, + "expensive" => false + } + + args_scope = %{ + "name" => "arguments", + "variablesReference" => args_id, + "namedVariables" => 0, + "indexedVariables" => Enum.count(frame.args), + "expensive" => false + } + + scopes = if Enum.count(frame.args) > 0, do: [vars_scope, args_scope], else: [vars_scope] + {state, scopes} + + nil -> + IO.warn("frameId #{inspect(frame_id)} not found") + {state, []} + end - scopes = if Enum.count(frame.args) > 0, do: [vars_scope, args_scope], else: [vars_scope] {%{"scopes" => scopes}, state} end defp handle_request(request(_, "variables", %{"variablesReference" => var_id} = args), state) do - {pid, var} = find_var(state.paused_processes, var_id) - {state, vars_json} = variables(state, pid, var, args["start"], args["count"], args["filter"]) + {state, vars_json} = + case find_var(state.paused_processes, var_id) do + {pid, var} -> + variables(state, pid, var, args["start"], args["count"], args["filter"]) + + nil -> + IO.warn("variablesReference #{inspect(var_id)} not found") + {state, []} + end {%{"variables" => vars_json}, state} end @@ -363,49 +384,80 @@ defmodule ElixirLS.Debugger.Server do end defp handle_request(continue_req(_, thread_id), state) do - pid = state.threads[thread_id] - state = remove_paused_process(state, pid) + state = + case state.threads[thread_id] do + nil -> + IO.warn("thread_id #{thread_id} not found") + state + + pid -> + :ok = :int.continue(pid) + remove_paused_process(state, pid) + end - :ok = :int.continue(pid) {%{"allThreadsContinued" => false}, state} end defp handle_request(next_req(_, thread_id), state) do - pid = state.threads[thread_id] - state = remove_paused_process(state, pid) + state = + case state.threads[thread_id] do + nil -> + IO.warn("thread_id #{thread_id} not found") + state + + pid -> + try do + :int.next(pid) + rescue + e in MatchError -> + IO.warn(":int.next failed, #{Exception.message(e)}") + end + + remove_paused_process(state, pid) + end - try do - :int.next(pid) - rescue - e in MatchError -> - IO.warn ":int.next failed, #{Exception.message(e)}" - end {%{}, state} end defp handle_request(step_in_req(_, thread_id), state) do - pid = state.threads[thread_id] - state = remove_paused_process(state, pid) + state = + case state.threads[thread_id] do + nil -> + IO.warn("thread_id #{thread_id} not found") + state + + pid -> + try do + :int.step(pid) + rescue + e in MatchError -> + IO.warn(":int.step failed, #{Exception.message(e)}") + end + + remove_paused_process(state, pid) + end - try do - :int.step(pid) - rescue - e in MatchError -> - IO.warn ":int.step failed, #{Exception.message(e)}" - end {%{}, state} end defp handle_request(step_out_req(_, thread_id), state) do - pid = state.threads[thread_id] - state = remove_paused_process(state, pid) + state = + case state.threads[thread_id] do + nil -> + IO.warn("thread_id #{thread_id} not found") + state + + pid -> + try do + :int.finish(pid) + rescue + e in MatchError -> + IO.warn(":int.finish failed, #{Exception.message(e)}") + end + + remove_paused_process(state, pid) + end - try do - :int.finish(pid) - rescue - e in MatchError -> - IO.warn ":int.finish failed, #{Exception.message(e)}" - end {%{}, state} end @@ -488,7 +540,9 @@ defmodule ElixirLS.Debugger.Server do defp all_variables(paused_processes) do paused_processes - |> Enum.flat_map(fn {_pid, paused_process} -> paused_process.frames |> Map.values() end) + |> Enum.flat_map(fn {_pid, %PausedProcess{} = paused_process} -> + paused_process.frames |> Map.values() + end) |> Enum.filter(&match?(%Frame{bindings: bindings} when is_map(bindings), &1)) |> Enum.flat_map(fn %Frame{bindings: bindings} -> bindings |> Enum.map(&rename_binding_to_classic_variable/1) @@ -508,7 +562,7 @@ defmodule ElixirLS.Debugger.Server do end defp find_var(paused_processes, var_id) do - Enum.find_value(paused_processes, fn {pid, paused_process} -> + Enum.find_value(paused_processes, fn {pid, %PausedProcess{} = paused_process} -> if Map.has_key?(paused_process.vars, var_id) do {pid, paused_process.vars[var_id]} end @@ -516,7 +570,7 @@ defmodule ElixirLS.Debugger.Server do end defp find_frame(paused_processes, frame_id) do - Enum.find_value(paused_processes, fn {pid, paused_process} -> + Enum.find_value(paused_processes, fn {pid, %PausedProcess{} = paused_process} -> if Map.has_key?(paused_process.frames, frame_id) do {pid, paused_process.frames[frame_id]} end @@ -543,6 +597,10 @@ defmodule ElixirLS.Debugger.Server do end defp ensure_var_id(state, pid, var) do + unless Map.has_key?(state.paused_processes, pid) do + raise ArgumentError, message: "paused process not found" + end + if Map.has_key?(state.paused_processes[pid].vars_inverse, var) do {state, state.paused_processes[pid].vars_inverse[var]} else @@ -561,7 +619,11 @@ defmodule ElixirLS.Debugger.Server do end) end - defp ensure_frame_id(state, pid, frame) do + defp ensure_frame_id(state, pid, %Frame{} = frame) do + unless Map.has_key?(state.paused_processes, pid) do + raise ArgumentError, message: "paused process not found" + end + if Map.has_key?(state.paused_processes[pid].frames_inverse, frame) do {state, state.paused_processes[pid].frames_inverse[frame]} else diff --git a/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex b/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex index 6c7a7d2f8..a5a887364 100644 --- a/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex +++ b/apps/elixir_ls_debugger/lib/debugger/stacktrace.ex @@ -6,7 +6,7 @@ defmodule ElixirLS.Debugger.Stacktrace do defmodule Frame do defstruct [:level, :file, :module, :function, :args, :line, :bindings] - def name(frame) do + def name(%__MODULE__{} = frame) do "#{inspect(frame.module)}.#{frame.function}/#{Enum.count(frame.args)}" end end diff --git a/apps/elixir_ls_debugger/test/debugger_test.exs b/apps/elixir_ls_debugger/test/debugger_test.exs index 43910aa3b..7c9f24e04 100644 --- a/apps/elixir_ls_debugger/test/debugger_test.exs +++ b/apps/elixir_ls_debugger/test/debugger_test.exs @@ -227,7 +227,8 @@ defmodule ElixirLS.Debugger.ServerTest do ) assert_receive( - response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}), 1000 + response(_, 3, "setBreakpoints", %{"breakpoints" => [%{"verified" => true}]}), + 1000 ) Server.receive_packet(server, request(4, "setExceptionBreakpoints", %{"filters" => []}))