Fix unbounded recursion on cyclic tables at the eval boundary

lib/lua/vm/display.ex

@@ -87,37 +87,49 @@ defmodule Lua.VM.Display do
   Wraps a single eval-result value for display.
 
   See `wrap_results/3` for the decode-mode matrix.
+
+  Tables that (transitively) contain themselves get a `:circular`
+  peek at the point of recurrence rather than recursing forever —
+  see `Lua.VM.Display.Table`.
   """
   @spec wrap_value(term(), State.t(), boolean()) :: term()
-  def wrap_value(value, state, decode?)
+  def wrap_value(value, state, decode?), do: wrap_value(value, state, decode?, %{})
 
   # decode: true — only wrap closures/native; tables and userdata
   # have already been decoded and are passed through unchanged.
-  def wrap_value({:lua_closure, _, _} = ref, _state, _decode?) do
+  defp wrap_value({:lua_closure, _, _} = ref, _state, _decode?, _ancestors) do
     wrap_closure(ref)
   end
 
-  def wrap_value({:compiled_closure, _, _} = ref, _state, _decode?) do
+  defp wrap_value({:compiled_closure, _, _} = ref, _state, _decode?, _ancestors) do
     wrap_closure(ref)
   end
 
-  def wrap_value({:native_func, fun} = ref, _state, _decode?) do
+  defp wrap_value({:native_func, fun} = ref, _state, _decode?, _ancestors) do
     %NativeFunc{fun: fun, ref: ref}
   end
 
   # decode: false — wrap tref/udref too, and recurse into table peek.
-  def wrap_value({:tref, id} = ref, state, false) do
-    peek = peek_table(state, id, false)
+  # `ancestors` holds the tref ids currently being peeked higher up
+  # this walk; revisiting one means the table contains itself.
+  defp wrap_value({:tref, id} = ref, state, false, ancestors) do
+    peek =
+      if Map.has_key?(ancestors, id) do
+        :circular
+      else
+        peek_table(state, id, false, Map.put(ancestors, id, true))
+      end
+
     %DTable{id: id, peek: peek, ref: ref}
   end
 
-  def wrap_value({:udref, id} = ref, state, false) do
+  defp wrap_value({:udref, id} = ref, state, false, _ancestors) do
     term = State.get_userdata(state, ref)
     %Userdata{id: id, term: term, ref: ref}
   end
 
   # decode: true catch-all (already-decoded values pass through)
-  def wrap_value(value, _state, _decode?), do: value
+  defp wrap_value(value, _state, _decode?, _ancestors), do: value
 
   # ---- internal helpers ----
 
@@ -138,15 +150,18 @@ defmodule Lua.VM.Display do
   # (1..N keys) render as a list; mixed-key tables render as a map.
   # Nested tables/closures are recursively wrapped so `Inspect` does
   # not have to know about live VM state.
-  defp peek_table(state, id, decode?) do
+  defp peek_table(state, id, decode?, ancestors) do
     case Map.fetch(state.tables, id) do
       {:ok, table} ->
         data = Lua.VM.Table.to_map(table)
 
         if sequence_like?(data) do
-          Enum.map(1..map_size(data), &wrap_value(Map.fetch!(data, &1), state, decode?))
+          Enum.map(
+            1..map_size(data),
+            &wrap_value(Map.fetch!(data, &1), state, decode?, ancestors)
+          )
         else
-          Map.new(data, fn {k, v} -> {k, wrap_value(v, state, decode?)} end)
+          Map.new(data, fn {k, v} -> {k, wrap_value(v, state, decode?, ancestors)} end)
         end
 
       :error ->

lib/lua/vm/display/table.ex

@@ -15,7 +15,10 @@ defmodule Lua.VM.Display.Table do
   - `:peek` — a snapshot of the table's data as it was at the time
     the eval boundary was crossed, suitable for human display. May
     be a list (sequence-like tables) or a map (mixed-key tables).
-    Truncated to `Inspect.Opts.limit` entries when rendered.
+    Truncated to `Inspect.Opts.limit` entries when rendered. When a
+    table (transitively) contains itself — e.g. the `T.__index = T`
+    OOP idiom — the recurring occurrence carries `:circular` instead
+    of a snapshot, bounding an otherwise infinite walk.
   - `:ref` — the original `{:tref, id}` tuple so callers can
     round-trip the value back into the VM (via `Lua.set!/3`,
     `Lua.encode!/2`, etc.).
@@ -25,7 +28,7 @@ defmodule Lua.VM.Display.Table do
 
   @type t :: %__MODULE__{
           id: non_neg_integer(),
-          peek: list() | map(),
+          peek: list() | map() | :circular,
           ref: tuple()
         }
 
@@ -34,6 +37,14 @@ defmodule Lua.VM.Display.Table do
   defimpl Inspect do
     import Inspect.Algebra
 
+    def inspect(%Lua.VM.Display.Table{id: id, peek: :circular}, _opts) do
+      concat([
+        "#Lua.Table<id: ",
+        Integer.to_string(id),
+        ", circular>"
+      ])
+    end
+
     def inspect(%Lua.VM.Display.Table{id: id, peek: peek}, opts) do
       concat([
         "#Lua.Table<id: ",

lib/lua/vm/value.ex

@@ -323,25 +323,39 @@ defmodule Lua.VM.Value do
 
   Tables are returned as lists of `{key, decoded_value}` tuples.
   Functions (closures, native) pass through as-is.
+
+  Cyclic tables (e.g. the common `T.__index = T` idiom) cannot be
+  represented as acyclic Elixir data, so the walk terminates at the
+  point of recurrence and leaves the table's `{:tref, id}` reference
+  there — mirroring how functions already pass through as opaque
+  references. Shared references that do not form a cycle decode
+  normally.
   """
   @spec decode(term(), State.t()) :: term()
