commit 2de41686e7927dddd63acfdd496d33386c3a8a31
author Chiachang Wang <chiachangwang@google.com> Thu Sep 23 10:46:03 2021 +0800
committer Chiachang Wang <chiachangwang@google.com> Thu Sep 30 10:30:35 2021 +0000
tree 17f02aa30ecf04158871eac50f5c43a9c5a39fcc
parent 859218c893e799c5c8adc689c3472d757196a9bc

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

framework/src/android/net/ConnectivityManager.java

diff --git a/framework/src/android/net/ConnectivityManager.java b/framework/src/android/net/ConnectivityManager.java
index 7294ec9..0c95732 100644
--- a/framework/src/android/net/ConnectivityManager.java
+++ b/framework/src/android/net/ConnectivityManager.java
@@ -2326,6 +2326,7 @@
         void onNetworkActive();
     }
 
+    @GuardedBy("mNetworkActivityListeners")
     private final ArrayMap<OnNetworkActiveListener, INetworkActivityListener>
             mNetworkActivityListeners = new ArrayMap<>();
 
@@ -2342,18 +2343,20 @@
      * @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 @@
      * @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();
+            }
         }
     }
 

tests/unit/java/android/net/ConnectivityManagerTest.java

diff --git a/tests/unit/java/android/net/ConnectivityManagerTest.java b/tests/unit/java/android/net/ConnectivityManagerTest.java
index 0914492..c475419 100644
--- a/tests/unit/java/android/net/ConnectivityManagerTest.java
+++ b/tests/unit/java/android/net/ConnectivityManagerTest.java
@@ -326,6 +326,8 @@
         verify(mService, times(1)).registerNetworkActivityListener(any());
         manager.removeDefaultNetworkActiveListener(listener);
         verify(mService, times(1)).unregisterNetworkActivityListener(any());
+        assertThrows(IllegalArgumentException.class,
+                () -> manager.removeDefaultNetworkActiveListener(listener));
     }
 
     @Test