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

Jump to bottom

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

Closed
timcooijmans opened this issue Apr 9, 2019 · 7 comments
Closed

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

timcooijmans opened this issue Apr 9, 2019 · 7 comments
Labels
FrozenDueToAge help wanted mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@timcooijmans
Copy link
Contributor

timcooijmans commented Apr 9, 2019 •
edited

Problem

For loading gobind-libraries on Android, currently the non-SDK method android.app.AppGlobals.getInitialApplication is used via reflection in LoadJNI.java to provide the ApplicationContext to the native-library.

Application appl = (Application)Class.forName("android.app.AppGlobals").getMethod("getInitialApplication").invoke(null);
androidCtx = appl.getApplicationContext();

Usage of non-SDK methods is not allowed in Android P and up (API level 28 and higher), for more information see: developer.android.com: Restrictions on non-SDK interfaces
From Android Q on this prevents an gobind-library from loading and causes an exception:

 D/StrictMode: StrictMode policy violation: android.os.strictmode.NonSdkApiUsedViolation: Landroid/app/AppGlobals;->getInitialApplication()Landroid/app/Application;
        at android.os.StrictMode.lambda$static$1(StrictMode.java:406)
        at android.os.-$$Lambda$StrictMode$lu9ekkHJ2HMz0jd3F8K8MnhenxQ.accept(Unknown Source:2)
        at java.lang.Class.getDeclaredMethodInternal(Native Method)
        at java.lang.Class.getPublicMethodRecursive(Class.java:2079)
        at java.lang.Class.getMethod(Class.java:2066)
        at java.lang.Class.getMethod(Class.java:1693)
        at go.LoadJNI.<clinit>(LoadJNI.java:28)
        at java.lang.Class.classForName(Native Method)
        at java.lang.Class.forName(Class.java:454)
        at java.lang.Class.forName(Class.java:379)
        at go.Seq.<clinit>(Seq.java:38)
        at go.Seq.touch(Seq.java:55)

Solution

I think we should consider asking the ApplicationContext to be provided by the application when loading the gobind-library instead of acquiring it via reflection.

An proposal to defer context-binding was discussed in #12725. Another option would be to defer loading of the library itself.
@gopherbot gopherbot added this to the Unreleased milestone Apr 9, 2019
@gopherbot gopherbot added the mobile Android, iOS, and x/mobile label Apr 9, 2019
@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. help wanted labels Apr 10, 2019
@timcooijmans
Copy link
Contributor Author

timcooijmans commented May 3, 2019 •
edited

I introduced a fix: golang/mobile#31

@mandrigin mandrigin mentioned this issue May 20, 2019
Closed
@timcooijmans
Copy link
Contributor Author

With the Play Store now giving warnings, can we look at this or another solution getting merged?

@dmitshur
Copy link
Contributor

dmitshur commented Jul 13, 2019 •
edited

This was resolved via CL 175103, closing.

(The reason it wasn't closed automatically is because the "Fixes #31364" line in the commit message was missing the "golang/go" prefix required for subrepo CLs. See details here).

@hajimehoshi
Copy link
Member

It looks like the fix makes app.RunOnVM unavailable for gomobile-bind. I realized that originally RunOnVM should not be used for gomobile-bind, so that's fine, I was surprised though.

@timcooijmans
Copy link
Contributor Author

timcooijmans commented Aug 10, 2019 •
edited

app.RunOnVm should still work, but you have to call the (new) static method Seq.init(Context context) Seq.setContext(Context context)to pass the required Context to Go before using app.RunOnVm

@hajimehoshi
Copy link
Member

hajimehoshi commented Aug 10, 2019 •
edited

I think you meant Seq.setContext? Yes, that fixed the issue. I missed that in the CL. Thank you!

BTW, (this is an off topic but,) I started to wonder if reverse binding would be better than RunOnJVM. RunOnJVM cannot be called in init functions while the reverse binding should be available anywhere. I'll check this later.

EDIT: nvm, the reverse binding is available only at the package specified by gomobile command, then this cannot be a substitute for RunOnVM.

@timcooijmans
Copy link
Contributor Author

Yes, sorry quick reply from my phone. I edited my original post.

@golang golang locked and limited conversation to collaborators Aug 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge help wanted mobile Android, iOS, and x/mobile NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
5 participants
@hajimehoshi @dmitshur @timcooijmans @bcmills @gopherbot