-  def decode(nil, _state), do: nil
-  def decode(value, _state) when is_boolean(value), do: value
-  def decode(value, _state) when is_number(value), do: value
-  def decode(value, _state) when is_binary(value), do: value
+  def decode(value, state), do: decode(value, state, %{})
+
+  defp decode(nil, _state, _ancestors), do: nil
+  defp decode(value, _state, _ancestors) when is_boolean(value), do: value
+  defp decode(value, _state, _ancestors) when is_number(value), do: value
+  defp decode(value, _state, _ancestors) when is_binary(value), do: value
 
-  def decode({:udref, _} = ref, state) do
+  defp decode({:udref, _} = ref, state, _ancestors) do
     value = State.get_userdata(state, ref)
     {:userdata, value}
   end
 
-  def decode({:tref, id}, state) do
-    table = Map.fetch!(state.tables, id)
+  defp decode({:tref, id} = ref, state, ancestors) do
+    if Map.has_key?(ancestors, id) do
+      ref
+    else
+      table = Map.fetch!(state.tables, id)
+      ancestors = Map.put(ancestors, id, true)
 
-    Enum.map(Lua.VM.Table.to_map(table), fn {k, v} -> {k, decode(v, state)} end)
+      Enum.map(Lua.VM.Table.to_map(table), fn {k, v} -> {k, decode(v, state, ancestors)} end)
+    end
   end
 
-  def decode(value, _state), do: value
+  defp decode(value, _state, _ancestors), do: value
 
   @doc """
   Decodes a list of Lua VM values.

test/lua/vm/cyclic_table_test.exs

@@ -0,0 +1,97 @@
+defmodule Lua.VM.CyclicTableTest do
+  @moduledoc """
+  Cyclic tables crossing the eval boundary must terminate.
+
+  The common Lua OOP idiom `T.__index = T` creates a table that
+  contains itself. Both boundary walks — `decode: true` (Value.decode)
+  and `decode: false` (Display peek) — previously recursed forever on
+  such values, growing memory without bound until the VM's
+  `max_heap_size` (when set) killed the process.
+  """
+
+  use ExUnit.Case, async: true
+
+  alias Lua.VM.Display.Table, as: DTable
+
+  @self_cycle """
+  local T = {}
+  T.__index = T
+  return T
+  """
+
+  @mutual_cycle """
+  local a = {}
+  local b = {a = a}
+  a.b = b
+  return a
+  """
+
+  describe "decode: false (Display peek)" do
+    test "self-referential table peeks as :circular at the recurrence" do
+      {[t], _} = Lua.eval!(Lua.new(), @self_cycle, decode: false)
+
+      assert %DTable{id: id, peek: %{"__index" => inner}} = t
+      assert %DTable{id: ^id, peek: :circular} = inner
+      assert inspect(inner) == "#Lua.Table<id: #{id}, circular>"
+    end
+
+    test "mutually recursive tables terminate and render" do
+      {[t], _} = Lua.eval!(Lua.new(), @mutual_cycle, decode: false)
+
+      assert %DTable{id: a_id, peek: %{"b" => %DTable{peek: %{"a" => inner_a}}}} = t
+      assert %DTable{id: ^a_id, peek: :circular} = inner_a
+      assert inspect(t) =~ "circular"
+    end
+
+    test "shared non-cyclic references still peek fully" do
+      code = """
+      local shared = {x = 1}
+      return {a = shared, b = shared}
+      """
+
+      {[t], _} = Lua.eval!(Lua.new(), code, decode: false)
+
+      assert %DTable{
+               peek: %{"a" => %DTable{peek: %{"x" => 1}}, "b" => %DTable{peek: %{"x" => 1}}}
+             } =
+               t
+    end
+  end
+
+  describe "decode: true (Value.decode)" do
+    test "self-referential table terminates with the table's reference at the recurrence" do
+      {[decoded], _} = Lua.eval!(Lua.new(), @self_cycle)
+
+      assert [{"__index", {:tref, id}}] = decoded
+      assert is_integer(id)
+    end
+
+    test "mutually recursive tables terminate with a reference" do
+      {[decoded], _} = Lua.eval!(Lua.new(), @mutual_cycle)
+
+      assert [{"b", [{"a", {:tref, _}}]}] = decoded
+    end
+
+    test "shared non-cyclic references decode normally" do
+      code = """
+      local shared = {x = 1}
+      return {a = shared, b = shared}
+      """
+
+      {[decoded], _} = Lua.eval!(Lua.new(), code)
+
+      assert Enum.sort(decoded) == [{"a", [{"x", 1}]}, {"b", [{"x", 1}]}]
+    end
+
+    test "a table appearing under multiple sibling keys is not a false-positive cycle" do
+      code = """
+      local leaf = {v = 1}
+      local mid = {l = leaf, r = leaf}
+      return {left = mid, right = mid}
+      """
+
+      {[decoded], _} = Lua.eval!(Lua.new(), code)
+      assert is_list(decoded)
+    end
+  end
+end