search

mozilla / ffi-support

A crate to help expose Rust functions over the FFI.
Apache License 2.0
40 stars 5 forks source link

Make the panic handling functionality public #6

Closed bendk closed 3 years ago

bendk commented 3 years ago

I think I will need this for uniffi#460. It seems reasonable that if ffi-support is going to set the global panic handling hook, then it should give other crates the opportunity to initialize it if they need to.

Updated the name to something that hopefully describes the intent a bit more.

bendk commented 3 years ago

@rfk what do you think about this one? If we're going to use this function in uniffi, then it would be nice to get this PR merged. OTOH, I think the only time this is called is from call_with_result() and call_with_output(), maybe copying the code into uniffi would work after all.

thomcc commented 3 years ago

Is it actually used? For a long time I remember the log_panics feature being disabled because it caused OOM on mobile.

(Specifically capturing a backtrace did on certain Android devices — I always suspected this was because of the weird behavior of libbionic's dlopen around shared libraries inside AAR (zip) files not being properly supported by the backtrace crate, but couldn't prove it)

rfk commented 3 years ago

I remember the log_panics feature being disabled because it caused OOM on mobile

You're right, I believe backtrace capture is currently disabled because of OOM issues which we haven't successfully gotten to the bottom of.

That said, I do agree with the general premise that it's useful for this to be public. @thomcc does making it public sit OK with you?

thomcc commented 3 years ago

Yep, makes complete sense.

rfk commented 3 years ago

@bendk in order to publish a new release here, we'll need to make you an owner on https://crates.io/crates/ffi-support. To allow that you'll first need to visit https://crates.io and sign in with your github account.