Merge branch '1643-controller-leaks' into 'master'

Fix controller memory leaks

Closes #1643

See merge request briar/briar!1184
This commit is contained in:
akwizgran
2019-11-07 16:33:18 +00:00
12 changed files with 42 additions and 24 deletions

View File

@@ -35,9 +35,6 @@ interface BaseController {
void repeatPost(BlogPostItem item, @Nullable String comment, void repeatPost(BlogPostItem item, @Nullable String comment,
ExceptionHandler<DbException> handler); ExceptionHandler<DbException> handler);
@UiThread
void setBlogListener(BlogListener listener);
@NotNullByDefault @NotNullByDefault
interface BlogListener { interface BlogListener {

View File

@@ -55,8 +55,6 @@ abstract class BaseControllerImpl extends DbControllerImpl
private final Map<MessageId, BlogPostHeader> headerCache = private final Map<MessageId, BlogPostHeader> headerCache =
new ConcurrentHashMap<>(); new ConcurrentHashMap<>();
private volatile BlogListener listener;
BaseControllerImpl(@DatabaseExecutor Executor dbExecutor, BaseControllerImpl(@DatabaseExecutor Executor dbExecutor,
LifecycleManager lifecycleManager, EventBus eventBus, LifecycleManager lifecycleManager, EventBus eventBus,
AndroidNotificationManager notificationManager, AndroidNotificationManager notificationManager,
@@ -71,7 +69,6 @@ abstract class BaseControllerImpl extends DbControllerImpl
@Override @Override
@CallSuper @CallSuper
public void onStart() { public void onStart() {
if (listener == null) throw new IllegalStateException();
eventBus.addListener(this); eventBus.addListener(this);
} }
@@ -81,11 +78,6 @@ abstract class BaseControllerImpl extends DbControllerImpl
eventBus.removeListener(this); eventBus.removeListener(this);
} }
@Override
public void setBlogListener(BlogListener listener) {
this.listener = listener;
}
@Override @Override
public void loadBlogPosts(GroupId groupId, public void loadBlogPosts(GroupId groupId,
ResultExceptionHandler<Collection<BlogPostItem>, DbException> handler) { ResultExceptionHandler<Collection<BlogPostItem>, DbException> handler) {
@@ -211,7 +203,6 @@ abstract class BaseControllerImpl extends DbControllerImpl
text = HtmlUtils.clean(blogManager.getPostText(m), ARTICLE); text = HtmlUtils.clean(blogManager.getPostText(m), ARTICLE);
textCache.put(m, text); textCache.put(m, text);
} }
//noinspection ConstantConditions
return text; return text;
} }

View File

@@ -9,6 +9,7 @@ import org.briarproject.briar.android.controller.handler.ResultExceptionHandler;
import java.util.Collection; import java.util.Collection;
import androidx.annotation.Nullable;
import androidx.annotation.UiThread; import androidx.annotation.UiThread;
@NotNullByDefault @NotNullByDefault
@@ -17,7 +18,7 @@ public interface BlogController extends BaseController {
void setGroupId(GroupId g); void setGroupId(GroupId g);
@UiThread @UiThread
void setBlogSharingListener(BlogSharingListener listener); void setBlogSharingListener(@Nullable BlogSharingListener listener);
void loadBlogPosts( void loadBlogPosts(
ResultExceptionHandler<Collection<BlogPostItem>, DbException> handler); ResultExceptionHandler<Collection<BlogPostItem>, DbException> handler);

View File

@@ -35,6 +35,8 @@ import java.util.logging.Logger;
import javax.inject.Inject; import javax.inject.Inject;
import androidx.annotation.Nullable;
import static java.util.logging.Level.WARNING; import static java.util.logging.Level.WARNING;
import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logDuration;
import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.logException;
@@ -51,6 +53,7 @@ class BlogControllerImpl extends BaseControllerImpl
private final BlogSharingManager blogSharingManager; private final BlogSharingManager blogSharingManager;
// UI thread // UI thread
@Nullable
private BlogSharingListener listener; private BlogSharingListener listener;
private volatile GroupId groupId = null; private volatile GroupId groupId = null;
@@ -93,14 +96,14 @@ class BlogControllerImpl extends BaseControllerImpl
} }
@Override @Override
public void setBlogSharingListener(BlogSharingListener listener) { public void setBlogSharingListener(@Nullable BlogSharingListener listener) {
super.setBlogListener(listener);
this.listener = listener; this.listener = listener;
} }
@Override @Override
public void eventOccurred(Event e) { public void eventOccurred(Event e) {
if (groupId == null) throw new IllegalStateException(); if (groupId == null || listener == null)
throw new IllegalStateException();
if (e instanceof BlogPostAddedEvent) { if (e instanceof BlogPostAddedEvent) {
BlogPostAddedEvent b = (BlogPostAddedEvent) e; BlogPostAddedEvent b = (BlogPostAddedEvent) e;
if (b.getGroupId().equals(groupId)) { if (b.getGroupId().equals(groupId)) {

View File

@@ -33,9 +33,9 @@ import org.briarproject.briar.api.blog.BlogPostHeader;
import java.util.Collection; import java.util.Collection;
import javax.annotation.Nullable;
import javax.inject.Inject; import javax.inject.Inject;
import androidx.annotation.Nullable;
import androidx.annotation.UiThread; import androidx.annotation.UiThread;
import androidx.appcompat.app.ActionBar; import androidx.appcompat.app.ActionBar;
import androidx.appcompat.app.AlertDialog; import androidx.appcompat.app.AlertDialog;
@@ -138,6 +138,12 @@ public class BlogFragment extends BaseFragment
list.stopPeriodicUpdate(); list.stopPeriodicUpdate();
} }
@Override
public void onDestroy() {
super.onDestroy();
blogController.setBlogSharingListener(null);
}
@Override @Override
public void onSaveInstanceState(Bundle outState) { public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState); super.onSaveInstanceState(outState);

View File

@@ -43,7 +43,6 @@ public class BlogPostFragment extends BasePostFragment implements BlogListener {
@Override @Override
public void injectFragment(ActivityComponent component) { public void injectFragment(ActivityComponent component) {
component.inject(this); component.inject(this);
blogController.setBlogListener(this);
} }
@Override @Override

View File

@@ -7,6 +7,7 @@ import org.briarproject.briar.api.blog.Blog;
import java.util.Collection; import java.util.Collection;
import androidx.annotation.Nullable;
import androidx.annotation.UiThread; import androidx.annotation.UiThread;
@NotNullByDefault @NotNullByDefault
@@ -18,7 +19,7 @@ public interface FeedController extends BaseController {
void loadPersonalBlog(ResultExceptionHandler<Blog, DbException> handler); void loadPersonalBlog(ResultExceptionHandler<Blog, DbException> handler);
@UiThread @UiThread
void setFeedListener(FeedListener listener); void setFeedListener(@Nullable FeedListener listener);
@NotNullByDefault @NotNullByDefault
interface FeedListener extends BlogListener { interface FeedListener extends BlogListener {

View File

@@ -26,6 +26,8 @@ import java.util.logging.Logger;
import javax.inject.Inject; import javax.inject.Inject;
import androidx.annotation.Nullable;
import static java.util.logging.Level.WARNING; import static java.util.logging.Level.WARNING;
import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logDuration;
import static org.briarproject.bramble.util.LogUtils.logException; import static org.briarproject.bramble.util.LogUtils.logException;
@@ -40,6 +42,7 @@ class FeedControllerImpl extends BaseControllerImpl implements FeedController {
Logger.getLogger(FeedControllerImpl.class.getName()); Logger.getLogger(FeedControllerImpl.class.getName());
// UI thread // UI thread
@Nullable
private FeedListener listener; private FeedListener listener;
@Inject @Inject
@@ -66,13 +69,13 @@ class FeedControllerImpl extends BaseControllerImpl implements FeedController {
} }
@Override @Override
public void setFeedListener(FeedListener listener) { public void setFeedListener(@Nullable FeedListener listener) {
super.setBlogListener(listener);
this.listener = listener; this.listener = listener;
} }
@Override @Override
public void eventOccurred(Event e) { public void eventOccurred(Event e) {
if (listener == null) throw new IllegalStateException();
if (e instanceof BlogPostAddedEvent) { if (e instanceof BlogPostAddedEvent) {
BlogPostAddedEvent b = (BlogPostAddedEvent) e; BlogPostAddedEvent b = (BlogPostAddedEvent) e;
LOG.info("Blog post added"); LOG.info("Blog post added");

View File

@@ -26,9 +26,9 @@ import org.briarproject.briar.api.blog.BlogPostHeader;
import java.util.Collection; import java.util.Collection;
import java.util.logging.Logger; import java.util.logging.Logger;
import javax.annotation.Nullable;
import javax.inject.Inject; import javax.inject.Inject;
import androidx.annotation.Nullable;
import androidx.annotation.UiThread; import androidx.annotation.UiThread;
import androidx.recyclerview.widget.LinearLayoutManager; import androidx.recyclerview.widget.LinearLayoutManager;
@@ -131,6 +131,12 @@ public class FeedFragment extends BaseFragment implements
// TODO save list position in database/preferences? // TODO save list position in database/preferences?
} }
@Override
public void onDestroy() {
super.onDestroy();
feedController.setFeedListener(null);
}
@Override @Override
public void onSaveInstanceState(Bundle outState) { public void onSaveInstanceState(Bundle outState) {
super.onSaveInstanceState(outState); super.onSaveInstanceState(outState);

View File

@@ -10,6 +10,8 @@ import org.briarproject.briar.api.privategroup.GroupMessageHeader;
import java.util.Collection; import java.util.Collection;
import javax.annotation.Nullable;
import androidx.annotation.UiThread; import androidx.annotation.UiThread;
@NotNullByDefault @NotNullByDefault
@@ -19,7 +21,7 @@ interface GroupListController extends DbController {
* The listener must be set right after the controller was injected * The listener must be set right after the controller was injected
*/ */
@UiThread @UiThread
void setGroupListListener(GroupListListener listener); void setGroupListListener(@Nullable GroupListListener listener);
@UiThread @UiThread
void onStart(); void onStart();

View File

@@ -39,6 +39,7 @@ import java.util.logging.Logger;
import javax.inject.Inject; import javax.inject.Inject;
import androidx.annotation.CallSuper; import androidx.annotation.CallSuper;
import androidx.annotation.Nullable;
import static java.util.logging.Level.WARNING; import static java.util.logging.Level.WARNING;
import static org.briarproject.bramble.util.LogUtils.logDuration; import static org.briarproject.bramble.util.LogUtils.logDuration;
@@ -61,6 +62,7 @@ class GroupListControllerImpl extends DbControllerImpl
private final EventBus eventBus; private final EventBus eventBus;
// UI thread // UI thread
@Nullable
private GroupListListener listener; private GroupListListener listener;
@Inject @Inject
@@ -78,7 +80,7 @@ class GroupListControllerImpl extends DbControllerImpl
} }
@Override @Override
public void setGroupListListener(GroupListListener listener) { public void setGroupListListener(@Nullable GroupListListener listener) {
this.listener = listener; this.listener = listener;
} }
@@ -99,6 +101,7 @@ class GroupListControllerImpl extends DbControllerImpl
@Override @Override
@CallSuper @CallSuper
public void eventOccurred(Event e) { public void eventOccurred(Event e) {
if (listener == null) throw new IllegalStateException();
if (e instanceof GroupMessageAddedEvent) { if (e instanceof GroupMessageAddedEvent) {
GroupMessageAddedEvent g = (GroupMessageAddedEvent) e; GroupMessageAddedEvent g = (GroupMessageAddedEvent) e;
LOG.info("Private group message added"); LOG.info("Private group message added");

View File

@@ -109,6 +109,12 @@ public class GroupListFragment extends BaseFragment implements
list.showProgressBar(); list.showProgressBar();
} }
@Override
public void onDestroy() {
super.onDestroy();
controller.setGroupListListener(null);
}
@Override @Override
public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) { public void onCreateOptionsMenu(Menu menu, MenuInflater inflater) {
inflater.inflate(R.menu.groups_list_actions, menu); inflater.inflate(R.menu.groups_list_actions, menu);