Skip to content

Commit

Permalink
cache module_info of interpreted modules
Browse files Browse the repository at this point in the history
call to module_info may deadlock when stopped on a breakpoint
Fixes #940
  • Loading branch information
lukaszsamson committed Jul 11, 2023
1 parent b2792a0 commit 0481fb8
Show file tree
Hide file tree
Showing 5 changed files with 159 additions and 6 deletions.
30 changes: 30 additions & 0 deletions apps/elixir_ls_debugger/lib/debugger/module_info_cache.ex
Original file line number Diff line number Diff line change
@@ -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
11 changes: 9 additions & 2 deletions apps/elixir_ls_debugger/lib/debugger/server.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ defmodule ElixirLS.Debugger.Server do
Variables,
Utils,
BreakpointCondition,
Binding
Binding,
ModuleInfoCache
}

alias ElixirLS.Debugger.Stacktrace.Frame
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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
Expand All @@ -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, [])
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
5 changes: 2 additions & 3 deletions apps/elixir_ls_debugger/lib/debugger/stacktrace.ex
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down Expand Up @@ -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
90 changes: 89 additions & 1 deletion apps/elixir_ls_debugger/test/debugger_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -22,6 +22,7 @@ defmodule ElixirLS.Debugger.ServerTest do
:int.no_break()
:int.clear()
BreakpointCondition.clear()
ModuleInfoCache.clear()
end)

{:ok, %{server: server}}
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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

0 comments on commit 0481fb8

Please sign in to comment.