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

sharedUserId breaks Compose Resources on Android #4742

Closed
garrison-henkle opened this issue May 1, 2024 · 2 comments · Fixed by #4895
Closed

sharedUserId breaks Compose Resources on Android #4742

garrison-henkle opened this issue May 1, 2024 · 2 comments · Fixed by #4895
Assignees
Labels

Comments

@garrison-henkle
Copy link

garrison-henkle commented May 1, 2024

Describe the bug
Compose Resources fails to load resources on Android when the application is using a sharedUserId

Affected platforms

  • Android

Versions

  • Kotlin version*: 1.9.23
  • Compose Multiplatform version*: 1.6.2
  • OS version(s)*: tested on Android 13 and 14, but likely affects all versions
  • OS architecture (x86 or arm64): tested on arm64, but likely affects x86 as well
  • Device (model or simulator for iOS issues): OnePlus 7 Pro (GM1917) and Samsung Galaxy s24 Ultra (SM-S928U1)
  • JDK (for desktop issues): N/A

To Reproduce
Minimal reproducer using kmp.jetbrains.com's Android template:
https://github.com/garrison-henkle/composeResourcesSharedUserIdBug

Stacktrace is also in the above repo's README.md

Steps:

  1. Launch the application with the sharedUserId attribute in the Manifest.xml.

Expected behavior
The app is able to load resources regardless of whether it uses a sharedUserId or not

Screenshots
N/A

Additional context
I maintain an application that has unfortunately inherited a sharedUserId. My team has been building out a replacement app using Compose Mutliplatform, and we recently added in the sharedUserId to prepare it for submission to the Play Store. The addition of the sharedUserId prevents the app from loading any images or strings (fonts load fine because the underlying implementation delegates to Android's asset loader).

I investigated the issue and narrowed it down to the class loader. Removing Thread.currentThread().contextClassLoader from ResourceReader.android.kt seemed to fix the issue for me:

// Before (crashes)
private fun getClassLoader(): ClassLoader {
    return Thread.currentThread().contextClassLoader ?: this.javaClass.classLoader!!
}

// After (runs)
private object ResourceReader
private fun getClassLoader(): ClassLoader {
    return ResourceReader.javaClass.classLoader!!
}

I know essentially nothing about class loaders, so I don't know the implications of just removing the Thread's context class loader here. Hopefully the above hack at least demonstrates what's causing the issue 😅

@igordmn
Copy link
Collaborator

igordmn commented May 8, 2024

Thanks!

@terrakok, it is reproduced on the provided project. sharedUserId is deprecated, but it isn't possible to migrate old projects that needs to be maintained. Can we use the suggested fix?

@terrakok
Copy link
Collaborator

The classloader logic was implemented by intention here: #2490

I will investigate it more because it seems suspicious

terrakok added a commit that referenced this issue May 29, 2024
…argets (#4895)

The class loader retrieval method has been modified in both
`ResourceReader.android.kt` and `ResourceReader.desktop.kt` files. The
return statement has been changed to prioritize java class classLoader
and provides a clearer error message when it can't be found.

Fixes #4887
Fixes #4742

## Release Notes
### Fixes - Resources
- Delete contextClassLoader usage on JVM targets
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants