Fixed a leak in GeofenceHardwareImpl.java.

Bug: 18542685.
This CL includes two changes:
* Fixed a leak of DeathRecipient when geofences are removed from the
  hardware.
* Avoid creating more DeathRecipient than needed. Use the underlying
  binder object instead of the callback object to tell if they are the
  same. So if the client passes the same callback instance to
  GeofenceHardwareImpl, only one DeathRecipient is created.

Change-Id: I7809e4bc04df4f9e3590de98a03178b276c821ea

diff --git a/core/java/android/hardware/location/GeofenceHardwareImpl.java b/core/java/android/hardware/location/GeofenceHardwareImpl.java
index 5c7a8da..6e5d064 100644 (file)
--- a/core/java/android/hardware/location/GeofenceHardwareImpl.java
+++ b/core/java/android/hardware/location/GeofenceHardwareImpl.java
@@ -23,6 +23,7 @@ import android.location.IGpsGeofenceHardware;
 import android.location.Location;
 import android.os.Handler;
 import android.os.IBinder;
+import android.os.IInterface;
 import android.os.Message;
 import android.os.PowerManager;
 import android.os.RemoteException;
@@ -30,6 +31,7 @@ import android.util.Log;
 import android.util.SparseArray;
 
 import java.util.ArrayList;
+import java.util.Iterator;
 
 /**
  * This class manages the geofences which are handled by hardware.
@@ -558,8 +560,34 @@ public final class GeofenceHardwareImpl {
                         try {
                             callback.onGeofenceRemove(geofenceId, msg.arg2);
                         } catch (RemoteException e) {}
+                        IBinder callbackBinder = callback.asBinder();
+                        boolean callbackInUse = false;
                         synchronized (mGeofences) {
                             mGeofences.remove(geofenceId);
+                            // Check if the underlying binder is still useful for other geofences,
+                            // if no, unlink the DeathRecipient to avoid memory leak.
+                            for (int i = 0; i < mGeofences.size(); i++) {
+                                 if (mGeofences.valueAt(i).asBinder() == callbackBinder) {
+                                     callbackInUse = true;
+                                     break;
+                                 }
+                            }
+                        }
+
+                        // Remove the reaper associated with this binder.
+                        if (!callbackInUse) {
+                            for (Iterator<Reaper> iterator = mReapers.iterator();
+                                    iterator.hasNext();) {
+                                Reaper reaper = iterator.next();
+                                if (reaper.mCallback != null &&
+                                        reaper.mCallback.asBinder() == callbackBinder) {
+                                    iterator.remove();
+                                    reaper.unlinkToDeath();
+                                    if (DEBUG) Log.d(TAG, String.format("Removed reaper %s " +
+                                          "because binder %s is no longer needed.",
+                                          reaper, callbackBinder));
+                                }
+                            }
                         }
                     }
                     releaseWakeLock();
@@ -803,8 +831,9 @@ public final class GeofenceHardwareImpl {
         @Override
         public int hashCode() {
             int result = 17;
-            result = 31 * result + (mCallback != null ? mCallback.hashCode() : 0);
-            result = 31 * result + (mMonitorCallback != null ? mMonitorCallback.hashCode() : 0);
+            result = 31 * result + (mCallback != null ? mCallback.asBinder().hashCode() : 0);
+            result = 31 * result + (mMonitorCallback != null
+                    ? mMonitorCallback.asBinder().hashCode() : 0);
             result = 31 * result + mMonitoringType;
             return result;
         }
@@ -815,9 +844,38 @@ public final class GeofenceHardwareImpl {
             if (obj == this) return true;
 
             Reaper rhs = (Reaper) obj;
-            return rhs.mCallback == mCallback && rhs.mMonitorCallback == mMonitorCallback &&
+            return binderEquals(rhs.mCallback, mCallback) &&
+                    binderEquals(rhs.mMonitorCallback, mMonitorCallback) &&
                     rhs.mMonitoringType == mMonitoringType;
         }
+
+        /**
+         * Compares the underlying Binder of the given two IInterface objects and returns true if
+         * they equals. null values are accepted.
+         */
+        private boolean binderEquals(IInterface left, IInterface right) {
+          if (left == null) {
+            return right == null;
+          } else {
+            return right == null ? false : left.asBinder() == right.asBinder();
+          }
+        }
+
+        /**
+         * Unlinks this DeathRecipient.
+         */
+        private boolean unlinkToDeath() {
+          if (mMonitorCallback != null) {
+            return mMonitorCallback.asBinder().unlinkToDeath(this, 0);
+          } else if (mCallback != null) {
+            return mCallback.asBinder().unlinkToDeath(this, 0);
+          }
+          return true;
+        }
+
+        private boolean callbackEquals(IGeofenceHardwareCallback cb) {
+          return mCallback != null && mCallback.asBinder() == cb.asBinder();
+        }
     }
 
     int getAllowedResolutionLevel(int pid, int uid) {