From 034e76dd5cd47872989e508109ee96cde2cbe998 Mon Sep 17 00:00:00 2001 From: Torsten Grote Date: Mon, 28 Oct 2019 16:32:55 -0300 Subject: [PATCH] [android] Fix controller memory leaks --- .../briarproject/briar/android/blog/BaseController.java | 3 --- .../briar/android/blog/BaseControllerImpl.java | 9 --------- .../briarproject/briar/android/blog/BlogController.java | 3 ++- .../briar/android/blog/BlogControllerImpl.java | 9 ++++++--- .../briarproject/briar/android/blog/BlogFragment.java | 8 +++++++- .../briar/android/blog/BlogPostFragment.java | 1 - .../briarproject/briar/android/blog/FeedController.java | 3 ++- .../briar/android/blog/FeedControllerImpl.java | 7 +++++-- .../briarproject/briar/android/blog/FeedFragment.java | 8 +++++++- .../android/privategroup/list/GroupListController.java | 4 +++- .../privategroup/list/GroupListControllerImpl.java | 5 ++++- .../android/privategroup/list/GroupListFragment.java | 6 ++++++ 12 files changed, 42 insertions(+), 24 deletions(-) diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseController.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseController.java index d204c7f21..ad7bbdc23 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseController.java @@ -35,9 +35,6 @@ interface BaseController { void repeatPost(BlogPostItem item, @Nullable String comment, ExceptionHandler handler); - @UiThread - void setBlogListener(BlogListener listener); - @NotNullByDefault interface BlogListener { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseControllerImpl.java index a287df57f..036fe9ad2 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseControllerImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BaseControllerImpl.java @@ -55,8 +55,6 @@ abstract class BaseControllerImpl extends DbControllerImpl private final Map headerCache = new ConcurrentHashMap<>(); - private volatile BlogListener listener; - BaseControllerImpl(@DatabaseExecutor Executor dbExecutor, LifecycleManager lifecycleManager, EventBus eventBus, AndroidNotificationManager notificationManager, @@ -71,7 +69,6 @@ abstract class BaseControllerImpl extends DbControllerImpl @Override @CallSuper public void onStart() { - if (listener == null) throw new IllegalStateException(); eventBus.addListener(this); } @@ -81,11 +78,6 @@ abstract class BaseControllerImpl extends DbControllerImpl eventBus.removeListener(this); } - @Override - public void setBlogListener(BlogListener listener) { - this.listener = listener; - } - @Override public void loadBlogPosts(GroupId groupId, ResultExceptionHandler, DbException> handler) { @@ -211,7 +203,6 @@ abstract class BaseControllerImpl extends DbControllerImpl text = HtmlUtils.clean(blogManager.getPostText(m), ARTICLE); textCache.put(m, text); } - //noinspection ConstantConditions return text; } diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogController.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogController.java index 6c36fc4e6..137f7066d 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogController.java @@ -9,6 +9,7 @@ import org.briarproject.briar.android.controller.handler.ResultExceptionHandler; import java.util.Collection; +import androidx.annotation.Nullable; import androidx.annotation.UiThread; @NotNullByDefault @@ -17,7 +18,7 @@ public interface BlogController extends BaseController { void setGroupId(GroupId g); @UiThread - void setBlogSharingListener(BlogSharingListener listener); + void setBlogSharingListener(@Nullable BlogSharingListener listener); void loadBlogPosts( ResultExceptionHandler, DbException> handler); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java index 519257c8b..c78173c7f 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogControllerImpl.java @@ -35,6 +35,8 @@ import java.util.logging.Logger; import javax.inject.Inject; +import androidx.annotation.Nullable; + import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; @@ -51,6 +53,7 @@ class BlogControllerImpl extends BaseControllerImpl private final BlogSharingManager blogSharingManager; // UI thread + @Nullable private BlogSharingListener listener; private volatile GroupId groupId = null; @@ -93,14 +96,14 @@ class BlogControllerImpl extends BaseControllerImpl } @Override - public void setBlogSharingListener(BlogSharingListener listener) { - super.setBlogListener(listener); + public void setBlogSharingListener(@Nullable BlogSharingListener listener) { this.listener = listener; } @Override public void eventOccurred(Event e) { - if (groupId == null) throw new IllegalStateException(); + if (groupId == null || listener == null) + throw new IllegalStateException(); if (e instanceof BlogPostAddedEvent) { BlogPostAddedEvent b = (BlogPostAddedEvent) e; if (b.getGroupId().equals(groupId)) { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java index 0f933e25e..0a68d8a95 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogFragment.java @@ -33,9 +33,9 @@ import org.briarproject.briar.api.blog.BlogPostHeader; import java.util.Collection; -import javax.annotation.Nullable; import javax.inject.Inject; +import androidx.annotation.Nullable; import androidx.annotation.UiThread; import androidx.appcompat.app.ActionBar; import androidx.appcompat.app.AlertDialog; @@ -138,6 +138,12 @@ public class BlogFragment extends BaseFragment list.stopPeriodicUpdate(); } + @Override + public void onDestroy() { + super.onDestroy(); + blogController.setBlogSharingListener(null); + } + @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java index ee37858a4..25eb5d23b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/BlogPostFragment.java @@ -43,7 +43,6 @@ public class BlogPostFragment extends BasePostFragment implements BlogListener { @Override public void injectFragment(ActivityComponent component) { component.inject(this); - blogController.setBlogListener(this); } @Override diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedController.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedController.java index 6a8cdd7cd..039eb78fd 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedController.java @@ -7,6 +7,7 @@ import org.briarproject.briar.api.blog.Blog; import java.util.Collection; +import androidx.annotation.Nullable; import androidx.annotation.UiThread; @NotNullByDefault @@ -18,7 +19,7 @@ public interface FeedController extends BaseController { void loadPersonalBlog(ResultExceptionHandler handler); @UiThread - void setFeedListener(FeedListener listener); + void setFeedListener(@Nullable FeedListener listener); @NotNullByDefault interface FeedListener extends BlogListener { diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedControllerImpl.java index efe68124e..714fc0525 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedControllerImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedControllerImpl.java @@ -26,6 +26,8 @@ import java.util.logging.Logger; import javax.inject.Inject; +import androidx.annotation.Nullable; + import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logException; @@ -40,6 +42,7 @@ class FeedControllerImpl extends BaseControllerImpl implements FeedController { Logger.getLogger(FeedControllerImpl.class.getName()); // UI thread + @Nullable private FeedListener listener; @Inject @@ -66,13 +69,13 @@ class FeedControllerImpl extends BaseControllerImpl implements FeedController { } @Override - public void setFeedListener(FeedListener listener) { - super.setBlogListener(listener); + public void setFeedListener(@Nullable FeedListener listener) { this.listener = listener; } @Override public void eventOccurred(Event e) { + if (listener == null) throw new IllegalStateException(); if (e instanceof BlogPostAddedEvent) { BlogPostAddedEvent b = (BlogPostAddedEvent) e; LOG.info("Blog post added"); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java index 2896fded4..575448b1b 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/blog/FeedFragment.java @@ -26,9 +26,9 @@ import org.briarproject.briar.api.blog.BlogPostHeader; import java.util.Collection; import java.util.logging.Logger; -import javax.annotation.Nullable; import javax.inject.Inject; +import androidx.annotation.Nullable; import androidx.annotation.UiThread; import androidx.recyclerview.widget.LinearLayoutManager; @@ -131,6 +131,12 @@ public class FeedFragment extends BaseFragment implements // TODO save list position in database/preferences? } + @Override + public void onDestroy() { + super.onDestroy(); + feedController.setFeedListener(null); + } + @Override public void onSaveInstanceState(Bundle outState) { super.onSaveInstanceState(outState); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListController.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListController.java index 5c030c0f0..d4e5c9f99 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListController.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListController.java @@ -10,6 +10,8 @@ import org.briarproject.briar.api.privategroup.GroupMessageHeader; import java.util.Collection; +import javax.annotation.Nullable; + import androidx.annotation.UiThread; @NotNullByDefault @@ -19,7 +21,7 @@ interface GroupListController extends DbController { * The listener must be set right after the controller was injected */ @UiThread - void setGroupListListener(GroupListListener listener); + void setGroupListListener(@Nullable GroupListListener listener); @UiThread void onStart(); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListControllerImpl.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListControllerImpl.java index 75fa033c3..3be2dd123 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListControllerImpl.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListControllerImpl.java @@ -39,6 +39,7 @@ import java.util.logging.Logger; import javax.inject.Inject; import androidx.annotation.CallSuper; +import androidx.annotation.Nullable; import static java.util.logging.Level.WARNING; import static org.briarproject.bramble.util.LogUtils.logDuration; @@ -61,6 +62,7 @@ class GroupListControllerImpl extends DbControllerImpl private final EventBus eventBus; // UI thread + @Nullable private GroupListListener listener; @Inject @@ -78,7 +80,7 @@ class GroupListControllerImpl extends DbControllerImpl } @Override - public void setGroupListListener(GroupListListener listener) { + public void setGroupListListener(@Nullable GroupListListener listener) { this.listener = listener; } @@ -99,6 +101,7 @@ class GroupListControllerImpl extends DbControllerImpl @Override @CallSuper public void eventOccurred(Event e) { + if (listener == null) throw new IllegalStateException(); if (e instanceof GroupMessageAddedEvent) { GroupMessageAddedEvent g = (GroupMessageAddedEvent) e; LOG.info("Private group message added"); diff --git a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java index 1fe13900d..f074be983 100644 --- a/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java +++ b/briar-android/src/main/java/org/briarproject/briar/android/privategroup/list/GroupListFragment.java @@ -109,6 +109,12 @@ public class GroupListFragment extends BaseFragment implements list.showProgressBar(); } + @Override + public void onDestroy() { + super.onDestroy(); + controller.setGroupListListener(null); + } + @Override public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { inflater.inflate(R.menu.groups_list_actions, menu);