fix(android): Leaking listeners (#118)

* Fix condition in RNPerformance.removeListener

* Fix ReactMarker listener leak

* Use invalidate instead of deprecated onCatalystInstanceDestroyed and add missing method from spec

* Prevent leaking listener when getPackages is called multiple times

* Remove unused method

* Remove comments

* Remove override

---------

Co-authored-by: Joel Arvidsson <joel@oblador.se>

Author: mateuuszzzzz

packages/react-native-performance/android/src/main/java/com/oblador/performance/PerformanceModule.java

@@ -25,64 +25,82 @@ public class PerformanceModule extends ReactContextBaseJavaModule implements Tur
     private static final Queue<PerformanceEntry> markBuffer = new ConcurrentLinkedQueue<>();
     private static boolean didEmit = false;
 
+    private static final ReactMarker.MarkerListener startupMarkerListener = (name, tag, instanceKey) -> {
+        switch (name) {
+            case RELOAD:
+                clearMarkBuffer();
+                addMark(new PerformanceMark(BRIDGE_SETUP_START, SystemClock.uptimeMillis()));
+                break;
+            case ATTACH_MEASURED_ROOT_VIEWS_END:
+            case ATTACH_MEASURED_ROOT_VIEWS_START:
+            case BUILD_NATIVE_MODULE_REGISTRY_END:
+            case BUILD_NATIVE_MODULE_REGISTRY_START:
+            case CONTENT_APPEARED:
+            case CREATE_CATALYST_INSTANCE_END:
+            case CREATE_CATALYST_INSTANCE_START:
+            case CREATE_REACT_CONTEXT_END:
+            case CREATE_REACT_CONTEXT_START:
+            case CREATE_UI_MANAGER_MODULE_CONSTANTS_END:
+            case CREATE_UI_MANAGER_MODULE_CONSTANTS_START:
+            case CREATE_UI_MANAGER_MODULE_END:
+            case CREATE_UI_MANAGER_MODULE_START:
+            case CREATE_VIEW_MANAGERS_END:
+            case CREATE_VIEW_MANAGERS_START:
+            case DOWNLOAD_END:
+            case DOWNLOAD_START:
+            case LOAD_REACT_NATIVE_SO_FILE_END:
+            case LOAD_REACT_NATIVE_SO_FILE_START:
+            case PRE_RUN_JS_BUNDLE_START:
+            case PRE_SETUP_REACT_CONTEXT_END:
+            case PRE_SETUP_REACT_CONTEXT_START:
+            case PROCESS_CORE_REACT_PACKAGE_END:
+            case PROCESS_CORE_REACT_PACKAGE_START:
+            case REACT_CONTEXT_THREAD_END:
+            case REACT_CONTEXT_THREAD_START:
+            case RUN_JS_BUNDLE_END:
+            case RUN_JS_BUNDLE_START:
+            case SETUP_REACT_CONTEXT_END:
+            case SETUP_REACT_CONTEXT_START:
+            case VM_INIT:
+                long startTime = SystemClock.uptimeMillis();
+                addMark(new PerformanceMark(getMarkName(name), startTime));
+                break;
+        }
+    };
+
+    private final ReactMarker.MarkerListener contentAppearedListener = (name, tag, instanceKey) -> {
+        switch (name) {
+            case CONTENT_APPEARED:
+                eventsBuffered = false;
+                emitNativeStartupTime();
+                emitBufferedMarks();
+                break;
+            case RELOAD:
+                eventsBuffered = true;
+                break;
+        }
+    };
+
     public PerformanceModule(@NonNull final ReactApplicationContext reactContext) {
         super(reactContext);
         setupMarkerListener();
         setupNativeMarkerListener();
     }
 
+    private void setupMarkerListener() {
+        ReactMarker.addListener(
+                contentAppearedListener
+        );
+    }
+
     private void setupNativeMarkerListener() {
         RNPerformance.getInstance().addListener(this);
     }
 
     // Need to set up the marker listener before the react module is initialized
     // to capture all events
     public static void setupListener() {
-        ReactMarker.addListener(
-                (name, tag, instanceKey) -> {
-                    switch (name) {
-                        case RELOAD:
-                            clearMarkBuffer();
-                            addMark(new PerformanceMark(BRIDGE_SETUP_START, SystemClock.uptimeMillis()));
-                            break;
-                        case ATTACH_MEASURED_ROOT_VIEWS_END:
-                        case ATTACH_MEASURED_ROOT_VIEWS_START:
-                        case BUILD_NATIVE_MODULE_REGISTRY_END:
-                        case BUILD_NATIVE_MODULE_REGISTRY_START:
-                        case CONTENT_APPEARED:
-                        case CREATE_CATALYST_INSTANCE_END:
-                        case CREATE_CATALYST_INSTANCE_START:
-                        case CREATE_REACT_CONTEXT_END:
-                        case CREATE_REACT_CONTEXT_START:
-                        case CREATE_UI_MANAGER_MODULE_CONSTANTS_END:
-                        case CREATE_UI_MANAGER_MODULE_CONSTANTS_START:
-                        case CREATE_UI_MANAGER_MODULE_END:
-                        case CREATE_UI_MANAGER_MODULE_START:
-                        case CREATE_VIEW_MANAGERS_END:
-                        case CREATE_VIEW_MANAGERS_START:
-                        case DOWNLOAD_END:
-                        case DOWNLOAD_START:
-                        case LOAD_REACT_NATIVE_SO_FILE_END:
-                        case LOAD_REACT_NATIVE_SO_FILE_START:
-                        case PRE_RUN_JS_BUNDLE_START:
-                        case PRE_SETUP_REACT_CONTEXT_END:
-                        case PRE_SETUP_REACT_CONTEXT_START:
-                        case PROCESS_CORE_REACT_PACKAGE_END:
-                        case PROCESS_CORE_REACT_PACKAGE_START:
-                        case REACT_CONTEXT_THREAD_END:
-                        case REACT_CONTEXT_THREAD_START:
-                        case RUN_JS_BUNDLE_END:
-                        case RUN_JS_BUNDLE_START:
-                        case SETUP_REACT_CONTEXT_END:
-                        case SETUP_REACT_CONTEXT_START:
-                        case VM_INIT:
-                            long startTime = SystemClock.uptimeMillis();
-                            addMark(new PerformanceMark(getMarkName(name), startTime));
-                            break;
-
-                    }
-                }
-        );
+        ReactMarker.addListener(startupMarkerListener);
     }
 
     private static void clearMarkBuffer() {
@@ -123,23 +141,6 @@ private void emitNativeStartupTime() {
         safelyEmitMark(new PerformanceMark("nativeLaunchEnd", StartTimeProvider.getEndTime()));
     }
 
-    private void setupMarkerListener() {
-        ReactMarker.addListener(
-                (name, tag, instanceKey) -> {
-                    switch (name) {
-                        case CONTENT_APPEARED:
-                            eventsBuffered = false;
-                            emitNativeStartupTime();
-                            emitBufferedMarks();
-                            break;
-                        case RELOAD:
-                            eventsBuffered = true;
-                            break;
-                    }
-                }
-        );
-    }
-
     private void safelyEmitMark(PerformanceEntry entry) {
         if (eventsBuffered) {
             addMark(entry);
@@ -217,13 +218,18 @@ public void logMarker(PerformanceEntry entry) {
     }
 
     @Override
-    public void onCatalystInstanceDestroy() {
-        super.onCatalystInstanceDestroy();
+    public void invalidate() {
+        super.invalidate();
         RNPerformance.getInstance().removeListener(this);
+        ReactMarker.removeListener(contentAppearedListener);
     }
 
     // Fix new arch runtime error
     public void addListener(String eventName) {
 
     }
+
+    public void removeListeners(double count) {
+
+    }
 }

packages/react-native-performance/android/src/main/java/com/oblador/performance/RNPerformance.java

@@ -43,7 +43,7 @@ protected void addListener(MarkerListener listener) {
 
     @DoNotStrip
     protected void removeListener(MarkerListener listener) {
-        if (!sListeners.contains(listener)) {
+        if (sListeners.contains(listener)) {
             sListeners.remove(listener);
         }
     }