AGP KMP breaks Gradle metadata for KMP projects that target JVM [396184120]

Fixed

Bug

Status Update

No update yet.

Description

da...@google.com

created issue #1

Feb 12, 2025 09:13PM

Applying the newer AGP KMP plugin to androidx.sqlite breaks the Gradle metadata such that a JVM project depending on the artifact will attempt (and fail) to use the android artifact instead of the JVM one.

This was originally reported in SQLite KMP b/396148592

Here is a diff on the metadata, left is before AGP KMP is used (i.e. after reverting the changes in this CL) and right is with AGP KMP applied: https://diff.googleplex.com/#key=EY2FqaJWQbUQ

Let me know if you need a sample project, but if you create a new JVM or Kotlin project and depend on androidx.sqlite:sqlite:2.5.0-beta01 and try to use classes from it, it will fail at runtime with a class not found exception. When inspecting the deps via :dependencies you will notice it attempts to use the -android artifact instead of the -jvm one:

runtimeClasspath - Runtime classpath of 'main'.
+--- org.jetbrains.kotlin:kotlin-stdlib:2.1.10
|    \--- org.jetbrains:annotations:13.0
\--- androidx.sqlite:sqlite:2.5.0-beta01
     +--- androidx.annotation:annotation:1.8.1
     |    \--- androidx.annotation:annotation-jvm:1.8.1
     |         \--- org.jetbrains.kotlin:kotlin-stdlib:1.7.10 -> 2.1.10 (*)
     +--- androidx.sqlite:sqlite-android:2.5.0-beta01
     |    +--- androidx.annotation:annotation:1.8.1 (*)
     |    +--- org.jetbrains.kotlin:kotlin-stdlib -> 2.1.10 (*)
     |    \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.10 (c)
     \--- org.jetbrains.kotlin:kotlin-stdlib:2.1.10 (*)

Comments

au...@google.com <au...@google.com> #2Feb 12, 2025 09:28PM

Thank you for reporting this issue. For us to further investigate this issue, please provide the following additional information:

Steps to reproduce
Please provide source code or apk of a sample application to reproduce the issue.
Also kindly mention the steps to be followed for reproducing the issue with the given sample application.

Expected output
What is the expected output?

Current output
What is the current output?

da...@google.com <da...@google.com> #3Feb 12, 2025 09:34PM

Seeing this with a FragmentPagerAdapter as well. Our app has three static tabs, and opens by default on tab 1. Previously, the fragments for tab 2 and tab 3 would have their onResume() called when the activity's onResume() was called, even though those tabs weren't visible yet.

Now, since 27.1.0, onResume() on those tabs' fragments is only called when the user navigates to them.

au...@google.com <au...@google.com> #4Feb 12, 2025 09:39PM

#3 can you try to provide a repro app? I've lost 4 hours already and can't find the root cause, the basic AS sample does not produce that :(
Maybe it's easier from your code.

da...@google.com <da...@google.com> #5Feb 12, 2025 09:45PM

