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

kotlinx-coroutines-android needs more Proguard rules to avoid having META-INF/services removed #983

Closed
benkuhn opened this issue Feb 12, 2019 · 10 comments
Labels

Comments

@benkuhn
Copy link

@benkuhn benkuhn commented Feb 12, 2019

We recently bumped our kotlin-coroutines version and our production (Proguard) build started crashing.

In addition to the proguard rules currently documented as being necessary, I also had to add the following Proguard rules:

-keep class kotlinx.coroutines.android.AndroidExceptionPreHandler
-keep class kotlinx.coroutines.android.AndroidDispatcherFactory

Without these, the META-INF/services files provided by kotlinx-coroutines-android were being removed by proguard, causing the following error message:

java.lang.IllegalStateException: Module with the Main dispatcher is missing. Add dependency providing the Main dispatcher, e.g. 'kotlinx-coroutines-android'

@benkuhn benkuhn changed the title kotlinx-coroutines-android META-INF/services gets stripped by Proguard kotlinx-coroutines-android needs more Proguard rules to avoid having META-INF/services removed Feb 12, 2019
@elizarov

This comment has been minimized.

Copy link
Member

@elizarov elizarov commented Feb 12, 2019

This is very strange. It seems to be some bug in your updated toolchain, as both those classes are marked with Android's @Keep annotation specifically to make sure they are preserved by ProGuard (default ProGuard rules are supposed to keep all classes marked with this annotation):

@elizarov elizarov added the question label Feb 12, 2019
@JakeWharton

This comment has been minimized.

Copy link
Contributor

@JakeWharton JakeWharton commented Feb 12, 2019

Turns out the annotations library shipped the wrong rules which didn't reference it's own types correctly. It's already been fixed but not yet released. I'll see if I can push a dot release through. Additionally, I'm pushing working with the R8 team to remove the need for any rules or annotations with a service loader. All the information for this to be handled automatically is there, no explicit rules or annotations are needed. Even if implemented, it will be months before it's available to everyone in a stable release so correct rules and annotations are important for now.

@plastiv

This comment has been minimized.

Copy link

@plastiv plastiv commented Feb 12, 2019

Jake, are default agp proguard rules valid or are also wrong?

shrinkResources true
proguardFiles getDefaultProguardFile('proguard-android-optimize.txt' or 'proguard-android.txt'), 'proguard-project.txt'

For AGP 3.3.1 I see inside proguard-android.txt next lines:

# Understand the @Keep support annotation.
-keep class android.support.annotation.Keep
-keep class androidx.annotation.Keep

-keep @android.support.annotation.Keep class * {*;}
-keep @androidx.annotation.Keep class * {*;}

-keepclasseswithmembers class * {
    @android.support.annotation.Keep <methods>;
}

-keepclasseswithmembers class * {
    @androidx.annotation.Keep <methods>;
}

-keepclasseswithmembers class * {
    @android.support.annotation.Keep <fields>;
}

-keepclasseswithmembers class * {
    @androidx.annotation.Keep <fields>;
}

-keepclasseswithmembers class * {
    @android.support.annotation.Keep <init>(...);
}

-keepclasseswithmembers class * {
    @androidx.annotation.Keep <init>(...);
}
@JakeWharton

This comment has been minimized.

Copy link
Contributor

@JakeWharton JakeWharton commented Feb 12, 2019

@JakeWharton

This comment has been minimized.

Copy link
Contributor

@JakeWharton JakeWharton commented Feb 12, 2019

