Avoid leaking Activities with Listeners
suggest changeIf you implement or create a listener in an Activity, always pay attention to the lifecycle of the object that has the listener registered.
Consider an application where we have several different activities/fragments interested in when a user is logged in or out. One way of doing this would be to have a singleton instance of a UserController that can be subscribed to in order to get notified when the state of the user changes:
public class UserController {
private static UserController instance;
private List<StateListener> listeners;
public static synchronized UserController getInstance() {
if (instance == null) {
instance = new UserController();
}
return instance;
}
private UserController() {
// Init
}
public void registerUserStateChangeListener(StateListener listener) {
listeners.add(listener);
}
public void logout() {
for (StateListener listener : listeners) {
listener.userLoggedOut();
}
}
public void login() {
for (StateListener listener : listeners) {
listener.userLoggedIn();
}
}
public interface StateListener {
void userLoggedIn();
void userLoggedOut();
}
}Then there are two activities, SignInActivity:
public class SignInActivity extends Activity implements UserController.StateListener{
UserController userController;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
this.userController = UserController.getInstance();
this.userController.registerUserStateChangeListener(this);
}
@Override
public void userLoggedIn() {
startMainActivity();
}
@Override
public void userLoggedOut() {
showLoginForm();
}
...
public void onLoginClicked(View v) {
userController.login();
}
}And MainActivity:
public class MainActivity extends Activity implements UserController.StateListener{
UserController userController;
@Override
protected void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
this.userController = UserController.getInstance();
this.userController.registerUserStateChangeListener(this);
}
@Override
public void userLoggedIn() {
showUserAccount();
}
@Override
public void userLoggedOut() {
finish();
}
...
public void onLogoutClicked(View v) {
userController.logout();
}
}What happens with this example is that every time the user logs in and then logs out again, a MainActivity instance is leaked. The leak occurs because there is a reference to the activity in UserController#listeners.
Please note: Even if we use an anonymous inner class as a listener, the activity would still leak:
...
this.userController.registerUserStateChangeListener(new UserController.StateListener() {
@Override
public void userLoggedIn() {
showUserAccount();
}
@Override
public void userLoggedOut() {
finish();
}
});
...The activity would still leak, because the anonymous inner class has an implicit reference to the outer class (in this case the activity). This is why it is possible to call instance methods in the outer class from the inner class. In fact, the only type of inner classes that do not have a reference to the outer class are static inner classes.
In short, all instances of non-static inner classes hold an implicit reference to the instance of the outer class that created them.
There are two main approaches to solving this, either by adding a method to remove a listener from UserController#listeners or using a WeakReference to hold the reference of the listeners.
Alternative 1: Removing listeners
Let us start by creating a new method removeUserStateChangeListener(StateListener listener):
public class UserController {
...
public void registerUserStateChangeListener(StateListener listener) {
listeners.add(listener);
}
public void removeUserStateChangeListener(StateListener listener) {
listeners.remove(listener);
}
...
}Then let us call this method in the activity’s onDestroy method:
public class MainActivity extends Activity implements UserController.StateListener{
...
@Override
protected void onDestroy() {
super.onDestroy();
userController.removeUserStateChangeListener(this);
}
}With this modification the instances of MainActivity are no longer leaked when the user logs in and out. However, if the documentation isn’t clear, chances are that the next developer that starts using UserController might miss that it is required to unregister the listener when the activity is destroyed, which leads us to the second method of avoiding these types of leaks.
Alternative 2: Using weak references
First off, let us start by explaining what a weak reference is. A weak reference, as the name suggests, holds a weak reference to an object. Compared to a normal instance field, which is a strong reference, a weak references does not stop the garbage collector, GC, from removing the objects. In the example above this would allow MainActivity to be garbage-collected after it has been destroyed if the UserController used WeakReference to the reference the listeners.
In short, a weak reference is telling the GC that if no one else has a strong reference to this object, go ahead and remove it.
Let us modify the UserController to use a list of WeakReference to keep track of it’s listeners:
public class UserController {
...
private List<WeakReference<StateListener>> listeners;
...
public void registerUserStateChangeListener(StateListener listener) {
listeners.add(new WeakReference<>(listener));
}
public void removeUserStateChangeListener(StateListener listenerToRemove) {
WeakReference referencesToRemove = null;
for (WeakReference<StateListener> listenerRef : listeners) {
StateListener listener = listenerRef.get();
if (listener != null && listener == listenerToRemove) {
referencesToRemove = listenerRef;
break;
}
}
listeners.remove(referencesToRemove);
}
public void logout() {
List referencesToRemove = new LinkedList();
for (WeakReference<StateListener> listenerRef : listeners) {
StateListener listener = listenerRef.get();
if (listener != null) {
listener.userLoggedOut();
} else {
referencesToRemove.add(listenerRef);
}
}
}
public void login() {
List referencesToRemove = new LinkedList();
for (WeakReference<StateListener> listenerRef : listeners) {
StateListener listener = listenerRef.get();
if (listener != null) {
listener.userLoggedIn();
} else {
referencesToRemove.add(listenerRef);
}
}
}
...
}With this modification it doesn’t matter whether or not the listeners are removed, since UserController holds no strong references to any of the listeners. However, writing this boilerplate code every time is cumbersome. Therefore, let us create a generic class called WeakCollection:
public class WeakCollection<T> {
private LinkedList<WeakReference<T>> list;
public WeakCollection() {
this.list = new LinkedList<>();
}
public void put(T item){
//Make sure that we don't re add an item if we already have the reference.
List<T> currentList = get();
for(T oldItem : currentList){
if(item == oldItem){
return;
}
}
list.add(new WeakReference<T>(item));
}
public List<T> get() {
List<T> ret = new ArrayList<>(list.size());
List<WeakReference<T>> itemsToRemove = new LinkedList<>();
for (WeakReference<T> ref : list) {
T item = ref.get();
if (item == null) {
itemsToRemove.add(ref);
} else {
ret.add(item);
}
}
for (WeakReference ref : itemsToRemove) {
this.list.remove(ref);
}
return ret;
}
public void remove(T listener) {
WeakReference<T> refToRemove = null;
for (WeakReference<T> ref : list) {
T item = ref.get();
if (item == listener) {
refToRemove = ref;
}
}
if(refToRemove != null){
list.remove(refToRemove);
}
}
}Now let us re-write UserController to use WeakCollection<T> instead:
public class UserController {
...
private WeakCollection<StateListener> listenerRefs;
...
public void registerUserStateChangeListener(StateListener listener) {
listenerRefs.put(listener);
}
public void removeUserStateChangeListener(StateListener listenerToRemove) {
listenerRefs.remove(listenerToRemove);
}
public void logout() {
for (StateListener listener : listenerRefs.get()) {
listener.userLoggedOut();
}
}
public void login() {
for (StateListener listener : listenerRefs.get()) {
listener.userLoggedIn();
}
}
...
}As shown in the code example above, the WeakCollection<T> removes all of the boilerplate code needed to use WeakReference instead of a normal list. To top it all off: If a call to UserController#removeUserStateChangeListener(StateListener) is missed, the listener, and all the objects it is referencing, will not leak.