understanding leaks in android MVP











up vote
0
down vote

favorite
2












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










share|improve this question


























    up vote
    0
    down vote

    favorite
    2












    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










    share|improve this question
























      up vote
      0
      down vote

      favorite
      2









      up vote
      0
      down vote

      favorite
      2






      2





      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










      share|improve this question













      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







      java android memory-leaks dagger-2 mvp






      share|improve this question













      share|improve this question











      share|improve this question




      share|improve this question










      asked Nov 10 at 19:13









      HukeLau_DABA

      1,19842144




      1,19842144
























          1 Answer
          1






          active

          oldest

          votes

















          up vote
          0
          down vote



          accepted










          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.





          1. 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.





          1. 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.





          1. 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).





          1. 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!)






          share|improve this answer























          • Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
            – HukeLau_DABA
            Nov 11 at 19:13










          • I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
            – Mark Keen
            Nov 11 at 21:02











          Your Answer






          StackExchange.ifUsing("editor", function () {
          StackExchange.using("externalEditor", function () {
          StackExchange.using("snippets", function () {
          StackExchange.snippets.init();
          });
          });
          }, "code-snippets");

          StackExchange.ready(function() {
          var channelOptions = {
          tags: "".split(" "),
          id: "1"
          };
          initTagRenderer("".split(" "), "".split(" "), channelOptions);

          StackExchange.using("externalEditor", function() {
          // Have to fire editor after snippets, if snippets enabled
          if (StackExchange.settings.snippets.snippetsEnabled) {
          StackExchange.using("snippets", function() {
          createEditor();
          });
          }
          else {
          createEditor();
          }
          });

          function createEditor() {
          StackExchange.prepareEditor({
          heartbeatType: 'answer',
          convertImagesToLinks: true,
          noModals: true,
          showLowRepImageUploadWarning: true,
          reputationToPostImages: 10,
          bindNavPrevention: true,
          postfix: "",
          imageUploader: {
          brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
          contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
          allowUrls: true
          },
          onDemand: true,
          discardSelector: ".discard-answer"
          ,immediatelyShowMarkdownHelp:true
          });


          }
          });














          draft saved

          draft discarded


















          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53242509%2funderstanding-leaks-in-android-mvp%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown

























          1 Answer
          1






          active

          oldest

          votes








          1 Answer
          1






          active

          oldest

          votes









          active

          oldest

          votes






          active

          oldest

          votes








          up vote
          0
          down vote



          accepted










          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.





          1. 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.





          1. 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.





          1. 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).





          1. 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!)






          share|improve this answer























          • Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
            – HukeLau_DABA
            Nov 11 at 19:13










          • I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
            – Mark Keen
            Nov 11 at 21:02















          up vote
          0
          down vote



          accepted










          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.





          1. 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.





          1. 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.





          1. 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).





          1. 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!)






          share|improve this answer























          • Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
            – HukeLau_DABA
            Nov 11 at 19:13










          • I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
            – Mark Keen
            Nov 11 at 21:02













          up vote
          0
          down vote



          accepted







          up vote
          0
          down vote



          accepted






          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.





          1. 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.





          1. 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.





          1. 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).





          1. 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!)






          share|improve this answer














          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.





          1. 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.





          1. 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.





          1. 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).





          1. 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!)







          share|improve this answer














          share|improve this answer



          share|improve this answer








          edited Nov 11 at 22:35

























          answered Nov 11 at 15:51









          Mark Keen

          4,60941641




          4,60941641












          • Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
            – HukeLau_DABA
            Nov 11 at 19:13










          • I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
            – Mark Keen
            Nov 11 at 21:02


















          • Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
            – HukeLau_DABA
            Nov 11 at 19:13










          • I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
            – Mark Keen
            Nov 11 at 21:02
















          Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
          – HukeLau_DABA
          Nov 11 at 19:13




          Thanks! "It is important to note that all anonymous classes in a Rx chain (usually operators like map, filter etc) that don't reference (capture) the enclosing class are leak free." Suppose we have this in an activity: Observable.just("Hello, world!") .map(new Func1<String, String>() { @Override public String call(String s) { return s + " -Dan"; } }) .subscribe(s -> System.out.println(s)); As an anon class, it holds a reference to the enclosing Activity so isnt there an implicit leak even if we arent dir referencing it?
          – HukeLau_DABA
          Nov 11 at 19:13












          I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
          – Mark Keen
          Nov 11 at 21:02




          I should have clarified that with operators map, filter etc are lambdas, which when non-capturing don't hold an implicit reference - normal inner classes / non lambdas will hold the implicit reference always - I was trying to distinguish between the operators (which tend to be functional interfaces) and subscribers, which in your example is not. Follow up reading if you're interested found on this answer stackoverflow.com/a/28447015/4252352
          – Mark Keen
          Nov 11 at 21:02


















          draft saved

          draft discarded




















































          Thanks for contributing an answer to Stack Overflow!


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          To learn more, see our tips on writing great answers.





          Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


          Please pay close attention to the following guidance:


          • Please be sure to answer the question. Provide details and share your research!

          But avoid



          • Asking for help, clarification, or responding to other answers.

          • Making statements based on opinion; back them up with references or personal experience.


          To learn more, see our tips on writing great answers.




          draft saved


          draft discarded














          StackExchange.ready(
          function () {
          StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53242509%2funderstanding-leaks-in-android-mvp%23new-answer', 'question_page');
          }
          );

          Post as a guest















          Required, but never shown





















































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown

































          Required, but never shown














          Required, but never shown












          Required, but never shown







          Required, but never shown







          Popular posts from this blog

          Guess what letter conforming each word

          Port of Spain

          Run scheduled task as local user group (not BUILTIN)