I also tried with the sample app (

https://github.com/googlesamples/android-HorizontalPaging) which doesn't have the issue.

I did find a clue in my app though: the first tab's fragment does getLoaderManager().initLoader() in its onActivityCreated(). If I comment this out, the onResume() methods of all the fragments are called at once as with support lib 27.0.2.

This might be related to this change in the support lib release notes:
> The underlying implementation of Loaders has been rewritten to use Lifecycle.

It's going to be quite a bit of work to get this into a small sample app though. I can't share my own app.

Message last modified on Feb 12, 2025 09:46PM

sqlitedrivertest.zip

182 KB

Download

da...@google.com <da...@google.com> #6Feb 12, 2025 09:56PM

Thanks I can also confirm the same thing in my case.

Removing the call to getLoaderManager().restartLoader(id, null, this); from onResume of the fragment then allows proper resuming of all fragments. (In my case all loaders Id are different).

This give a clue at how to reproduce, will see if I can tonight.

hm...@google.com <hm...@google.com> #7Feb 13, 2025 10:15AM

A sample app is here:

https://github.com/calvarez-ov/android-HorizontalPaging

If you build it as-is, you see three onResume() logs at once, when the app is launched:

02-28 14:03:43.534 15309 15309 V MainActivity/DummySectionFragment: onResume 1
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 2
02-28 14:03:43.682 15309 15309 V MainActivity/DummySectionFragment: onResume 3

If you change it to use support library 27.1.0 (Application/build.gradle), you only see the first onResume(). You see the other ones after you manually click on the tabs:

02-28 14:07:15.491 15443 15443 V MainActivity/DummySectionFragment: onResume 1
...
02-28 14:07:21.504 15443 15443 V MainActivity/DummySectionFragment: onResume 3
...
02-28 14:07:25.119 15443 15443 V MainActivity/DummySectionFragment: onResume 2

Note, this sample app doesn't look anything like the real app which has the same problem :) It's just the smallest example I could make to reproduce the issue. There's no ListView or RecyclerView and no adapter to display the loader data. I explicitly call forceLoad() to make the loader's loadInBackground() be called. If I don't do this, I have the bug even with support lib 27.0.2.

Message last modified on Feb 13, 2025 12:58PM

hm...@google.com <hm...@google.com> #8Feb 13, 2025 01:15PM

In case this helps troubleshooting: if you post the restartLoader() (or initLoader()) call to a Handler, the bug goes away:

@Override
public void onActivityCreated(@Nullable Bundle savedInstanceState) {
super.onActivityCreated(savedInstanceState);
- getLoaderManager().restartLoader(1337, null, this);
+ new Handler().post(new Runnable() {
+ @Override
+ public void run() {
+ getLoaderManager().restartLoader(1337, null, FragmentWithLoader.this);
+ }
+ });
}

hm...@google.com <hm...@google.com> #9Feb 13, 2025 01:46PM

Actually from Ian Lake:

https://plus.google.com/u/0/+IanLake/posts/UNXxBrsBUzJ?cfem=1 This is a normal behavior from before that seems more visible now.

What you can test in the loaderinbackground is a long thread sleep the issue should be back even when running from the handler.

I guess the root issue is that before the loader was marked activated for a very very short amount of time (the actual loading so a few ms for most) and now it's marked active for a way longer time leading to the issue. Or maybe for the same amount of time but the loading occurs exactly at the time the fragment manager check for it before loading the others.

Won't be something easy to change I fear :(

da...@google.com <da...@google.com> #10Feb 13, 2025 02:03PM

There was pre-existing coupling between Loaders and FragmentManager:
1. FragmentManager wouldn't start new Fragments with userVisibleHint=false if current Fragments had running Loaders
2. LoaderManager would directly contact FragmentManager upon Loader completion to start any of those pending Fragments

With 27.1.0, #2 was removed, but #1 remained. This mismatch causes the issue.

While both removing #1 and re-instating #2 are options, we are going to proceed with removing #1 as that continues the goal to separate Loaders from the other APIs (it is not intuitive that adding Loaders affects the Fragment's lifecycle) and bring Loaders more in line with other APIs (such as ViewModels+LiveData).

da...@google.com <da...@google.com> #11Feb 13, 2025 07:22PM

Thanks. Do you think it might worth extending the API to allow anyone who want that behavior for performance reasons to continue?

Like a way to query if there's active loaders for visible fragments and a listener for their end? With that and the already existing isVisibleToUser this would allow anyone to keep that.

Or maybe even better, to be more generic a busy indicator for visible fragments, with a listener when nothing busy. To handle that even with normal viewmodels. (Then it would be up to the user to manually toggle that in oncreate / onfinished)
I know one can use a viewmodel + livedata tied to activity to handle that, but native support for that, to allow anyone to easily priorise the visible fragment with an easy way to do something when the visible fragment have done it's delayed init would be really a nice addition.
It's tons of boiler plate code to achieve that currently and prone to mistakes.

Message last modified on Feb 13, 2025 07:23PM

ow...@google.com <ow...@google.com> Feb 18, 2025 05:47PM

Reassigned to hm...@google.com.

hm...@google.com <hm...@google.com> #12Feb 26, 2025 12:40PM

As AsyncTaskLoader already limits its upper bound of running threads based on the number of cores (see

https://developer.android.com/reference/android/os/AsyncTask.html#THREAD_POOL_EXECUTOR), much of what you are requesting is done already when it comes to Loaders.

Capping the number of threads / concurrent operations should be something your repository/backend layer should already try to take into account (for example, Room already does this).

Please file separate feature requests for new API proposals.

hm...@google.com <hm...@google.com> #13Mar 3, 2025 10:25AM

Reassigned to da...@google.com.

As a note: one possible workaround is to move your initLoader call to onStart() - as Loaders do no work until onStart anyways, this is functionally equivalent to doing it in onActivityCreated().

It should also be noted that calling restartLoader in any lifecycle method is almost always the wrong choice as that negates the whole purpose of Loaders (restartLoader effectively throws away any results that were previously computed). restartLoader should only be called when your arguments to the Loader have changed.

da...@google.com <da...@google.com> Mar 3, 2025 01:42PM

Marked as fixed.

au...@google.com <au...@google.com> #14Mar 4, 2025 05:12PM

Well you missunderstood it's about the current functionality that will be lost.

Currently active fragment will load, then finish it's load then other offscreen fragments will start their loading. This is not related to threads limits or anything, this is being sure active fragment have more ressources and load before the unseen ones.

If you remove #1 then all fragments will start at the same time, meaning all loaders too, without any guaranteed order. Imagine a 1 core device (or multiple cores but multiple loaders per fragment) then without luck the offscreen fragments will load before the main that will be delayed until a thread is free.

Providing an API to allow users to keep that way of acting for low end devices or heavy / long loaders should be thought before removing #1. I was just proposing a more global approach as alternative, but at least there's need to have something that ensure active fragments loaders start first to avoid problems.