Repository navigation
Conversation
…eak PYREFST. Python objects passed into Lua could stop being equal to themselves inside of Lua, or get deallocated while Lua still used them.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Found this while debugging something at my startup. We run long-running trading simulations that push Python objects into Lua, and every 100k pushes or so a stock's price came back as a different stock's. Turns out the slot numbers for
PYREFSTcome fromluaL_refon that same weak table, and this happens:py_to_lua_customwraps object X in userdata A, gets slotrfromluaL_refonPYREFST, and recordsrin_pyrefs_in_lua[refkey].PYREFST, but A's__gchas not run yet.luaL_refonly keeps numbers unique among entries still in the table, seesrempty, and gives it to the new wrapper B.__gcruns:luaL_unref(r)putsron the free list and overwrites B's slot, and_pyrefs_in_lua.pop(refkey)removes the entry, which for the same object is now B's._PyReferenceis gone, so X can be freed while B is still live in Lua.The patch takes the numbers from a strong
PYREFSLOTStable instead and deletes the_pyrefs_in_luaentry only ifpyref._ref == py_obj.ref.PYREFSLOTS[0] = 0on Lua < 5.4 keepsluaL_unrefinside__gcfrom allocating; teardown withmax_memoryexhausted crashed without it. The lookup path is unchanged.This fixes #293, but we did some before/after microbenchmarks and they were slightly worse: over 3M pushes, lua51 1.10s to 1.13s, luajit21 1.00s to 1.02s.
Note: The full suite passes on 3.15.0. Not tested on previous Python versions. Would like help here, or I can get around to it later.
Full disclosure: I used Claude Fable 5.1 to help dig into this. Feel free to close this if you guys have a no-AI policy. We are using this build in prod though.