Skip to content

Commit aeb33ae

Browse files
committed
fix(lisp): prevent closure and HOF state leaks
Stop closures from retaining turn history and resolve *1/*2/*3 from the active caller context. Clean up the higher-order-function side-effect stash on error paths, and avoid registry atom load-order failures for static builtin names. Verified with mix test.
1 parent 50ac559 commit aeb33ae

5 files changed

Lines changed: 145 additions & 23 deletions

File tree

lib/ptc_runner/lisp/eval.ex

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -402,14 +402,14 @@ defmodule PtcRunner.Lisp.Eval do
402402
# would inflate session memory by ~18 KB per closure.
403403
{captured_env, captured_locals} = capture_lexical_scope(eval_ctx)
404404
meta = locals_meta(captured_locals, %{})
405-
{:ok, {:closure, params, body, captured_env, eval_ctx.turn_history, meta}, eval_ctx}
405+
{:ok, {:closure, params, body, captured_env, [], meta}, eval_ctx}
406406
end
407407

408408
# Named fn: (fn name [params] body) — name is bound inside body for self-recursion
409409
defp do_eval({:fn, name, params, body}, %EvalContext{} = eval_ctx) do
410410
{captured_env, captured_locals} = capture_lexical_scope(eval_ctx)
411411
meta = locals_meta(captured_locals, %{fn_name: name})
412-
{:ok, {:closure, params, body, captured_env, eval_ctx.turn_history, meta}, eval_ctx}
412+
{:ok, {:closure, params, body, captured_env, [], meta}, eval_ctx}
413413
end
414414

415415
# ============================================================
@@ -1235,7 +1235,7 @@ defmodule PtcRunner.Lisp.Eval do
12351235

12361236
# Convert a closure to a zero-arity Erlang function for use in pcalls
12371237
defp pcalls_fn_to_erlang(
1238-
{:closure, [], body, closure_env, turn_history, metadata} = closure,
1238+
{:closure, [], body, closure_env, _turn_history, metadata} = closure,
12391239
%EvalContext{} = eval_ctx
12401240
) do
12411241
closure_env = Apply.bind_self_recursion(closure_env, metadata, closure)
@@ -1247,7 +1247,7 @@ defmodule PtcRunner.Lisp.Eval do
12471247
eval_ctx.user_ns,
12481248
closure_env,
12491249
eval_ctx.tool_exec,
1250-
turn_history
1250+
eval_ctx.turn_history
12511251
)
12521252

12531253
ctx = %{

lib/ptc_runner/lisp/eval/apply.ex

Lines changed: 34 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -127,9 +127,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
127127
converted_args = Enum.map(args, fn arg -> closure_to_fun(arg, eval_ctx, do_eval_fn) end)
128128

129129
try do
130-
push_side_effect_stash()
131-
result = apply(fun, converted_args)
132-
{:ok, result, pop_side_effects(eval_ctx)}
130+
with_side_effect_stash(eval_ctx, fn -> apply(fun, converted_args) end)
133131
rescue
134132
FunctionClauseError ->
135133
# Provide a helpful error message for type mismatches
@@ -261,12 +259,10 @@ defmodule PtcRunner.Lisp.Eval.Apply do
261259
when is_function(fun, 1) do
262260
# Convert any closures/builtins in args to callable functions
263261
converted_args = Enum.map(args, fn arg -> closure_to_fun(arg, eval_ctx, do_eval_fn) end)
264-
push_side_effect_stash()
265-
result = fun.(converted_args)
266-
{:ok, result, pop_side_effects(eval_ctx)}
262+
263+
with_side_effect_stash(eval_ctx, fn -> fun.(converted_args) end)
267264
rescue
268265
e in ExecutionError ->
269-
pop_side_effects(eval_ctx)
270266
{:error, execution_error_tuple(e)}
271267
end
272268

@@ -286,9 +282,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
286282
fun = elem(funs, idx)
287283

288284
try do
289-
push_side_effect_stash()
290-
result = apply(fun, converted_args)
291-
{:ok, result, pop_side_effects(eval_ctx)}
285+
with_side_effect_stash(eval_ctx, fn -> apply(fun, converted_args) end)
292286
rescue
293287
FunctionClauseError ->
294288
# Provide a helpful error message for type mismatches
@@ -308,9 +302,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
308302

309303
defp do_apply_fun(fun, args, %EvalContext{} = eval_ctx, _do_eval_fn)
310304
when is_function(fun) do
311-
push_side_effect_stash()
312-
result = apply(fun, args)
313-
{:ok, result, pop_side_effects(eval_ctx)}
305+
with_side_effect_stash(eval_ctx, fn -> apply(fun, args) end)
314306
end
315307

316308
# Map as function: (map key) → Map.get(map, key)
@@ -598,7 +590,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
598590
body,
599591
closure_env,
600592
%EvalContext{} = eval_context,
601-
closure_turn_history,
593+
_closure_turn_history,
602594
metadata,
603595
do_eval_fn
604596
) do
@@ -628,7 +620,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
628620
eval_context.user_ns,
629621
new_env,
630622
eval_context.tool_exec,
631-
closure_turn_history,
623+
eval_context.turn_history,
632624
pmap_timeout: eval_context.pmap_timeout,
633625
catalog_exec: eval_context.catalog_exec
634626
)
@@ -656,11 +648,35 @@ defmodule PtcRunner.Lisp.Eval.Apply do
656648
# Uses a stack to handle nested HOFs: each HOF pushes a fresh accumulator,
657649
# inner HOFs push/pop their own level, and the outer HOF collects everything.
658650

651+
defp with_side_effect_stash(%EvalContext{} = eval_ctx, fun) when is_function(fun, 0) do
652+
push_side_effect_stash()
653+
654+
try do
655+
result = fun.()
656+
{:ok, result, pop_side_effects(eval_ctx)}
657+
rescue
658+
e ->
659+
drop_side_effect_stash()
660+
reraise e, __STACKTRACE__
661+
catch
662+
kind, reason ->
663+
drop_side_effect_stash()
664+
:erlang.raise(kind, reason, __STACKTRACE__)
665+
end
666+
end
667+
659668
defp push_side_effect_stash do
660669
stack = Process.get(:__ptc_hof_stack, [])
661670
Process.put(:__ptc_hof_stack, [%{tool_calls: [], prints: [], catalog_ops: []} | stack])
662671
end
663672

673+
defp drop_side_effect_stash do
674+
case Process.get(:__ptc_hof_stack, []) do
675+
[_top | rest] -> Process.put(:__ptc_hof_stack, rest)
676+
[] -> :ok
677+
end
678+
end
679+
664680
defp stash_side_effects(%EvalContext{} = ctx) do
665681
case Process.get(:__ptc_hof_stack, []) do
666682
[top | rest] ->
@@ -737,7 +753,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
737753
end
738754

739755
defp do_execute_closure(
740-
{:closure, _closure_patterns, body, closure_env, closure_turn_history, meta} = closure,
756+
{:closure, _closure_patterns, body, closure_env, _closure_turn_history, meta} = closure,
741757
binding_patterns,
742758
args,
743759
%EvalContext{ctx: ctx, user_ns: user_ns, tool_exec: tool_exec} = caller_ctx,
@@ -754,7 +770,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
754770
_ -> new_env
755771
end
756772

757-
closure_ctx = EvalContext.new(ctx, user_ns, new_env, tool_exec, closure_turn_history)
773+
closure_ctx = EvalContext.new(ctx, user_ns, new_env, tool_exec, caller_ctx.turn_history)
758774

759775
# Carry accumulated state from caller so tool_calls/cache aren't lost across closure calls.
760776
# `locals` is rebuilt from the closure's lexical capture + this invocation's
@@ -844,7 +860,7 @@ defmodule PtcRunner.Lisp.Eval.Apply do
844860
Enum.reduce(user_ns, user_ns, fn
845861
{name, {:closure, ^params, ^body, ^env, ^th, old_meta}}, acc ->
846862
new_meta = Map.put(old_meta, :return_type, return_type)
847-
Map.put(acc, name, {:closure, params, body, env, th, new_meta})
863+
Map.put(acc, name, {:closure, params, body, env, [], new_meta})
848864

849865
_, acc ->
850866
acc

lib/ptc_runner/lisp/registry.ex

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,9 @@ defmodule PtcRunner.Lisp.Registry do
123123
def builtins_by_category(category) do
124124
implemented()
125125
|> Enum.filter(&(&1.category == category and &1.dispatch == :env))
126-
|> Enum.map(&String.to_existing_atom(&1.name))
126+
# Names come from the closed compile-time registry, not user input.
127+
# Using to_atom/1 avoids depending on Env being loaded first.
128+
|> Enum.map(&String.to_atom(&1.name))
127129
end
128130

129131
@doc """

test/ptc_runner/lisp/closure_capture_test.exs

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -195,5 +195,43 @@ defmodule PtcRunner.Lisp.ClosureCaptureTest do
195195
assert memory_size < 2_000,
196196
"expected defn memory < 2000 bytes, got #{memory_size}"
197197
end
198+
199+
test "closures do not retain prior turn history in session memory" do
200+
large_history_entry = String.duplicate("x", 100_000)
201+
202+
{:ok, step} =
203+
Lisp.run("(def f (fn [] 1))",
204+
profile: :mcp_no_tools,
205+
mode: :multi_turn,
206+
turn_history: [large_history_entry]
207+
)
208+
209+
memory_size = :erlang.external_size(step.memory)
210+
{:closure, _params, _body, _env, captured_history, _meta} = step.memory[:f]
211+
212+
assert captured_history == []
213+
214+
assert memory_size < 2_000,
215+
"expected closure memory to exclude turn history, got #{memory_size} bytes"
216+
end
217+
218+
test "closure reads current caller turn history, not definition-time history" do
219+
{:ok, step1} =
220+
Lisp.run("(def f (fn [] *1))",
221+
profile: :mcp_no_tools,
222+
mode: :multi_turn,
223+
turn_history: ["definition-time"]
224+
)
225+
226+
{:ok, step2} =
227+
Lisp.run("(f)",
228+
profile: :mcp_no_tools,
229+
mode: :multi_turn,
230+
memory: step1.memory,
231+
turn_history: ["call-time"]
232+
)
233+
234+
assert step2.return == "call-time"
235+
end
198236
end
199237
end

test/ptc_runner/lisp/hof_side_effects_test.exs

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,16 @@ defmodule PtcRunner.Lisp.HofSideEffectsTest do
1111
use ExUnit.Case, async: true
1212

1313
alias PtcRunner.Lisp
14+
alias PtcRunner.Lisp.{Env, Eval}
15+
16+
defp clear_hof_stack! do
17+
Process.delete(:__ptc_hof_stack)
18+
on_exit(fn -> Process.delete(:__ptc_hof_stack) end)
19+
end
20+
21+
defp assert_hof_stack_empty do
22+
assert Process.get(:__ptc_hof_stack, []) == []
23+
end
1424

1525
describe "tool_calls inside reduce" do
1626
test "tool calls made inside reduce are captured in step.tool_calls" do
@@ -140,4 +150,60 @@ defmodule PtcRunner.Lisp.HofSideEffectsTest do
140150
assert step.prints == ["storing 1", "storing 2", "storing 3"]
141151
end
142152
end
153+
154+
describe "side-effect stash cleanup on errors" do
155+
test "multi-arity HOF callback errors do not retain process dictionary state" do
156+
clear_hof_stack!()
157+
158+
ast =
159+
{:call, {:var, :map},
160+
[
161+
{:fn, [{:var, :x}], {:call, {:var, :+}, [{:var, :x}, nil]}},
162+
{:vector, [1, 2, 3]}
163+
]}
164+
165+
assert {:error, {:type_error, _message, _args}} =
166+
Eval.eval(ast, %{}, %{}, Env.initial(), fn _, _ -> nil end)
167+
168+
assert_hof_stack_empty()
169+
end
170+
171+
test "normal HOF callback errors do not retain process dictionary state" do
172+
clear_hof_stack!()
173+
174+
ast =
175+
{:call, {:var, :filter},
176+
[
177+
{:fn, [{:var, :x}], {:call, {:var, :+}, [{:var, :x}, nil]}},
178+
{:vector, [1, 2, 3]}
179+
]}
180+
181+
assert {:error, {:type_error, _message, _args}} =
182+
Eval.eval(ast, %{}, %{}, Env.initial(), fn _, _ -> nil end)
183+
184+
assert_hof_stack_empty()
185+
end
186+
187+
test "collect builtin errors do not retain process dictionary state" do
188+
clear_hof_stack!()
189+
190+
assert_raise ArgumentError, ~r/every-pred requires at least 1 predicate/, fn ->
191+
Eval.eval({:call, {:var, :"every-pred"}, []}, %{}, %{}, Env.initial(), fn _, _ -> nil end)
192+
end
193+
194+
assert_hof_stack_empty()
195+
end
196+
197+
test "plain function errors do not retain process dictionary state" do
198+
clear_hof_stack!()
199+
200+
bad = fn _ -> raise "boom" end
201+
202+
assert_raise RuntimeError, "boom", fn ->
203+
Eval.eval({:call, {:var, :bad}, [1]}, %{}, %{}, %{bad: bad}, fn _, _ -> nil end)
204+
end
205+
206+
assert_hof_stack_empty()
207+
end
208+
end
143209
end

0 commit comments

Comments
 (0)