Nrthugu

I've read some similar questions on here but due to the lack of code presented I'm not certain my question describes the same scenarios.

I hope that the following snippets and questions will help others clarify what and when something is leaked in this MVP implementation: https://github.com/frogermcs/GithubClient/tree/1bf53a2a36c8a85435e877847b987395e482ab4a

BaseActivity.java:

public abstract class BaseActivity extends AppCompatActivity {



    @Override

    protected void onCreate(Bundle savedInstanceState) {

        super.onCreate(savedInstanceState);

        setupActivityComponent();

    }



    protected abstract void setupActivityComponent();

}

SplashActivityModule.java:

@Module

public class SplashActivityModule {

    private SplashActivity splashActivity;



    public SplashActivityModule(SplashActivity splashActivity) {

        this.splashActivity = splashActivity;

    }



    @Provides

    @ActivityScope

    SplashActivity provideSplashActivity() {

        return splashActivity;

    }



    @Provides

    @ActivityScope

    SplashActivityPresenter

    provideSplashActivityPresenter(Validator validator, UserManager 

    userManager, HeavyLibraryWrapper heavyLibraryWrapper) {

        return new SplashActivityPresenter(splashActivity, validator, 

                                           userManager, heavyLibraryWrapper);

    }

}

SplashActivityPresenter is injected inside SplashActivity.java:

public class SplashActivity extends BaseActivity {

    ...



    @Inject

    SplashActivityPresenter presenter;



    @Override

    protected void setupActivityComponent() {

        GithubClientApplication.get(this)

                .getAppComponent()

                .plus(new SplashActivityModule(this))

                .inject(this);

    }

SplashActivityPresenter.java:

public class SplashActivityPresenter {

    public String username;



    private SplashActivity splashActivity;

    private Validator validator;

    private UserManager userManager;

    private HeavyLibraryWrapper heavyLibraryWrapper;



    public SplashActivityPresenter(SplashActivity splashActivity, 

        Validator validator, UserManager userManager, 

        HeavyLibraryWrapper heavyLibraryWrapper) {

        this.splashActivity = splashActivity;

        this.validator = validator;

        this.userManager = userManager;

            this.heavyLibraryWrapper = heavyLibraryWrapper;



        //This calls should be delivered to ExternalLibrary right after it will be initialized

        this.heavyLibraryWrapper.callMethod();

        this.heavyLibraryWrapper.callMethod();

        this.heavyLibraryWrapper.callMethod();

        this.heavyLibraryWrapper.callMethod();

    }



    public void onShowRepositoriesClick() {

        if (validator.validUsername(username)) {

            splashActivity.showLoading(true);

            userManager.getUser(username).subscribe(new 

SimpleObserver<User>() {

                @Override

                public void onNext(User user) {

                    splashActivity.showLoading(false);

                    splashActivity.showRepositoriesListForUser(user);

                }



                @Override

                public void onError(Throwable e) {

                    splashActivity.showLoading(false);

                    splashActivity.showValidationError();

                }

            });

        } else {

            splashActivity.showValidationError();

        }

    }

}

1. If the user rotates the screen while the username is being fetched we are leaking the activity instance being referenced inside the observer's callbacks since the activity is destroyed.


2. If the user rotates the screen without an in-progress fetch, the activity instance is not leaked.


3. In order to fix this leak(1), we need to store the subscription and unsubscribe it in presenter.onDestroy() (called from SplashActivity onDestroy()).


4. Someone told me that doing (3) is not enough, and that inside onDestroy() we must also set the activity instance to null. I disagree because unsubscribing the subscription cancels the request, preventing the callbacks(like onNext(User)) that reference the activity from being invoked.


5. He also told me that while (3) and (4) prevent the ACTIVITY leaking, the PRESENTER is leaked during rotation ALSO since the activity references it. I disagree because a new presenter is created per rotation and initialized to the injected presenter when BaseActivity onCreate invokes setupActivityComponent. The old presenter is automatically garbage collected as a member of SplashActivity.

Can someone respond to the points I outlined above so I can confirm my understanding or learn where I may be wrong? Thank you

I'll do my best to answer these as accurately as possible (welcome edits if inaccurate), but this answer is good reading: https://stackoverflow.com/a/10968689/4252352

1. If the user rotates the screen while the username is being fetched we are leaking the activity instance being referenced inside the
   observer's callbacks since the activity is destroyed.

A : This is correct, however will be short term memory leak until the resources are cleared after dispose. However this should not be relied upon, if you had a Flowable/Observable it may never dispose and clear resources. It is important to note that all lambdas in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free.

2. If the user rotates the screen without an in-progress fetch, the activity instance is not leaked.

A. Correct, you never have an active subscription.

3. In order to fix this leak(1), we need to store the subscription and unsubscribe it in presenter.onDestroy() (called from SplashActivity
   onDestroy())

A This should stop the issue. However a better approach, and in MVP the View should be an abstraction/interface and your Presenter should have entry and exit points for the view and not on the constructor i.e bind(view : View) and unbind() <-- cleanup here, a presenter should not be aware of specific android hook callbacks. This has massive advantages, not only from a OOP (program to interfaces not implementations) aspect, but also testing.

4. Someone told me that doing (3) is not enough, and that inside onDestroy() we must also set the activity instance to null. I disagree
   because unsubscribing the subscription cancels the request, preventing
   the callbacks(like onNext(User)) that reference the activity from
   being invoked.

A. I'd would firstly ask then to clarify their reasoning. As your Presenter is scoped to the Activity (they both have the same lifecycle) unsubscribing should be enough. However if your Presenter has a longer lifecycle than the Activity it would be necessary to remove the reference (this may have been the rationale of the person who you spoke to).

5. He also told me that while (3) and (4) prevent the ACTIVITY leaking, the PRESENTER is leaked during rotation ALSO since the
   activity references it. I disagree because a new presenter is created
   per rotation and initialized to the injected presenter when
   BaseActivity onCreate invokes setupActivityComponent. The old
   presenter is automatically garbage collected as a member of
   SplashActivity.

A. The Presenter is leaked, if the Activity is leaked (along with everything the activity references!)