From 2de41686e7927dddd63acfdd496d33386c3a8a31 Mon Sep 17 00:00:00 2001
From: Chiachang Wang <chiachangwang@google.com>
Date: Thu, 23 Sep 2021 10:46:03 +0800
Subject: [PATCH] Remove reference when active listener is unregistered

The registered listeners are added into internal ArrayMap as a
reference but are not removed when the listeners are
unregistered. The actions for registration should be done in
pairs.

Test: atest FrameworksNetTests
Change-Id: Id9e674f5104d1471dd81224b6a271a8a92172e34
---
 .../src/android/net/ConnectivityManager.java  | 34 +++++++++++--------
 .../android/net/ConnectivityManagerTest.java  |  2 ++
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java
index 7294ec950a..0c9573202e 100644
--- a/framework/src/android/net/ConnectivityManager.java
+++ b/framework/src/android/net/ConnectivityManager.java
@@ -2326,6 +2326,7 @@ public class ConnectivityManager {
         void onNetworkActive();
     }
 
+    @GuardedBy("mNetworkActivityListeners")
     private final ArrayMap<OnNetworkActiveListener, INetworkActivityListener>
             mNetworkActivityListeners = new ArrayMap<>();
 
@@ -2342,18 +2343,20 @@ public class ConnectivityManager {
      * @param l The listener to be told when the network is active.
      */
     public void addDefaultNetworkActiveListener(final OnNetworkActiveListener l) {
-        INetworkActivityListener rl = new INetworkActivityListener.Stub() {
+        final INetworkActivityListener rl = new INetworkActivityListener.Stub() {
             @Override
             public void onNetworkActive() throws RemoteException {
                 l.onNetworkActive();
             }
         };
 
-        try {
-            mService.registerNetworkActivityListener(rl);
-            mNetworkActivityListeners.put(l, rl);
-        } catch (RemoteException e) {
-            throw e.rethrowFromSystemServer();
+        synchronized (mNetworkActivityListeners) {
+            try {
+                mService.registerNetworkActivityListener(rl);
+                mNetworkActivityListeners.put(l, rl);
+            } catch (RemoteException e) {
+                throw e.rethrowFromSystemServer();
+            }
         }
     }
 
@@ -2364,14 +2367,17 @@ public class ConnectivityManager {
      * @param l Previously registered listener.
      */
     public void removeDefaultNetworkActiveListener(@NonNull OnNetworkActiveListener l) {
-        INetworkActivityListener rl = mNetworkActivityListeners.get(l);
-        if (rl == null) {
-            throw new IllegalArgumentException("Listener was not registered.");
-        }
-        try {
-            mService.unregisterNetworkActivityListener(rl);
-        } catch (RemoteException e) {
-            throw e.rethrowFromSystemServer();
+        synchronized (mNetworkActivityListeners) {
+            final INetworkActivityListener rl = mNetworkActivityListeners.get(l);
+            if (rl == null) {
+                throw new IllegalArgumentException("Listener was not registered.");
+            }
+            try {
+                mService.unregisterNetworkActivityListener(rl);
+                mNetworkActivityListeners.remove(l);
+            } catch (RemoteException e) {
+                throw e.rethrowFromSystemServer();
+            }
         }
     }
 
diff --git a/tests/unit/java/android/net/ConnectivityManagerTest.java b/tests/unit/java/android/net/ConnectivityManagerTest.java
index 0914492704..c475419b8a 100644
--- a/tests/unit/java/android/net/ConnectivityManagerTest.java
+++ b/tests/unit/java/android/net/ConnectivityManagerTest.java
@@ -326,6 +326,8 @@ public class ConnectivityManagerTest {
         verify(mService, times(1)).registerNetworkActivityListener(any());
         manager.removeDefaultNetworkActiveListener(listener);
         verify(mService, times(1)).unregisterNetworkActivityListener(any());
+        assertThrows(IllegalArgumentException.class,
+                () -> manager.removeDefaultNetworkActiveListener(listener));
     }
 
     @Test