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

Comments

Projects
None yet
4 participants
@karalabe
Copy link
Contributor

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

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2016

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

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2016

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

This comment has been minimized.

Copy link

commented Sep 14, 2016

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

@golang golang locked and limited conversation to collaborators Sep 16, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.