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

x/mobile: passing error into a callback crashes the library init #17073

Closed
karalabe opened this issue Sep 12, 2016 · 3 comments
Closed

x/mobile: passing error into a callback crashes the library init #17073

karalabe opened this issue Sep 12, 2016 · 3 comments

Comments

@karalabe
Copy link
Contributor

karalabe commented Sep 12, 2016

When a method returns an error type, gomobile will internally convert that into a Java exception Android side. Of course, converting errors into exceptions would mean that an error cannot be passed to a callback method (implemented in Java) since it's not actually a real type, rather a special case.

However doing exactly this will cause gomobile to happily build the archive without reporting an error. This will result in the library crashing the android application when it's first accessed. Since errors are handled in a special way I can fully accept that they cannot be passed to functions as an argument, however we should probably abort compilation in that case and bail out with an error.

e.g. the below interface will build and crash upon library load

type I interface {
    OnError(err error)
}

The error is something like this:

JNI DETECTED ERROR IN APPLICATION: JNI GetMethodID called with pending exception java.lang.NoSuchMethodError: no non-static method "Lorg/ethereum/geth/FilterLogsHandler;.onError(Lgo/error;)V"
@quentinmit quentinmit changed the title mobile: passing error into a callback crashes the library init x/mobile: passing error into a callback crashes the library init Sep 12, 2016
@quentinmit quentinmit added this to the Unreleased milestone Sep 12, 2016
@eliasnaur
Copy link
Contributor

Thank you. Is the type declaration enough to crash the program? If not, please provide the complete code and I'll take a look.

@karalabe
Copy link
Contributor Author

It is enough. I've added the following code to the end of bind/testpkg/testpkg.go:

type ErrorCrasher interface {
    OnError(err error)
}

And ran the TestJavaSeqTest test inside bind/java, which fails with

go.SeqTest > testAdd[Nexus 5X - 6.0.1] FAILED 
Test failed to run to completion. Reason: 'Instrumentation run failed due to 'Native crash''. Check device logcat for details
Tests on Nexus 5X - 6.0.1 failed: Instrumentation run failed due to 'Native crash'

@gopherbot
Copy link

CL https://golang.org/cl/29176 mentions this issue.

@golang golang locked and limited conversation to collaborators Sep 16, 2017
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 10, 2021
Errors was recently converted to use objects as representation instead
of strings. Issue golang/go#17073 exposed a few places that wasn't properly
updated. Fix them and add the test case from the the issue.

Fixes golang/go#17073

Change-Id: I0191993a8427d930540716407fc09032f282fc66
Reviewed-on: https://go-review.googlesource.com/29176
Reviewed-by: David Crawshaw <crawshaw@golang.org>
imWildCat pushed a commit to imWildCat/go-mobile that referenced this issue Apr 11, 2021
Errors was recently converted to use objects as representation instead
of strings. Issue golang/go#17073 exposed a few places that wasn't properly
updated. Fix them and add the test case from the the issue.

Fixes golang/go#17073

Change-Id: I0191993a8427d930540716407fc09032f282fc66
Reviewed-on: https://go-review.googlesource.com/29176
Reviewed-by: David Crawshaw <crawshaw@golang.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants