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
x/mobile/bind/java: translate panic to a java error #11382
Comments
where is the panic from? |
The scenario I was testing was a Go function called from Java. Would be nice not to have to wrap all my external functions with panic/recover and have the runtime just support this, but maybe I'm asking too much. To be a bit clearer, I'm not expecting any of my Go code to panic. But if I'm accessing an array out of bounds and the app is crashing for half my users, I want to have reporting that tells me that. |
one option David Crawshaw suggested is to make gobind generate a code that recover and translate the panic to java exception. |
A Go panic has a lot of technical similarities to Java exceptions, but several conventional differences. Java exceptions are part of normal programming, Go panics are not. We may be able to match the conventions by throwing a java.lang.Error. While throwable, it's not a subclass of java.lang.Exception, so the usual (It is more than a little confusing that Java Errors are like Go panics, and Java Exceptions are like Go errors.) The only concern left is any potential performance impact from the defer/recover block, but I believe the language barrier will always incur a more significant overhead. |
I took a stab at implementing this: https://go-review.googlesource.com/#/c/11920/ Feedback welcome. Haven't written C in years, and JNI code in... ever. :) |
CL https://golang.org/cl/11920 mentions this issue. |
What was wrong with the suggestion of replacing exit(2) with abort(3)? Not every Go call originates from java, and it seems more appropriate for a library (libgojni.so) to abort rather than exit the foreign main program. |
after chatting with @alandonovan the other day, I also realized that turning the panic into a java error addresses only part of the problem. There could be background goroutine causing crash, some library underneath deciding to crash, gc panics, or deadlock(not sure if it applies to library). In order to capture these cases, I also wish we could use abort instead of exit. @crawshaw @ianlancetaylor |
That makes sense to me. Any benefit in translating to Error where we can (i.e. in Go code directly invoked from Java) and aborting when we can't? Maybe cleaner stack traces on the Errors? |
@rsc pointed out runtime has already a way to change the program crash behavior by setting GOTRACEBACK env var and we may be able to utilize the similar hook. http://golang.org/pkg/runtime/ "If GOTRACEBACK=crash, the per-goroutine stack traces include run-time functions, and if possible the program crashes in an operating-specific manner instead of exiting. For example, on Unix systems, the program raises SIGABRT to trigger a core dump." On linux, the SIGABRT is handled and creates a core dump. On mobile, change the full stack trace to the Errors? |
CL 12032 is my proposal for fixing this issue. GOTRACEBACK is read too early in the runtime initialization to allow for gomobile to fiddle with GOTRACEBACK and in any case I believe the aborting behaviour should cover all cases where C owns the process. In any case, GOTRACEBACK=crash behaviour includes GOTRACEBACK=2 behaviour (print runtime functions in tracebacks) which is not what we want here. |
A
panic()
from Go code eventually results in a syscall toexit(2)
which terminates the Android app. This "clean" exit prevents Crashlytics (https://get.fabric.io/crashlytics) from capturing a Go panic as a crash.Crashlytics support told me:
Could we recover these panics and throw a Java Error? Or perhaps syscall
abort
(which should be caught by a SIGABRT handler) instead ofexit
?The text was updated successfully, but these errors were encountered: