Skip to content

Commit f019515

Browse files
smnclaudedavydog187
authored
fix(string.rep): allocate proportionally to result, not repeat count (#376)
Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: Dave Lucia <davelucianyc@gmail.com>
1 parent 8b1625f commit f019515

3 files changed

Lines changed: 65 additions & 5 deletions

File tree

.github/workflows/ci.yml

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,9 @@ jobs:
2222
matrix:
2323
include:
2424
- elixir: "1.19.0"
25-
otp: "28.0"
25+
# 28.1+, not 28.0: Hex 2.5.0 (setup-beam's autoinstalled default)
26+
# calls :re.import/1, which only exists in OTP 28.1+.
27+
otp: "28.1"
2628
bootstrap_beam: false
2729
# OTP 29 RC's :httpc can't reach builds.hex.pm, so we tell setup-beam
2830
# to skip Hex/Rebar autoinstall and curl them ourselves below.
@@ -92,7 +94,9 @@ jobs:
9294
matrix:
9395
include:
9496
- elixir: "1.19.0"
95-
otp: "28.0"
97+
# 28.1+, not 28.0: Hex 2.5.0 (setup-beam's autoinstalled default)
98+
# calls :re.import/1, which only exists in OTP 28.1+.
99+
otp: "28.1"
96100
bootstrap_beam: false
97101

98102
steps:
@@ -126,7 +130,9 @@ jobs:
126130
matrix:
127131
include:
128132
- elixir: "1.19.0"
129-
otp: "28.0"
133+
# 28.1+, not 28.0: Hex 2.5.0 (setup-beam's autoinstalled default)
134+
# calls :re.import/1, which only exists in OTP 28.1+.
135+
otp: "28.1"
130136
bootstrap_beam: false
131137

132138
steps:
@@ -224,7 +230,9 @@ jobs:
224230
matrix:
225231
include:
226232
- elixir: "1.19.0"
227-
otp: "28.0"
233+
# 28.1+, not 28.0: Hex 2.5.0 (setup-beam's autoinstalled default)
234+
# calls :re.import/1, which only exists in OTP 28.1+.
235+
otp: "28.1"
228236
bootstrap_beam: false
229237

230238
steps:

lib/lua/vm/stdlib/string.ex

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,12 +217,23 @@ defmodule Lua.VM.Stdlib.String do
217217
# oversized request fails with a catchable error instead of trying to
218218
# allocate (and OOM the host on) a multi-petabyte binary.
219219
Limits.check_string_size!(n * byte_size(str) + (n - 1) * byte_size(sep), state.max_string_bytes)
220-
Enum.map_join(1..n, sep, fn _ -> str end)
220+
build_rep(str, n, sep)
221221
end
222222

223223
{[result], state}
224224
end
225225

226+
# Build the repeated string with allocation proportional to the *result*
227+
# size. The obvious `Enum.map_join(1..n, sep, ...)` materializes an
228+
# n-element list (plus its iolist) as transient garbage — for a large `n`
229+
# that is tens of times the size of the result itself, which spikes the
230+
# process heap and trips `max_heap_size` on sandboxed processes (and bloats
231+
# the reachable garbage observed through `Lua.call_function!/3`, since that
232+
# path returns before the next GC sweeps it). `:binary.copy/2` allocates the
233+
# backing binary exactly once, in a single pass.
234+
defp build_rep(str, n, ""), do: :binary.copy(str, n)
235+
defp build_rep(str, n, sep), do: :binary.copy(str <> sep, n - 1) <> str
236+
226237
# string.reverse(s) - reverses a string byte-by-byte (Lua strings are
227238
# byte arrays, not codepoint sequences — so a NUL or non-UTF-8 byte
228239
# must come back at the mirror position).

test/lua/vm/string_test.exs

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,47 @@ defmodule Lua.VM.StringTest do
141141
state = Stdlib.install(State.new())
142142
assert {:ok, [""], _state} = VM.execute(proto, state)
143143
end
144+
145+
# Building the repeated string must allocate proportionally to the result,
146+
# not to the repeat count. The previous `Enum.map_join(1..n, ...)`
147+
# implementation materialized an n-element list (tens of times the result
148+
# size) as transient garbage, spiking the heap and tripping `max_heap_size`
149+
# on sandboxed processes for an otherwise modest output. A 16 MB result
150+
# comfortably fits a 128 MB cap; the old code blew past 1 GB.
151+
test "string.rep allocates proportionally to the result, not the count" do
152+
size = 16 * 1024 * 1024
153+
154+
assert {:ok, result} =
155+
heap_capped(128 * 1024 * 1024, fn ->
156+
{[string], _lua} = Lua.eval!(Lua.new(), ~s[return string.rep("x", #{size})])
157+
byte_size(string)
158+
end)
159+
160+
assert result == size
161+
end
162+
end
163+
164+
# Runs `fun` in a process whose heap is capped at `byte_limit` (killed if it
165+
# exceeds), returning `{:ok, result}` or `:killed`. Lets a test assert that
166+
# work stays within a heap budget rather than sampling noisy global memory.
167+
defp heap_capped(byte_limit, fun) do
168+
parent = self()
169+
word_limit = div(byte_limit, :erlang.system_info(:wordsize))
170+
cap = %{size: word_limit, kill: true, error_logger: false}
171+
172+
{pid, ref} =
173+
:erlang.spawn_opt(
174+
fn -> send(parent, {:result, fun.()}) end,
175+
[:monitor, max_heap_size: cap]
176+
)
177+
178+
receive do
179+
{:result, result} -> {:ok, result}
180+
{:DOWN, ^ref, :process, ^pid, :killed} -> :killed
181+
{:DOWN, ^ref, :process, ^pid, reason} -> {:error, reason}
182+
after
183+
30_000 -> :timeout
184+
end
144185
end
145186

146187
describe "string.reverse" do

0 commit comments

Comments
 (0)