New "fast" service loader makes the app crash when used with Apply Changes from Android Studio

[LouisCAD]

I don't know if the regular service loader would exhibit this behavior as I'm only getting started using the new Apply changes from Android Studio 3.5.

Here's the logs when the crash happened.
After restarting the app, no crash, but that kinda defeats the purpose of Apply Changes.

2019-04-08 10:05:10.807 17059-17059/? E/System: Unable to open zip file: /data/app/com.beepiz.test.app-QROyb725zrc_OUcykKPQMQ==/base.apk
2019-04-08 10:05:10.809 17059-17059/? E/System: java.io.FileNotFoundException: File doesn't exist: /data/app/com.beepiz.test.app-QROyb725zrc_OUcykKPQMQ==/base.apk
        at java.util.zip.ZipFile.<init>(ZipFile.java:215)
        at java.util.zip.ZipFile.<init>(ZipFile.java:152)
        at java.util.jar.JarFile.<init>(JarFile.java:160)
        at java.util.jar.JarFile.<init>(JarFile.java:97)
        at libcore.io.ClassPathURLStreamHandler.<init>(ClassPathURLStreamHandler.java:47)
        at dalvik.system.DexPathList$Element.maybeInit(DexPathList.java:702)
        at dalvik.system.DexPathList$Element.findResource(DexPathList.java:729)
        at dalvik.system.DexPathList.findResources(DexPathList.java:526)
        at dalvik.system.BaseDexClassLoader.findResources(BaseDexClassLoader.java:174)
        at java.lang.ClassLoader.getResources(ClassLoader.java:839)
        at kotlinx.coroutines.internal.FastServiceLoader.loadProviders$kotlinx_coroutines_core(FastServiceLoader.kt:46)
        at kotlinx.coroutines.internal.FastServiceLoader.load$kotlinx_coroutines_core(FastServiceLoader.kt:35)
        at kotlinx.coroutines.internal.MainDispatcherLoader.loadMainDispatcher(MainDispatchers.kt:15)
        at kotlinx.coroutines.internal.MainDispatcherLoader.<clinit>(MainDispatchers.kt:10)
        at kotlinx.coroutines.Dispatchers.getMain(Dispatchers.kt:55)
        at kotlinx.coroutines.android.HandlerDispatcherKt.awaitFrame(HandlerDispatcher.kt:176)

17059-17059/? E/AndroidRuntime: FATAL EXCEPTION: main
    Process: com.mycompanyname.myapp, PID: 17059
    java.lang.IllegalStateException: Module with the Main dispatcher is missing. Add dependency providing the Main dispatcher, e.g. 'kotlinx-coroutines-android'
        at kotlinx.coroutines.internal.MissingMainCoroutineDispatcher.missing(MainDispatchers.kt:72)
        at kotlinx.coroutines.internal.MissingMainCoroutineDispatcher.dispatch(MainDispatchers.kt:65)
        at kotlinx.coroutines.internal.MissingMainCoroutineDispatcher.dispatch(MainDispatchers.kt:45)
        at kotlinx.coroutines.android.HandlerDispatcherKt.awaitFrame(HandlerDispatcher.kt:176)

Related to #878

[qwwdfsad]

I will investigate this.
You can try to disable new service loader with system property kotlinx.coroutines.fast.service.loader set to false

[LouisCAD]

@qwwdfsad How do I set a "system property" in an Android project that uses Gradle?

[qwwdfsad]

You can do it programmatically with System.setProperty

[qwwdfsad]

I've experimented with my sample project on AS 3.5 (Canary 9) and it appears to work.
Could you please provide a reproducer?

[LouisCAD]

I haven't been able to reproduce the issue either.
Likely a bug in Apply Changes, I reported it here: https://issuetracker.google.com/issues/130141145

[acleung]

We (Apply Changes Team in Studio) have discovered that the issues is mostly related to ServiceLoader used in the library.

What happen is that the ServiceLoader needs to read configuration files in the APK's META-INFO/. It does this by calling getResources on the classloader. If a swap were to happen and an new APK is installed in the process of this is happening, the classloader can still be referencing the old APK and trigger the FileNotFound exception.

There is very little we can do on our end to make this work, especially for deploying to previous versions of Android.

Is it possible to avoid the ServiceLoader solution here? At least for Android devices?

[elizarov]

@acleung Kotlin coroutines is not an Android-specific library that has to support Android conventions out-of-the box without extra configuration, so that's not much we can do, but to rely on ServiceLoader. The only upside here, is that the latest version of R8 should remove all usages of ServiceLoader from the resulting dex files.

[qwwdfsad]

@acleung

Is it possible to avoid the ServiceLoader solution here?

Potentially, yes, we can replace it with hierarchical Class.forName lookups, though it will complicate things a lot for us (especially maintainability and extensibility).

But I am not sure whether it is a right approach at all. I realize there might be a lot of technical difficulties, but ServiceLoader is both officially supported and widespread in the JVM ecosystem. Even though we could avoid it in kotlinx.coroutines, there are a lot of libraries in the wild that cannot (or even are no longer maintained). So it won't solve the problem with apply changes in the first place, but only mask it for a while.

[acleung]

To be clear, I agree with @elizarov and @qwwdfsad. ServiceLoader is well supported in JVM and currently working as intended on Android.

Yes, R8 has support for optimizing ServiceLoader but I don't think we can rely on it because most people who wants to Hot Swap code is probably doing development and unlikely to run full R8 on development build. I just spoke to the R8 team, who was incidentally visiting us last week, about supporting this in D8 instead but that would not make sense.

Kotlin (+coroutine) is going to be important part of Android development going forward and I am doing every under my power to make sure Kotlin will work well for hotswapping. it would be a real shame if Kotlin developers get left out of Hot Swapping code as soon as they start using coroutine. I'll continue to push for better ServiceLoader reload support in Android devices in the future but it will only be available for future devices. I would really appreciate it if the developer team here at least bring up the ServiceLoader design / workaround decision once more if possible.

Thank you!

[qwwdfsad]

@acleung after thinking about it for a while, I think we can find a compromise that satisfies both parties. If I got the problem right, this is a race between swapping APKs and resolving META-INF within APK that is being swapped.

The most straightforward solution here is to retry service-loading if we detect we are running on the Android (btw, what's the most reliable way to do it? It would be nice to check some system property instead of using Class.forName tricks) and just retry the load.
WDYT about this solution?
Moreover, the default ServiceLoader implementation on Android can do this in the debug mode at some point

[acleung]

As far as we understand, the runtime's classloader holds a path to the old APK and repeated calls to it will continue to get the stale path of a previous installed APK.

I'll investigate a bit more and get back to you. In the mean time, if you can think of anything else, please let me know.

Thanks!