x/mobile: android bind/java/Seq.java forces application context binding on library load #12725

Qubitium · 2015-09-23T09:46:23Z

Problem

It is not required to bind an application context for go <-> jni code to work. For native activities that uses context, this might be case, but not all go library code needs this.
Compatibility issues if context is auto-bound. In the wild we have experienced wild/random crashes that is related to the order of webview native libary loading before/after golib plus other jni libraries. Our fix required us to load each library in a specific order and/or not binding to application context.

Source

At Line 29 of bind/java/Seq.java, application context is passed/set to golib via jni.

static {
        // Look for the shim class auto-generated by gomobile bind.
        // Its only purpose is to call System.loadLibrary.
        try {
            Class.forName("go.LoadJNI");
        } catch (ClassNotFoundException e) {
            // Ignore, assume the user will load JNI for it.
            Log.w("GoSeq", "LoadJNI class not found");
        }

        try {
            // TODO(hyangah): check proguard rule.
            Application appl = (Application)Class.forName("android.app.AppGlobals").getMethod("getInitialApplication").invoke(null);
            Context ctx = appl.getApplicationContext();
            setContext(ctx);

        } catch (Exception e) {
            Log.w("GoSeq", "Global context not found:" + e);
        }

        initSeq();
        new Thread("GoSeq") {
            public void run() { Seq.receive(); }
        }.start();
    }

Proposal

Do not auto bind application context on library load. Let user decide.

static {
        // Look for the shim class auto-generated by gomobile bind.
        // Its only purpose is to call System.loadLibrary.
        try {
            Class.forName("go.LoadJNI");
        } catch (ClassNotFoundException e) {
            // Ignore, assume the user will load JNI for it.
            Log.w("GoSeq", "LoadJNI class not found");
        }

        initSeq();
        new Thread("GoSeq") {
            public void run() { Seq.receive(); }
        }.start();
    }

hyangah · 2015-09-23T15:10:27Z

You're right. Our intention was to allow users to use the other golang.org/x/mobile packages (asset, sensors, etc) in gobind apps seamlessly (they are currently designed for native Go apps).

We can make it explicit and inform users to call them before using any of the packages.

One question I have is if it's right to cache the context in go side, and if so what context we should cache. I guess ApplicationContext would work for asset, sensor, audio once the application context is initialized. For UI components however (creating a webview?) ActivityContext or some other context should be used.

cc @crawshaw

Qubitium · 2015-09-23T17:57:46Z

@hyangah There are so many bugs/memory leaks with WebView that a lot of devs are using it with application context since a WebView leak this way doesn't leak the Activity. With this in mind, a native activity with UI elements might need to support having a case of mixed contexts depending on dev pref: WebView on application context, rest on Activity context.

crawshaw · 2015-09-24T15:06:22Z

@diegomontoya there seem to be a few things going on here and I'd like to pull them apart and address them one at a time.

The first is that you say the piece of code above that grabs the global application context is causing your program to crash. I'd like to fix that. Can you give us some more information? For example, the log output?

The second is whether or not we need to provide a way for users to pass a context of their own to the native Go packages. If you have a specific need for this, please file another issue. Right now the only things we use that android context for only need the global context, and I would expect a user to plumb through their own android context if they need it. (If I'm reading this bug properly, it sounds like your problem is that you don't need it plumbed through, and our current code doesn't always work.)

dskinner · 2015-09-25T23:07:22Z

@crawshaw given my original issue was closed before I managed to put together an example, and a comment there was that it's likely out-of-date, I tried to build against the latest of everything and still experience a crash on certain android platforms.

I created an example app that experiences the issue here:
https://github.com/dskinner/issue12725

you'll want to update https://github.com/dskinner/issue12725/blob/master/android/client/build.gradle appropriately.

The example app does make a network call in go.

On an API 16 ARM emulator, the app works fine. On a Nexus 10 Android 5.1.1, the app experiences a WIN DEATH and exits immediately.

Qubitium · 2015-10-17T01:23:20Z

@crawshaw The crash problem is not a go problem but based on what I have read, really bad webview code in several versions of android in which the webview is misusing a signal handler which results in the current "fix" in the wild where if you are using jni lib, it is best to force webview to load first, then the regular libs. @dskinner's crash might also be caused by webview since the google ad itself is a wrapped in a webview.

https://code.google.com/p/chromium/issues/detail?id=476831

Based on our own experience, the scope of the problem is wider than just the bug above.

There is nothing wrong with Go generated code or Go code itself.
But if we use Go generated code, there will be lots of random crashes if any jni/library is used along with webview.

Not sure what the best way to go about this but to give user the option to manually bind the context at their choosing instead of auto-binding at a random access point in which the first go code is executed.

Qubitium · 2015-10-17T01:28:55Z

@crawshaw To clarify the potential crash due to bad webview and jni libs, go included. I am not referring to instant crash when the java load golib-jni.so code is called. The signal handler issue with webview and jni lib loading order causes a "future" crash point which is unpredictable.

dskinner · 2015-10-17T04:04:27Z

in the past, I had found that loading the webview first did resolve the issue, but only for trivial cases. That's one of the reasons the example app I provided uses a recycler view that places ads at every nth position. Such a case inevitably crashes at some point regardless of how the app bootstraps on start.

Perhaps that is because the webview code eventually reinits mid scroll? I don't know. But the crash is most definitely caused by the webview google ads uses if I didn't make that clear.

Qubitium changed the title ~~x/mobile android bind/java/Seq.java LoadLibrary() forces application context binding~~ x/mobile android bind/java/Seq.java forces application context binding on library load Sep 23, 2015

hyangah mentioned this issue Sep 23, 2015

x/mobile: Android net.Dial("tcp", ip, port) returns "protocol not available" #12598

Closed

hyangah added this to the Unreleased milestone Sep 23, 2015

hyangah mentioned this issue Sep 23, 2015

x/mobile: loading webviewchromium after Go.init crashes on 5.0.1 #9507

Closed

rakyll changed the title ~~x/mobile android bind/java/Seq.java forces application context binding on library load~~ x/mobile: android bind/java/Seq.java forces application context binding on library load Sep 23, 2015

gopherbot added the mobile Android, iOS, and x/mobile label Jul 20, 2017

timcooijmans mentioned this issue Apr 9, 2019

x/mobile/bind: Reflection to get ApplicationContext is disallowed on Android P (API Level 28) and causes a crash on Android Q #31364

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

x/mobile: android bind/java/Seq.java forces application context binding on library load #12725

x/mobile: android bind/java/Seq.java forces application context binding on library load #12725

Qubitium commented Sep 23, 2015

hyangah commented Sep 23, 2015

Qubitium commented Sep 23, 2015

crawshaw commented Sep 24, 2015

dskinner commented Sep 25, 2015

Qubitium commented Oct 17, 2015

Qubitium commented Oct 17, 2015

dskinner commented Oct 17, 2015

x/mobile: android bind/java/Seq.java forces application context binding on library load #12725

x/mobile: android bind/java/Seq.java forces application context binding on library load #12725

Comments

Qubitium commented Sep 23, 2015

Problem

Source

Proposal

hyangah commented Sep 23, 2015

Qubitium commented Sep 23, 2015

crawshaw commented Sep 24, 2015

dskinner commented Sep 25, 2015

Qubitium commented Oct 17, 2015

Qubitium commented Oct 17, 2015

dskinner commented Oct 17, 2015