-
Notifications
You must be signed in to change notification settings - Fork 110
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
Add panic guards to callbacks #412
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
ndk/src/utils.rs
Outdated
// Use the Rust logger if installed and enabled, otherwise fall back to the Android system | ||
// logger so there is at least some record of the panic | ||
let use_log = log_enabled!(Level::Error); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rib we might want to port this back to android-activity
.
03464a1
to
7b81320
Compare
2ac9c2d
to
66701ee
Compare
66701ee
to
de131a9
Compare
@spencercw not sure if worth/relevant here, but I learned about Not that it adds much though; it's basically a panic as if you were to For example, I already want to port this code to Final note: I think we forgot to wrap the callback in |
As with #412 we shouldn't let panics unwind into the FFI boundary; use the new helper `abort_on_panic()` utility to catch these and abort the process instead.
I do actually use I'm not super familiar with |
As with #412 we shouldn't let panics unwind into the FFI boundary; use the new helper `abort_on_panic()` utility to catch these and abort the process instead.
Exactly, if there's no clear point where the callback is going to be called this won't work. What's your thought about not getting a second backtrace from the point you called For EDIT: https://developer.android.com/ndk/reference/group/looper#alooper_addfd
Not sure if that means "any thread, as long as it's the thread that calls However, there are APIs like |
Not quite sure what you mean by this.
I presume this means you can submit file descriptors to a looper from any thread, but the polling will happen on the thread that the looper is actually running on.
This does seem like an ideal use case for |
Apologies for the late reply, it's been a while since I could find time.
Likely, but there's no wording excluding that. We could test to find out but no guarantees... I'll keep this in mind regardless as this API needs to be revisited anyway.
It is, hoping to submit the PR for this soon including mappings for |
As discussed in #410. Mostly copied from the equivalent code in android-activity. Resolves #268.