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

luv_cfpcall: Fix stack balancing after an uncaught error #736

Merged
merged 1 commit into from
Nov 15, 2024

Conversation

mwild1
Copy link
Contributor

@mwild1 mwild1 commented Nov 14, 2024

The default error handling code for printing uncaught errors uses luaL_tolstring(). This returns the resulting string, but also pushes it onto the stack. After we have printed it, it is safe to pop from the stack.

Leaving it on the stack causes the stack to be unbalanced.

Fixes #735 (where a test case can be found)

@mwild1 mwild1 force-pushed the fix/balance-stack-on-uncaught-error branch from bbc627a to 219c88d Compare November 14, 2024 16:16
@mwild1
Copy link
Contributor Author

mwild1 commented Nov 14, 2024

I just force-pushed an updated commit, as I found a second place where luaL_tolstring() is used but the result is not removed.

@zhaozg
Copy link
Member

zhaozg commented Nov 15, 2024

Please add a test to cover this PR.

@mwild1
Copy link
Contributor Author

mwild1 commented Nov 15, 2024

I don't know how - do you have any suggestions? Unbalanced stack cannot be detected from Lua.

@zhaozg
Copy link
Member

zhaozg commented Nov 15, 2024

I don't know how - do you have any suggestions? Unbalanced stack cannot be detected from Lua.

That's true, ignore it.

The default error handling code for printing uncaught errors uses
luaL_tolstring(). This returns the resulting string, but also pushes it onto
the stack. After we have printed it, it is safe to pop from the stack.

Leaving it on the stack causes the stack to be unbalanced.

Fixes luvit#735 (where a test case can be found)
@mwild1 mwild1 force-pushed the fix/balance-stack-on-uncaught-error branch from 219c88d to 09a98a4 Compare November 15, 2024 14:16
@mwild1
Copy link
Contributor Author

mwild1 commented Nov 15, 2024

https://github.com/luvit/luv/pull/736/files#diff-d153acd02208bccdfe1c42a92c8464b423661b480f683720acff42c102ea64bbR718-L720

should do lua_pop too.

Sorry, thought I pushed a fix for that yesterday. Just updated the branch now.

src/luv.c Show resolved Hide resolved
@zhaozg zhaozg enabled auto-merge November 15, 2024 14:20
@zhaozg zhaozg merged commit 353df88 into luvit:master Nov 15, 2024
14 checks passed
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.

Unbalanced stack after uncaught errors in work callback
2 participants