Make debugger conform to DAP 1.51 continue/stepin/stepout/stepover (#678)

* remove not supported SetExceptionBreakpoints request handler

Clients should only call this request if the capability ‘exceptionBreakpointFilters’ returns one or more filters.
No way to implement it via :int module

* implement function breakpoints

* test erlang breakpoints

* fix small memory leak when unsetting last breakpoint in file

* add more breakpoint tests

* make tests more synchronous

* add function breakpoints tests

* interpret modules

* extract and test mfa parsing

* run formatter

* Update readme

* cleanup

* extract and test erlang binding processing

fix some isses when var is usend more than 10 times
add explicite ordering by variable instance
discard underscored variables

* breakpoint conditions support added

do not warn when setting already set breakpoint

* update readme

* log when expression crashes

* add support for conditional function breakpoints

* update readme

* add support for breakpoint hit condition

* readme updated

* implement log message

* readme updated

* add support for terminateThreads request

* add support for pause

* make continue, next, stepIn, stepout requests conform to DAP 1.51

Fixes #669
reworks fixes for #455

* cleanup :int workaround

* tests wip

* format

Author: lukaszsamson

apps/elixir_ls_debugger/lib/debugger/server.ex

@@ -186,11 +186,12 @@ defmodule ElixirLS.Debugger.Server do
     )
 
     {thread_id, threads_inverse} = state.threads_inverse |> Map.pop(pid)
-    state = remove_paused_process(state, pid)
+    paused_processes = remove_paused_process(state, pid)
 
     state = %{
       state
       | threads: state.threads |> Map.delete(thread_id),
+        paused_processes: paused_processes,
         threads_inverse: threads_inverse
     }
 
@@ -565,76 +566,48 @@ defmodule ElixirLS.Debugger.Server do
     {json, state}
   end
 
-  defp handle_request(continue_req(_, thread_id), state = %__MODULE__{}) do
+  defp handle_request(continue_req(_, thread_id) = args, state = %__MODULE__{}) do
     pid = get_pid_by_thread_id!(state, thread_id)
 
-    try do
-      :int.continue(pid)
-      state = remove_paused_process(state, pid)
-      {%{"allThreadsContinued" => false}, state}
-    rescue
-      e in MatchError ->
-        raise ServerError,
-          message: "serverError",
-          format: ":int.continue failed: {message}",
-          variables: %{
-            "message" => inspect(Exception.message(e))
-          }
-    end
+    safe_int_action(pid, :continue)
+
+    paused_processes = remove_paused_process(state, pid)
+    paused_processes = maybe_continue_other_processes(args, paused_processes, pid)
+
+    processes_paused? = paused_processes |> Map.keys() |> Enum.any?(&is_pid/1)
+
+    {%{"allThreadsContinued" => not processes_paused?},
+     %{state | paused_processes: paused_processes}}
   end
 
-  defp handle_request(next_req(_, thread_id), state = %__MODULE__{}) do
+  defp handle_request(next_req(_, thread_id) = args, state = %__MODULE__{}) do
     pid = get_pid_by_thread_id!(state, thread_id)
 
-    try do
-      :int.next(pid)
-      state = remove_paused_process(state, pid)
-      {%{}, state}
-    rescue
-      e in MatchError ->
-        raise ServerError,
-          message: "serverError",
-          format: ":int.next failed: {message}",
-          variables: %{
-            "message" => inspect(Exception.message(e))
-          }
-    end
+    safe_int_action(pid, :next)
+    paused_processes = remove_paused_process(state, pid)
+
+    {%{},
+     %{state | paused_processes: maybe_continue_other_processes(args, paused_processes, pid)}}
   end
 
-  defp handle_request(step_in_req(_, thread_id), state = %__MODULE__{}) do
+  defp handle_request(step_in_req(_, thread_id) = args, state = %__MODULE__{}) do
     pid = get_pid_by_thread_id!(state, thread_id)
 
