Skip to content

Commit 804f4e7

Browse files
SUPERCILEXsamtstern
authored andcommitted
Ouch! Fix terrible db listener leak (#828)
1 parent bee4c1a commit 804f4e7

File tree

4 files changed

+54
-46
lines changed

4 files changed

+54
-46
lines changed

database/src/main/java/com/firebase/ui/database/CachingObservableSnapshotArray.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,14 @@ public T getObject(int index) {
4343
}
4444
}
4545

46+
@Override
47+
protected void onDestroy() {
48+
super.onDestroy();
49+
clearData();
50+
}
51+
52+
/** @deprecated use {@link ObservableSnapshotArray#onDestroy()} instead */
53+
@Deprecated
4654
protected void clearData() {
4755
getSnapshots().clear();
4856
mObjectCache.clear();

database/src/main/java/com/firebase/ui/database/FirebaseArray.java

Lines changed: 8 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,6 @@
1414

1515
package com.firebase.ui.database;
1616

17-
import android.support.annotation.NonNull;
18-
1917
import com.google.firebase.database.ChildEventListener;
2018
import com.google.firebase.database.DataSnapshot;
2119
import com.google.firebase.database.DatabaseError;
@@ -66,30 +64,17 @@ protected List<DataSnapshot> getSnapshots() {
6664
}
6765

6866
@Override
69-
public ChangeEventListener addChangeEventListener(@NonNull ChangeEventListener listener) {
70-
boolean wasListening = isListening();
71-
super.addChangeEventListener(listener);
72-
73-
// Only start listening when the first listener is added
74-
if (!wasListening) {
75-
mQuery.addChildEventListener(this);
76-
mQuery.addValueEventListener(this);
77-
}
78-
79-
return listener;
67+
protected void onCreate() {
68+
super.onCreate();
69+
mQuery.addChildEventListener(this);
70+
mQuery.addValueEventListener(this);
8071
}
8172

8273
@Override
83-
public void removeChangeEventListener(@NonNull ChangeEventListener listener) {
84-
super.removeChangeEventListener(listener);
85-
86-
// Clear data when all listeners are removed
87-
if (!isListening()) {
88-
mQuery.removeEventListener((ValueEventListener) this);
89-
mQuery.removeEventListener((ChildEventListener) this);
90-
91-
clearData();
92-
}
74+
protected void onDestroy() {
75+
super.onDestroy();
76+
mQuery.removeEventListener((ValueEventListener) this);
77+
mQuery.removeEventListener((ChildEventListener) this);
9378
}
9479

9580
@Override

database/src/main/java/com/firebase/ui/database/FirebaseIndexArray.java

Lines changed: 15 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414

1515
package com.firebase.ui.database;
1616

17-
import android.support.annotation.NonNull;
1817
import android.support.v7.widget.RecyclerView;
1918
import android.util.Log;
2019

@@ -85,10 +84,25 @@ public String parseSnapshot(DataSnapshot snapshot) {
8584
return snapshot.getKey();
8685
}
8786
});
87+
}
8888

89+
@Override
90+
protected void onCreate() {
91+
super.onCreate();
8992
mKeySnapshots.addChangeEventListener(this);
9093
}
9194

95+
@Override
96+
protected void onDestroy() {
97+
super.onDestroy();
98+
mKeySnapshots.removeChangeEventListener(this);
99+
100+
for (DatabaseReference ref : mRefs.keySet()) {
101+
ref.removeEventListener(mRefs.get(ref));
102+
}
103+
mRefs.clear();
104+
}
105+
92106
@Override
93107
public void onChildChanged(EventType type, DataSnapshot snapshot, int index, int oldIndex) {
94108
switch (type) {
@@ -121,29 +135,11 @@ public void onCancelled(DatabaseError error) {
121135
Log.e(TAG, "A fatal error occurred retrieving the necessary keys to populate your adapter.");
122136
}
123137

124-
@Override
125-
public void removeChangeEventListener(@NonNull ChangeEventListener listener) {
126-
super.removeChangeEventListener(listener);
127-
if (!isListening()) {
128-
for (DatabaseReference ref : mRefs.keySet()) {
129-
ref.removeEventListener(mRefs.get(ref));
130-
}
131-
132-
clearData();
133-
}
134-
}
135-
136138
@Override
137139
protected List<DataSnapshot> getSnapshots() {
138140
return mDataSnapshots;
139141
}
140142

141-
@Override
142-
protected void clearData() {
143-
super.clearData();
144-
mRefs.clear();
145-
}
146-
147143
private int getIndexForKey(String key) {
148144
int dataCount = size();
149145
int index = 0;

database/src/main/java/com/firebase/ui/database/ObservableSnapshotArray.java

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,8 @@ public ObservableSnapshotArray(@NonNull SnapshotParser<E> parser) {
5151
public ChangeEventListener addChangeEventListener(@NonNull ChangeEventListener listener) {
5252
Preconditions.checkNotNull(listener);
5353

54+
boolean wasListening = isListening();
55+
5456
mListeners.add(listener);
5557
for (int i = 0; i < size(); i++) {
5658
listener.onChildChanged(ChangeEventListener.EventType.ADDED, get(i), i, -1);
@@ -59,20 +61,37 @@ public ChangeEventListener addChangeEventListener(@NonNull ChangeEventListener l
5961
listener.onDataChanged();
6062
}
6163

64+
if (!wasListening) { onCreate(); }
65+
6266
return listener;
6367
}
6468

69+
/**
70+
* Called when the {@link ObservableSnapshotArray} is active and should start listening to the
71+
* Firebase database.
72+
*/
73+
@CallSuper
74+
protected void onCreate() {}
75+
6576
/**
6677
* Detach a {@link com.google.firebase.database.ChildEventListener} from this array.
6778
*/
6879
@CallSuper
6980
public void removeChangeEventListener(@NonNull ChangeEventListener listener) {
7081
mListeners.remove(listener);
7182

72-
// Reset mHasDataChanged if there are no more listeners
73-
if (!isListening()) {
74-
mHasDataChanged = false;
75-
}
83+
if (!isListening()) { onDestroy(); }
84+
}
85+
86+
/**
87+
* Called when the {@link ObservableSnapshotArray} is inactive and should stop listening to the
88+
* Firebase database.
89+
* <p>
90+
* All data should also be cleared here.
91+
*/
92+
@CallSuper
93+
protected void onDestroy() {
94+
mHasDataChanged = false;
7695
}
7796

7897
/**

0 commit comments

Comments
 (0)