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

Synthetic MviPresenter generated classes instances not GC cleared after Activity/fragment destruction #343

Open
ruieduardosoares opened this issue Dec 7, 2020 · 12 comments

Comments

@ruieduardosoares
Copy link

Mosby Version: 3.1.1

Expected behavior

MviPresenter$classes instances should be garbage collected after leaving fragment and its activity host and not bound to any INSTANCE object

Actual behavior (include a stacktrace if crash)
No crash associated.

The problem is that the generated synthetic classes instances in presenter are somehow bounded to an INSTANCE that is not garbage collected after leaving the screen despite several attempts to force GC on Android profiler.

Most certain the synthetic classes can probably be the anonymous functions in the flatMap calls

See
image
image

Snippet of source code of presenter

image

This presenter lifecycle is being handled by mosby and not reference nor it is being passed to any other component outside the Fragment it belongs to.

The TakePhotoInteractor instance is created on demand also

Steps to reproduce the behavior or link to a sample repository

Just implement this library and follow http://hannesdorfmann.com/android/mosby3-mvi-2

Doesnt mosby disposes that observables created from the view intents?

Any thoughts about this?

@sockeqwe
Copy link
Owner

sockeqwe commented Dec 8, 2020

Hm, need to check this more in detail but that issue is new or it broke because of missing androidx compatability.

@ruieduardosoares
Copy link
Author

ruieduardosoares commented Dec 8, 2020

Not sure androidx compatibility is an issue because only the package name were changed from android.support to androidx.

But this is just a guess, I am only aware of what android documentation states

Also I have jetifier enabled so mosby byte code is change to access the correct package from android.support to androidx.

I want to continue to use this library and with a few tweaks I am able to use it with Android Navigation component by only using fragments.

Thanks for the reply.

@ruieduardosoares
Copy link
Author

ruieduardosoares commented Dec 9, 2020

More digging i found out the INSTANCE might be a "constant" referenced somewhere that is the Synthetic class itself

See
image

I just hope this is not kotlin's fault.
Neither mosby, but i still dont know how is this happening.

@ruieduardosoares
Copy link
Author

At first sight this seems to be the View interface object which is the fragment 🤔

image

But this is not possible because the subscription to View is being canceled when fragment is onStop().

And TakePhotoFragment already was GC

image

I can only see companion objects here

And the TakePhotoFragment$Companion doesnt have any reference to any presenter except to a TAG string field
image

@sockeqwe
Copy link
Owner

sockeqwe commented Dec 12, 2020 via email

@ruieduardosoares
Copy link
Author

The companion object is the one declared in the TakePhotoFragment, it only has a TAG property to use in logs.

There is no big issue with this companion object

My concern is only those synthetic classes from bindintent() method from the previous screenshots I posted, that have an object allocation for each view method.

Like I said, not sure if is kotlin or mosby the source of this issue, but the profiler is telling that those synthetic classes have one corresponding instance and are using 40 bytes on shallow size and 40 on retained size.

By themselves they are not referencing any other object in the heap, but are stuck there still don't know why.

They must be being referenced somewhere as the GC cannot collect them.

This is some dark magic.

I will need to dig more on this

@ruieduardosoares
Copy link
Author

Checked kotlin issue tracker and found this.

@ruieduardosoares
Copy link
Author

Instead of using lambdas with SAM conversion, I will use object expression for the intent() ViewIntentBinder and check what happens

@ruieduardosoares
Copy link
Author

My suspicions are true after all.

Converted all Lambda SAM conversions into object expressions making the code a bit ugly and larger
(Changed Android studio Theme BTW)
image

But after GC all previous synthetic classes instances are no longer allocated 😕

image

So this seems to by a kotlin issue rather than mosby.

@ruieduardosoares
Copy link
Author

Now, to reduce the code size i will try to use Anonymous functions instead of Object Expressions and see if this also works

@ruieduardosoares
Copy link
Author

And their are back with Anonymous functions 😢

image

image

@ruieduardosoares
Copy link
Author

ruieduardosoares commented Dec 14, 2020

Never mind

image

The issue continue....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants