From 4cc80a758c3e4f044c2e53b6210df0e515536a1b Mon Sep 17 00:00:00 2001
From: Zhentao Sun <robinvane@google.com>
Date: Wed, 3 Dec 2014 14:27:26 -0800
Subject: 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
---
 .../hardware/location/GeofenceHardwareImpl.java    | 64 +++++++++++++++++++++-
 1 file changed, 61 insertions(+), 3 deletions(-)

diff --git a/core/java/android/hardware/location/GeofenceHardwareImpl.java b/core/java/android/hardware/location/GeofenceHardwareImpl.java
index 5c7a8da..6e5d064 100644
--- 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) {
-- 
cgit v1.1