User Class.forName instead of ServiceLoader to instantiate Dispatchers.Main on Android #1572
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
JakeWharton
reviewed
Sep 25, 2019
kotlinx-coroutines-core/jvm/resources/META-INF/proguard/coroutines.pro
Outdated
Show resolved
Hide resolved
|
In principle it looks good. I'm wondering if you then still want to have the R8-specific |
qwwdfsad
force-pushed
the
android-service-loader
branch
2 times, most recently
from
98dbf26 to
7a625e8
Compare
qwwdfsad
force-pushed
the
android-service-loader
branch
from
41281e5 to
cf9fc1a
Compare
yamasa
added a commit
to line/lich
that referenced
this pull request
Jan 10, 2020
FastServiceLoader has been fixed in Kotlin Coroutines 1.3.3, so we no longer need to disable it. cf. Kotlin/kotlinx.coroutines#1572
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ServiceLoaderhas proven itself as a very unstable mechanism on Android (see all the issues in aloadMainDispatcherFactorycomment). Moreover, failures in SL are considered bykotlinx.coroutinesusers as errors in the library itself, making it look unstable. We are not really happy with that, so here is an attempt to avoidServiceLoaderon Android forDispatchers.Main.Though I am not sure whether this change should be merged in
kotlinx.coroutines. Moreover, I am not sure how exactly to evaluate the potential impact of this change.@wojtek-kalicinski and @JakeWharton could you please help with reviewing this change first?
Fixes #1557
Fixes #878