Skip to content

fix(database): refactor "removeObserver" to fix memory leak #2717

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,33 @@
import io.flutter.plugin.common.BinaryMessenger;
import io.flutter.plugin.common.MethodChannel;
import io.flutter.plugin.common.PluginRegistry;
import io.flutter.plugin.common.PluginRegistry.ViewDestroyListener;
import io.flutter.view.FlutterNativeView;

/** FirebaseDatabasePlugin */
public class FirebaseDatabasePlugin implements FlutterPlugin {

private MethodChannel channel;
private MethodCallHandlerImpl methodCallHandler;

public static void registerWith(PluginRegistry.Registrar registrar) {
FirebaseDatabasePlugin plugin = new FirebaseDatabasePlugin();
final FirebaseDatabasePlugin plugin = new FirebaseDatabasePlugin();
plugin.setupMethodChannel(registrar.messenger());

registrar.addViewDestroyListener(
new ViewDestroyListener() {
@Override
public boolean onViewDestroy(FlutterNativeView view) {
plugin.cleanup();
return false;
}
});
}

private void setupMethodChannel(BinaryMessenger messenger) {
channel = new MethodChannel(messenger, "plugins.flutter.io/firebase_database");
MethodCallHandlerImpl handler = new MethodCallHandlerImpl(channel);
channel.setMethodCallHandler(handler);
methodCallHandler = new MethodCallHandlerImpl(channel);
channel.setMethodCallHandler(methodCallHandler);
}

@Override
Expand All @@ -32,6 +44,12 @@ public void onAttachedToEngine(FlutterPluginBinding binding) {

@Override
public void onDetachedFromEngine(FlutterPluginBinding binding) {
cleanup();
}

private void cleanup() {
methodCallHandler.cleanup();
methodCallHandler = null;
channel.setMethodCallHandler(null);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -53,18 +53,37 @@ class MethodCallHandlerImpl implements MethodChannel.MethodCallHandler {
this.channel = channel;
}

void cleanup() {
final int size = observers.size();
for (int i = 0; i < size; i++) {
final EventObserver observer = observers.valueAt(i);
final Query query = observer.query;
if (observer.requestedEventType.equals(EVENT_TYPE_VALUE)) {
query.removeEventListener((ValueEventListener) observer);
} else {
query.removeEventListener((ChildEventListener) observer);
}
}

observers.clear();
}

private DatabaseReference getReference(FirebaseDatabase database, Map<String, Object> arguments) {
String path = (String) arguments.get("path");
DatabaseReference reference = database.getReference();
if (path != null) reference = reference.child(path);
if (path != null) {
reference = reference.child(path);
}
return reference;
}

private Query getQuery(FirebaseDatabase database, Map<String, Object> arguments) {
Query query = getReference(database, arguments);
@SuppressWarnings("unchecked")
Map<String, Object> parameters = (Map<String, Object>) arguments.get("parameters");
if (parameters == null) return query;
if (parameters == null) {
return query;
}
Object orderBy = parameters.get("orderBy");
if ("child".equals(orderBy)) {
query = query.orderByChild((String) parameters.get("orderByChildKey"));
Expand Down Expand Up @@ -148,6 +167,7 @@ private Query getQuery(FirebaseDatabase database, Map<String, Object> arguments)
}

private class DefaultCompletionListener implements DatabaseReference.CompletionListener {

private final MethodChannel.Result result;

DefaultCompletionListener(MethodChannel.Result result) {
Expand All @@ -164,12 +184,17 @@ public void onComplete(@Nullable DatabaseError error, @NonNull DatabaseReference
}
}

private class EventObserver implements ChildEventListener, ValueEventListener {
private static class EventObserver implements ChildEventListener, ValueEventListener {

private MethodChannel channel;
private String requestedEventType;
private Query query;
private int handle;

EventObserver(String requestedEventType, int handle) {
EventObserver(MethodChannel channel, String requestedEventType, Query query, int handle) {
this.channel = channel;
this.requestedEventType = requestedEventType;
this.query = query;
this.handle = handle;
}

Expand Down Expand Up @@ -249,6 +274,7 @@ public void onMethodCall(final MethodCall call, @NonNull final MethodChannel.Res
} else {
database = FirebaseDatabase.getInstance();
}

switch (call.method) {
case "FirebaseDatabase#goOnline":
{
Expand Down Expand Up @@ -347,7 +373,8 @@ public void onMethodCall(final MethodCall call, @NonNull final MethodChannel.Res
@NonNull
@Override
public Transaction.Result doTransaction(@NonNull MutableData mutableData) {
// Tasks are used to allow native execution of doTransaction to wait while Snapshot is
// Tasks are used to allow native execution of doTransaction to wait while
// Snapshot is
// processed by logic on the Dart side.
final TaskCompletionSource<Map<String, Object>> updateMutableDataTCS =
new TaskCompletionSource<>();
Expand Down Expand Up @@ -514,24 +541,25 @@ public void run() {
{
String eventType = call.argument("eventType");
int handle = nextHandle++;
Query query = getQuery(database, arguments);
MethodCallHandlerImpl.EventObserver observer =
new MethodCallHandlerImpl.EventObserver(eventType, handle);
new MethodCallHandlerImpl.EventObserver(channel, eventType, query, handle);
observers.put(handle, observer);
if (EVENT_TYPE_VALUE.equals(eventType)) {
getQuery(database, arguments).addValueEventListener(observer);
query.addValueEventListener(observer);
} else {
getQuery(database, arguments).addChildEventListener(observer);
query.addChildEventListener(observer);
}
result.success(handle);
break;
}

case "Query#removeObserver":
{
Query query = getQuery(database, arguments);
Integer handle = call.argument("handle");
MethodCallHandlerImpl.EventObserver observer = observers.get(handle);
if (observer != null) {
final Query query = observer.query;
if (observer.requestedEventType.equals(EVENT_TYPE_VALUE)) {
query.removeEventListener((ValueEventListener) observer);
} else {
Expand Down