-    try do
-      :int.step(pid)
-      state = remove_paused_process(state, pid)
-      {%{}, state}
-    rescue
-      e in MatchError ->
-        raise ServerError,
-          message: "serverError",
-          format: ":int.stop failed: {message}",
-          variables: %{
-            "message" => inspect(Exception.message(e))
-          }
-    end
+    safe_int_action(pid, :step)
+    paused_processes = remove_paused_process(state, pid)
+
+    {%{},
+     %{state | paused_processes: maybe_continue_other_processes(args, paused_processes, pid)}}
   end
 
-  defp handle_request(step_out_req(_, thread_id), state = %__MODULE__{}) do
+  defp handle_request(step_out_req(_, thread_id) = args, state = %__MODULE__{}) do
     pid = get_pid_by_thread_id!(state, thread_id)
 
-    try do
-      :int.finish(pid)
-      state = remove_paused_process(state, pid)
-      {%{}, state}
-    rescue
-      e in MatchError ->
-        raise ServerError,
-          message: "serverError",
-          format: ":int.finish failed: {message}",
-          variables: %{
-            "message" => inspect(Exception.message(e))
-          }
-    end
+    safe_int_action(pid, :finish)
+    paused_processes = remove_paused_process(state, pid)
+
+    {%{},
+     %{state | paused_processes: maybe_continue_other_processes(args, paused_processes, pid)}}
   end
 
   defp handle_request(request(_, command), _state = %__MODULE__{}) when is_binary(command) do
@@ -646,6 +619,36 @@ defmodule ElixirLS.Debugger.Server do
       }
   end
 
+  defp maybe_continue_other_processes(%{"singleThread" => true}, paused_processes, requested_pid) do
+    resumed_pids =
+      for {paused_pid, %PausedProcess{ref: ref}} when paused_pid != requested_pid <-
+            paused_processes do
+        safe_int_action(paused_pid, :continue)
+        true = Process.demonitor(ref, [:flush])
+        paused_pid
+      end
+
+    paused_processes |> Map.drop(resumed_pids)
+  end
+
+  defp maybe_continue_other_processes(_, paused_processes, _requested_pid), do: paused_processes
+
+  # TODO consider removing this workaround as the problem seems to no longer affect OTP 24
+  defp safe_int_action(pid, action) do
+    apply(:int, action, [pid])
+    :ok
+  catch
+    kind, payload ->
+      # when stepping out of interpreted code a MatchError is risen inside :int module (at least in OTP 23)
+      IO.warn(":int.#{action}(#{inspect(pid)}) failed: #{Exception.format(kind, payload)}")
+
+      unless action == :continue do
+        safe_int_action(pid, :continue)
+      end
+
+      :ok
+  end
+
   defp get_pid_by_thread_id!(state = %__MODULE__{}, thread_id) do
     case state.threads[thread_id] do
       nil ->
@@ -668,7 +671,7 @@ defmodule ElixirLS.Debugger.Server do
       true = Process.demonitor(process.ref, [:flush])
     end
 
-    %__MODULE__{state | paused_processes: paused_processes}
+    paused_processes
   end
 
   defp variables(state = %__MODULE__{}, pid, var, start, count, filter) do
@@ -948,6 +951,7 @@ defmodule ElixirLS.Debugger.Server do
       "supportsValueFormattingOptions" => false,
       "supportsExceptionInfoRequest" => false,
       "supportsTerminateThreadsRequest" => true,
+      "supportsSingleThreadExecutionRequests" => true,
       "supportTerminateDebuggee" => false
     }
   end

apps/elixir_ls_debugger/test/debugger_test.exs

@@ -196,7 +196,7 @@ defmodule ElixirLS.Debugger.ServerTest do
                      1000
 
       Server.receive_packet(server, continue_req(10, thread_id))
-      assert_receive response(_, 10, "continue", %{"allThreadsContinued" => false})
+      assert_receive response(_, 10, "continue", %{"allThreadsContinued" => true})
     end)
   end
 
@@ -333,7 +333,7 @@ defmodule ElixirLS.Debugger.ServerTest do
                      )
 
       Server.receive_packet(server, continue_req(15, thread_id))
-      assert_receive response(_, 15, "continue", %{"allThreadsContinued" => false})
+      assert_receive response(_, 15, "continue", %{"allThreadsContinued" => true})
 
       Server.receive_packet(server, stacktrace_req(7, thread_id))
       thread_id_str = inspect(thread_id)