Skip to content

Pass lua_State* to inline destructors as well#2454

Open
PhoenixWhitefire wants to merge 6 commits into
luau-lang:masterfrom
PhoenixWhitefire:inline-dtor-lstate
Open

Pass lua_State* to inline destructors as well#2454
PhoenixWhitefire wants to merge 6 commits into
luau-lang:masterfrom
PhoenixWhitefire:inline-dtor-lstate

Conversation

@PhoenixWhitefire

@PhoenixWhitefire PhoenixWhitefire commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

Currently, inline destructors set with lua_newuserdatadtor do not receive an L, while tag-based destructors do (#467).

This can be inconvenient if the lua_State* is needed during the destruction process, such as for a lua_getthreaddata, or to release a reference, where they are instead forced to store the L in their userdata object (e.g. @Bottersnike/eryx/src/modules/_fs_watch.cpp#L132). Embedders would be accepting the same risk that already exists with tag-based destructors.

This PR changes lua_newuserdatadtor to instead take a lua_Destructor, same as lua_setuserdatadtor, and passes the same L it would for tag-based destructors.

@vegorov-rbx

Copy link
Copy Markdown
Collaborator

We still think this is a risky direction to take, see the issue #1446 and discussion at #510

@vegorov-rbx vegorov-rbx reopened this Jul 9, 2026
@vegorov-rbx

vegorov-rbx commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator

We will probably be moving to take the path where we will try to api_check all invalid uses from inside GC (edit: in the future separately from this PR).
So with that and with some documentation about how bad it is too use 'L' that's provided, I think we can accept this.

@vegorov-rbx vegorov-rbx self-assigned this Jul 9, 2026
@PhoenixWhitefire

PhoenixWhitefire commented Jul 20, 2026

Copy link
Copy Markdown
Contributor Author

we will try to api_check all invalid uses from inside GC

Would this be like asserting the GC state in the barriers?

@vegorov-rbx

Copy link
Copy Markdown
Collaborator

Not asserting on a GC state, we will likely have a flag raised for the duration of the callback specifically.
It will be used for some new functionality coming in future weekly updates as well.

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

Successfully merging this pull request may close these issues.

2 participants