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.

Sign up for GitHub

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

android: crash every time I run a test using Android 12 #1897

Closed
bassosimone opened this issue Nov 24, 2021 · 0 comments · Fixed by ooni/probe-android#461
Closed

android: crash every time I run a test using Android 12 #1897

bassosimone opened this issue Nov 24, 2021 · 0 comments · Fixed by ooni/probe-android#461
Assignees
@bassosimone
Labels
bug Something isn't working platform/android priority/high user feedback requests that have been added to the backlog as a direct result of user feedback or testing

Comments

@bassosimone
Copy link
Contributor

Describe the bug

Every time I run a test with Android 12, the app crashes.

To Reproduce

Install the app on Android 12 and run any test. (I installed the experimental flavor but this information is irrelevant because users have reported to us the same issue in the last two days, so...)

Expected behavior

Tests should complete and the app should not crash.

Screenshots

N/A

System information (please complete the following information):

  • Device: Pixel
  • OS: Android 12
  • OONI Probe version: 3.4.0 + minimal diff to add new engine

Additional context

This is the stack trace:

2021-11-24 09:44:24.765 26404-26404/? D/AndroidRuntime: Shutting down VM
2021-11-24 09:44:24.766 26404-26404/? E/AndroidRuntime: FATAL EXCEPTION: main
    Process: org.openobservatory.ooniprobe.experimental, PID: 26404
    java.lang.RuntimeException: Unable to start service org.openobservatory.ooniprobe.common.service.RunTestService@1c00fa8 with Intent { cmp=org.openobservatory.ooniprobe.experimental/org.openobservatory.ooniprobe.common.service.RunTestService (has extras) }: java.lang.IllegalArgumentException: org.openobservatory.ooniprobe.experimental: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
    Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
        at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:4657)
        at android.app.ActivityThread.access$2000(ActivityThread.java:247)
        at android.app.ActivityThread$H.handleMessage(ActivityThread.java:2091)
        at android.os.Handler.dispatchMessage(Handler.java:106)
        at android.os.Looper.loopOnce(Looper.java:201)
        at android.os.Looper.loop(Looper.java:288)
        at android.app.ActivityThread.main(ActivityThread.java:7838)
        at java.lang.reflect.Method.invoke(Native Method)
        at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:548)
        at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:1003)
     Caused by: java.lang.IllegalArgumentException: org.openobservatory.ooniprobe.experimental: Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.
    Strongly consider using FLAG_IMMUTABLE, only use FLAG_MUTABLE if some functionality depends on the PendingIntent being mutable, e.g. if it needs to be used with inline replies or bubbles.
        at android.app.PendingIntent.checkFlags(PendingIntent.java:375)
        at android.app.PendingIntent.getBroadcastAsUser(PendingIntent.java:645)
        at android.app.PendingIntent.getBroadcast(PendingIntent.java:632)
        at org.openobservatory.ooniprobe.common.service.RunTestService.onStartCommand(RunTestService.java:83)
        at android.app.ActivityThread.handleServiceArgs(ActivityThread.java:4639)
        	... 9 more

The key piece of information here is:

Targeting S+ (version 31 and above) requires that one of FLAG_IMMUTABLE or FLAG_MUTABLE be specified when creating a PendingIntent.

The fix seems to modify the code to provide the correct flags to the pendingIntent.

(This is not my domain of expertise, hence it may be that I am not fully grokking the problem here.)
@bassosimone bassosimone added bug Something isn't working triage labels Nov 24, 2021
@bassosimone bassosimone self-assigned this Nov 24, 2021
@bassosimone bassosimone added platform/android priority/high user feedback requests that have been added to the backlog as a direct result of user feedback or testing and removed triage labels Nov 24, 2021
bassosimone added a commit to ooni/probe-android that referenced this issue Nov 24, 2021
@bassosimone
fix(RunTestService.java): pending intent should be IMMUTABLE
162707d 
The app was crashing because the code did not specify the pending
intent as IMMUTABLE (as far as I understand the stacktrace).

Closes ooni/probe#1897

(This is not my domain of expertise and I think this patch could
and should be better refactored so that we always use a single
factory to create all the intents with the correct flags.)
@bassosimone bassosimone mentioned this issue Nov 24, 2021
Merged
bassosimone added a commit to ooni/probe-android that referenced this issue Nov 24, 2021
@bassosimone
fix(RunTestService.java): use factories to avoid repeating ourselves
1994ba9 
This diff addresses my previous concern that I fixed the immediate
problem but did not proactively check the rest of the codebase.

Now, `git grep -i pendingintent.newactivity` and
`git grep -i pendingintent.getbroadcast` say that we
have correctly wrapped all instance flags.

See ooni/probe#1897
lorenzoPrimi pushed a commit to ooni/probe-android that referenced this issue Nov 25, 2021
@bassosimone
fix(RunTestService.java): pending intent should be IMMUTABLE (#461)
eaf23ec 
* fix(RunTestService.java): pending intent should be IMMUTABLE

The app was crashing because the code did not specify the pending
intent as IMMUTABLE (as far as I understand the stacktrace).

Closes ooni/probe#1897

(This is not my domain of expertise and I think this patch could
and should be better refactored so that we always use a single
factory to create all the intents with the correct flags.)

* fix(RunTestService.java): use factories to avoid repeating ourselves

This diff addresses my previous concern that I fixed the immediate
problem but did not proactively check the rest of the codebase.

Now, `git grep -i pendingintent.newactivity` and
`git grep -i pendingintent.getbroadcast` say that we
have correctly wrapped all instance flags.

See ooni/probe#1897
@bassosimone bassosimone mentioned this issue Dec 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@bassosimone bassosimone

Labels
bug Something isn't working platform/android priority/high user feedback requests that have been added to the backlog as a direct result of user feedback or testing
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(RunTestService.java): pending intent should be IMMUTABLE ooni/probe-android
1 participant
@bassosimone