Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Large Lua string allocation/concatenations crashes Lua environment. #7

Open
tlubke opened this issue Jan 1, 2025 · 3 comments
Open

Comments

@tlubke
Copy link

tlubke commented Jan 1, 2025

Doing iterative string building like this can lead to issues.

local n = 9

for i=1,n do
  local s = ""
  for i=1,(2^i) do
    s = s.."!"
  end
end

For n = 9, this is fine, but n = 10 appears to hang something in the firmware. LEDs will hold their state, diii continues to show as connected, but all code execution seems to stop. Soft reboot via ^^r doesn't work, but physically reconnecting seems to fix things.

Interesting to note that building one constant string of size 2^10 does not cause the same behavior. Maybe the strings spawned by concatenation are not garbage collected fast enough and cause an out-of-memory failure because of it?

EDIT: Quick calc using the summation formula would mean n=9 => 131,238B and n=10 => 524,800B assuming nothing gets free'd before the end of the loop. n=10 would be more than available SRAM of the Pico 2.

EDIT 2: Yeah, I think garbage collection is the issue. Adding collectgarbage("collect") to the inner for-loop fixes the crash. I don't think this allocation/collection problem is realistically something to fix, but would be nice to keep the Lua environment running if possible.

@tehn
Copy link
Member

tehn commented Jan 3, 2025

thanks for taking a look.

i need to do some real benchmarking of the memory limits within this system. will see if i can put in some introspection utilities for better understanding what's going on.

@tehn
Copy link
Member

tehn commented Jan 8, 2025

heap has about 220k so i think your assessment makes sense!

i'll update the README to indicate the memory limitations. let me know if you'd like this issue reopened, if there is a proposal for a structural change.

@tehn tehn closed this as completed Jan 8, 2025
@catfact
Copy link

catfact commented Jan 10, 2025

i think this can be fixed, and is worth fixing. i would try using a custom allocator that errors gracefully rather than panicking...

@tehn tehn reopened this Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants