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

Adding using keyword and implement IDisposable pattern for lua_State #12

Open
tilkinsc opened this issue Dec 19, 2023 · 2 comments
Open
Labels

Comments

@tilkinsc
Copy link
Owner

tilkinsc commented Dec 19, 2023

Proposal

lua_State should follow the IDisposable pattern to use the using keyword.

Concerns

Undefined Behavior occurs when lua_close(L) closes a lua_State a second time and can result in application crashes.

Implementation

To mitigate the undefined behavior resulting from lua_close(L)ing a lua_State twice, we can set Handle to 0 in each Lua API's lua_close(L) after the call to native lua_close(L). Further, if lua_close(L) knows how to handle 0 we do not need to do anything further; otherwise we need to also check if the Handle is 0 before calling the native. This ensures that Handle == 0 to detect if the lua_State is closed or not when implementing the IDisposable pattern.

Benefits

Clean integration with C#'s using feature when quickly spinning up a lua_State to process something. Capturing the lua_State variable in a scope to avoid leaking lua_State's. Unfortunately, there's no way to exit from a using statement other than a goto so there is an additional scope; which isn't a problem depending on design.

using (lua_State L = luaL_newstate())
{
    if (L != 0)
    {
        // Do Lua stuff
    }
}
@tilkinsc tilkinsc pinned this issue Dec 20, 2023
@tilkinsc tilkinsc unpinned this issue Jan 24, 2024
@tilkinsc
Copy link
Owner Author

tilkinsc commented Jan 4, 2025

I think IDisposable pattern makes sense as we have pretty much an unsafe handle. Making a safe handle forces you into using IDisposable but optionally and people can use the,

using lua_State L = luaL_newstate();

pattern if an extra scope is an issue. I am not sure my original thought on the idea, as it revolved around it being almost required to be in a function or something, perhaps I was concerned about having two scopes. No idea anymore.

@ParadiseFallen
Copy link

it's better to keep things simple. If your project is just Dll bindings - then you dont need it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants