Skip to content

Ability to turn off the pcall / xpcall wrappers #190

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

Closed
jugglerchris opened this issue Sep 20, 2020 · 9 comments
Closed

Ability to turn off the pcall / xpcall wrappers #190

jugglerchris opened this issue Sep 20, 2020 · 9 comments
Milestone

Comments

@jugglerchris
Copy link
Collaborator

As copied from #172 (comment)

rlua needs the ability to turn off the pcall / xpcall wrappers or any other provided function wrappers to be used in module position. They exist for a reason, but in module position it's very rude to muck with the global Lua state like this.

@yjhmelody
Copy link
Contributor

Exporting a api to append preload lua code is enough to achive it .

@SoniEx2
Copy link

SoniEx2 commented Dec 28, 2020

They exist to prevent catching panics which is just silly. Catching panics isn't marked as unsafe (as long as you respect UnwindSafe, anyway).

@SoniEx2
Copy link

SoniEx2 commented Dec 29, 2020

What would happen if we required UnwindSafe, anyway?

@jugglerchris
Copy link
Collaborator Author

I've not sure what the reason is for preventing Lua from catching Rust panics rather than just converting them into Lua errors which could be intercepted with pcall, though it's clearly intended and documented to do this - perhaps if @kyren has a moment she could give the reasoning?
@SoniEx2 Are you suggesting adding a UnwindSafe bound on Userdata types or something else?

@SoniEx2
Copy link

SoniEx2 commented Jan 9, 2021

on Function types. or well, Function closures? like... registering a Rust function as a Lua function should require the Rust function to be UnwindSafe.

@jugglerchris
Copy link
Collaborator Author

If I understand UnwindSafe correctly (which I may not!), it's not the function but any data it may access - which would include any data in the Lua state. But I'm not sure that's directly relevant to the question about Lua functions being able to catch panics - though my understanding is a bit fuzzy around this area at the moment.

@kyren
Copy link
Contributor

kyren commented Jan 19, 2021

the original reason for catching rust panics was the same reason for a lot of decisions in rlua, that it matched with what I was using it for at the time.

Catching rust panics is a serious thing to do, catching panics (with how I generally use them) should generally be reserved for only extreme circumstances like running cleanup operations or possibly at the very very top level of a rust program.

In Lua there is a mismatch, catching Lua errors is easy and it is natural to do it all the time, to the extent that it is actually easy to accidentally catch errors in Lua more than you might have intended.

The combination of this with Rust panics turning into errors means that scripts we were writing would accidentally catch rust panics, causing all kinds of madness, and that was something we absolutely never ever wanted. But, by turning rust panics into an un-catchable Lua error, we could get better backtraces through several layers of Lua scripts.

An alternate approach is to simply make panics across the Lua boundary into aborts, but I still don't like the idea of transparently turning panics into lua errors, lua errors are so lightweight that it feels wrong to turn a panic into one.

@SoniEx2
Copy link

SoniEx2 commented Jan 19, 2021

That is how UnwindSafe is meant to be used. It creates a marker to say, "hey, maybe you should be careful with this".

So if you see UnwindSafe, expect a panic to be potentially dropped. This is where Mutex poisoning comes in handy as even if it gets dropped it keeps panicking next time you try to use it, ideally driving the program to halt.

See https://doc.rust-lang.org/std/panic/trait.UnwindSafe.html

@jugglerchris
Copy link
Collaborator Author

I've added a flag in #234 which allows disabling the pcall/xpcall wrappers. When disabled, we instead abort if a panic comes out of a callback (I think this will become the default unwind behaviour when unwinding out of an extern "C" function anyway when the ffi-unwind work is done; this may be worth revisiting when extern "C-unwind" becomes available).

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

4 participants