Now that I'm at a computer, when R8 runs in full mode it might collapse the service interface and single implementation together. It does this despite the -keepnames declaration. Still trying to figure out if that's correct or not (it's a matter of interpretation). Changing to -keep will prevent the class merging.

When the types are merged, the call to ServiceLoader.load is updated but the META-INF/services/ file name is not changed. This is an R8 bug.

But again, that being said, I am emploring the R8 team to do away with all this nonsense such that calls to ServiceLoader.load correlate to the META-INF/services/ file name and it to its contents default constructors. This means that during obfuscation the file name and content will be correctly update. During optimization (like class merging) the file name will be updated. During shrinking, if the call to ServiceLoader.load is to be removed, the resources file and its implementations go with it. If it is not to be removed, the resource file and the types it refers to will automatically be kept.

qwwdfsad added a commit that referenced this issue Feb 18, 2019
@qwwdfsad qwwdfsad closed this Apr 13, 2019
@Panajev

This comment has been minimized.

Copy link

@Panajev Panajev commented May 5, 2019

Hello @qwwdfsad @JakeWharton ,

I am finding the same issue now with Android Studio 3.4.0, gradle plugin 3.4.0, R8 full mode explicitly disabled, coroutines 1.2.1, and R8 v1.4.77 (build b74371231cb896b02f83285efb055b7c00ff64d8).

Building it and exporting it through the commandline (and minifying the code), installing it, ... will lead to the same crash at the top of this Issue.
Here is the coroutines related portion of our proguard.pro file:

# ServiceLoader support
-keepnames class kotlinx.coroutines.internal.MainDispatcherFactory {}
-keepnames class kotlinx.coroutines.CoroutineExceptionHandler {}
-keepnames class kotlinx.coroutines.android.AndroidExceptionPreHandler {}
-keepnames class kotlinx.coroutines.android.AndroidDispatcherFactory {}

-keep class kotlinx.coroutines.internal.MainDispatcherFactory {}
-keep class kotlinx.coroutines.CoroutineExceptionHandler {}
-keep class kotlinx.coroutines.android.AndroidExceptionPreHandler {}
-keep class kotlinx.coroutines.android.AndroidDispatcherFactory {}

# Most of volatile fields are updated with AFU and should not be mangled
-keepclassmembernames class kotlinx.** {
    volatile <fields>;
}

gradle.properties file:

# Project-wide Gradle settings.

# IDE (e.g. Android Studio) users:
# Gradle settings configured through the IDE *will override*
# any settings specified in this file.

# For more details on how to configure your build environment visit
# http://www.gradle.org/docs/current/userguide/build_environment.html

# Specifies the JVM arguments used for the daemon process.
# The setting is particularly useful for tweaking memory settings.
org.gradle.jvmargs=-Xmx1536m

# When configured, Gradle will run in incubating parallel mode.
# This option should only be used with decoupled projects. More details, visit
# http://www.gradle.org/docs/current/userguide/multi_project_builds.html#sec:decoupled_projects
org.gradle.configureondemand=false
org.gradle.daemon=false
android.useAndroidX=true
android.enableJetifier=true
android.debug.obsoleteApi=true
android.enableR8.fullMode=false

ADB crashlog:

05-06 00:05:43.681 18709 18709 E AndroidRuntime: java.lang.IllegalStateException: Module with the Main dispatcher is missing. Add dependency providing the Main dispatcher, e.g. 'kotlinx-coroutines-android'
05-06 00:05:43.681 18709 18709 E AndroidRuntime:        at kotlinx.coroutines.internal.MissingMainCoroutineDispatcher.missing(MainDispatchers.kt:72)
05-06 00:05:43.681 18709 18709 E AndroidRuntime:        at kotlinx.coroutines.internal.MissingMainCoroutineDispatcher.isDispatchNeeded(MainDispatchers.kt:53)
05-06 00:05:43.681 18709 18709 E AndroidRuntime:        at kotlinx.coroutines.DispatchedKt.resumeCancellable(Dispatched.kt:410)
05-06 00:05:43.681 18709 18709 E AndroidRuntime:        at kotlinx.coroutines.intrinsics.CancellableKt.startCoroutineCancellable(Cancellable.kt:25)
05-06 00:05:43.681 18709 18709 E AndroidRuntime:        at kotlinx.coroutines.CoroutineStart.invoke(CoroutineStart.kt:109)
05-06 00:05:43.681 18709 18709 E AndroidRuntime:        at kotlinx.coroutines.AbstractCoroutine.start(AbstractCoroutine.kt:154)

Could this bug be reopened please? Should I also open an issue on the R8 bug tracker?

Kind Regards,

Goffredo

@navczydev

This comment has been minimized.

Copy link

@navczydev navczydev commented May 9, 2019

Getting same issue : any update?
Android Studio 3.4
Kotlin : 1.3.31
coroutines : 1.2.1

@qwwdfsad

This comment has been minimized.

Copy link
Member

@qwwdfsad qwwdfsad commented May 13, 2019

#657 (comment) does it helps?

@Panajev could you please attach a reproducing project?

@Panajev

This comment has been minimized.

Copy link

@Panajev Panajev commented May 14, 2019

Hey @qwwdfsad ,

Sorry for not replying earlier, but thanks for the followup.

We found the issue and it was related to our manual signing process which was perhaps the oldest legacy code we had in our Android codebase. We solved with some research before seeing that comment, but yeah it would have lead to #657 (comment) which is spot on:

Basically, we were re-signing the apk and removing the META-INF folder in the process. So, for the ones that are still facing the issue, please make sure that you are not doing the same (re-signing your apk and removing the META-INF folder after the release build is done)

The problem was due to this step in the signing+aligning phase (I presume to make it easy to resign it):

zip -d "$apkInputFullPath" META-INF/\*

# Sign with distribution key
jarsigner -verbose -storepass [...]

(moved away from it and modernised that path finally, signging, zipping, and aligning production builds through gradle signing properties...while a quick and dirty fix would have been [I tested it and it was working]:

zip -d "$apkInputFullPath" META-INF/\*.RSA
zip -d "$apkInputFullPath" META-INF/\*.SF
zip -d "$apkInputFullPath" META-INF/\*.MF

as those are the only files created / modified during the jarsigner signing phase...
)

As far as I know, luckily, this was the first case where the application had a runtime issue due to this.

Thank you again for following up to my message!

Kind Regards,

Goffredo

@AfzalivE

This comment has been minimized.

Copy link

@AfzalivE AfzalivE commented Sep 4, 2019

Still seeing this issue with AGP 3.5.0 and Coroutines 1.3.0

@Panajev Your message saved me countless hours! That was my issue too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.