Merge "Naming fix and add comments" am: 3c03d5bd2b am: 24b9851605

am: 4a901144fe

Change-Id: I1e2446f67e0505d30edb230f2dde70855c9fe00c

core/java/android/os/Binder.java

@@ -23,7 +23,6 @@ import android.util.Log;
 import android.util.Slog;
 
 import com.android.internal.util.FastPrintWriter;
-import com.android.internal.util.FunctionalUtils;
 import com.android.internal.util.FunctionalUtils.ThrowingRunnable;
 import com.android.internal.util.FunctionalUtils.ThrowingSupplier;
 
@@ -202,7 +201,7 @@ public class Binder implements IBinder {
      * then its own pid is returned.
      */
     public static final native int getCallingPid();
-    
+
     /**
      * Return the Linux uid assigned to the process that sent you the
      * current transaction that is being processed.  This uid can be used with
@@ -335,7 +334,7 @@ public class Binder implements IBinder {
      * it needs to.
      */
     public static final native void flushPendingCommands();
-    
+
     /**
      * Add the calling thread to the IPC thread pool.  This function does
      * not return until the current process is exiting.
@@ -372,7 +371,7 @@ public class Binder implements IBinder {
             }
         }
     }
-    
+
     /**
      * Convenience method for associating a specific interface with the Binder.
      * After calling, queryLocalInterface() will be implemented for you
@@ -383,7 +382,7 @@ public class Binder implements IBinder {
         mOwner = owner;
         mDescriptor = descriptor;
     }
-    
+
     /**
      * Default implementation returns an empty interface name.
      */
@@ -408,7 +407,7 @@ public class Binder implements IBinder {
     public boolean isBinderAlive() {
         return true;
     }
-    
+
     /**
      * Use information supplied to attachInterface() to return the
      * associated IInterface if it matches the requested
@@ -630,7 +629,7 @@ public class Binder implements IBinder {
         }
         return r;
     }
-    
+
     /**
      * Local implementation is a no-op.
      */
@@ -643,7 +642,7 @@ public class Binder implements IBinder {
     public boolean unlinkToDeath(@NonNull DeathRecipient recipient, int flags) {
         return true;
     }
-    
+
     protected void finalize() throws Throwable {
         try {
             destroyBinder();
@@ -730,7 +729,15 @@ public class Binder implements IBinder {
     }
 }
 
+/**
+ * Java proxy for a native IBinder object.
+ * Allocated and constructed by the native javaObjectforIBinder function. Never allocated
+ * directly from Java code.
+ */
 final class BinderProxy implements IBinder {
+    // See android_util_Binder.cpp for the native half of this.
+    // TODO: Consider using NativeAllocationRegistry instead of finalization.
+
     // Assume the process-wide default value when created
     volatile boolean mWarnOnBlocking = Binder.sWarnOnBlocking;
 
@@ -789,7 +796,7 @@ final class BinderProxy implements IBinder {
             reply.recycle();
         }
     }
-    
+
     public void dumpAsync(FileDescriptor fd, String[] args) throws RemoteException {
         Parcel data = Parcel.obtain();
         Parcel reply = Parcel.obtain();
@@ -826,7 +833,7 @@ final class BinderProxy implements IBinder {
     BinderProxy() {
         mSelf = new WeakReference(this);
     }
-    
+
     @Override
     protected void finalize() throws Throwable {
         try {
@@ -835,9 +842,9 @@ final class BinderProxy implements IBinder {
             super.finalize();
         }
     }
-    
+
     private native final void destroy();
-    
+
     private static final void sendDeathNotice(DeathRecipient recipient) {
         if (false) Log.v("JavaBinder", "sendDeathNotice to " + recipient);
         try {
@@ -848,8 +855,20 @@ final class BinderProxy implements IBinder {
                     exc);
         }
     }
-    
+
+    // This WeakReference to "this" is used only by native code to "attach" to the
+    // native IBinder object.
+    // Using WeakGlobalRefs instead currently appears unsafe, in that they can yield a
+    // non-null value after the BinderProxy is enqueued for finalization.
+    // Used only once immediately after construction.
+    // TODO: Consider making the extra native-to-java call to compute this on the fly.
     final private WeakReference mSelf;
+
+    // Native pointer to the wrapped native IBinder object. Counted as strong reference.
     private long mObject;
+
+    // Native pointer to native DeathRecipientList. Counted as strong reference.
+    // Basically owned by the JavaProxy object. Reference counted only because DeathRecipients
+    // hold a weak reference that can be temporarily promoted.
     private long mOrgue;
 }

core/jni/android_util_Binder.cpp

@@ -517,8 +517,8 @@ protected:
 
 private:
     JavaVM* const mVM;
-    jobject mObject;
-    jweak mObjectWeak; // will be a weak ref to the same VM-side DeathRecipient after binderDied()
+    jobject mObject;  // Initial strong ref to Java-side DeathRecipient. Cleared on binderDied().
+    jweak mObjectWeak; // weak ref to the same Java-side DeathRecipient after binderDied().
     wp<DeathRecipientList> mList;
 };
 
@@ -590,7 +590,7 @@ static void proxy_cleanup(const void* id, void* obj, void* cleanupCookie)
     env->DeleteGlobalRef((jobject)obj);
 }
 
-static Mutex mProxyLock;
+static Mutex gProxyLock;
 
 jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val)
 {
@@ -605,7 +605,7 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val)
 
     // For the rest of the function we will hold this lock, to serialize
     // looking/creation/destruction of Java proxies for native Binder proxies.
-    AutoMutex _l(mProxyLock);
+    AutoMutex _l(gProxyLock);
 
     // Someone else's...  do we know about it?
     jobject object = (jobject)val->findObject(&gBinderProxyOffsets);
@@ -1281,7 +1281,7 @@ static jboolean android_os_BinderProxy_unlinkToDeath(JNIEnv* env, jobject obj,
 static void android_os_BinderProxy_destroy(JNIEnv* env, jobject obj)
 {
     // Don't race with construction/initialization
-    AutoMutex _l(mProxyLock);
+    AutoMutex _l(gProxyLock);
 
     IBinder* b = (IBinder*)
             env->GetLongField(obj, gBinderProxyOffsets.mObject);