Fix reference cycle in InputEventReceiver and Sender.

If the receiver or sender was not properly disposed, then
the underlying input channel might be leaked because the
native peer was holding a strong reference to the object.

Switched to using a weak reference.

Also updated Binder to use a new helper created for this purpose.

Change-Id: I19680bf96d0548777bff02aa1d91874d1e8e41da

3 files changed, 54 insertions, 35 deletions

diff --git a/core/jni/android_util_Binder.cpp b/core/jni/android_util_Binder.cpp
index a7eede2..8766cf9 100644
--- a/core/jni/android_util_Binder.cpp
+++ b/core/jni/android_util_Binder.cpp
@@ -93,14 +93,6 @@ static struct debug_offsets_t
 
 // ----------------------------------------------------------------------------
 
-static struct weakreference_offsets_t
-{
-    // Class state.
-    jclass mClass;
-    jmethodID mGet;
-
-} gWeakReferenceOffsets;
-
 static struct error_offsets_t
 {
     jclass mClass;
@@ -570,7 +562,7 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val)
     // Someone else's...  do we know about it?
     jobject object = (jobject)val->findObject(&gBinderProxyOffsets);
     if (object != NULL) {
-        jobject res = env->CallObjectMethod(object, gWeakReferenceOffsets.mGet);
+        jobject res = jniGetReferent(env, object);
         if (res != NULL) {
             ALOGV("objectForBinder %p: found existing %p!\n", val.get(), res);
             return res;
@@ -1211,13 +1203,6 @@ static int int_register_android_os_BinderProxy(JNIEnv* env)
 {
     jclass clazz;
 
-    clazz = env->FindClass("java/lang/ref/WeakReference");
-    LOG_FATAL_IF(clazz == NULL, "Unable to find class java.lang.ref.WeakReference");
-    gWeakReferenceOffsets.mClass = (jclass) env->NewGlobalRef(clazz);
-    gWeakReferenceOffsets.mGet
-        = env->GetMethodID(clazz, "get", "()Ljava/lang/Object;");
-    assert(gWeakReferenceOffsets.mGet);
-
     clazz = env->FindClass("java/lang/Error");
     LOG_FATAL_IF(clazz == NULL, "Unable to find class java.lang.Error");
     gErrorOffsets.mClass = (jclass) env->NewGlobalRef(clazz);
diff --git a/core/jni/android_view_InputEventReceiver.cpp b/core/jni/android_view_InputEventReceiver.cpp
index 198814a..c350521 100644
--- a/core/jni/android_view_InputEventReceiver.cpp
+++ b/core/jni/android_view_InputEventReceiver.cpp
@@ -34,6 +34,8 @@
 #include "android_view_KeyEvent.h"
 #include "android_view_MotionEvent.h"
 
+#include <ScopedLocalRef.h>
+
 namespace android {
 
 static struct {
@@ -47,7 +49,7 @@ static struct {
 class NativeInputEventReceiver : public LooperCallback {
 public:
     NativeInputEventReceiver(JNIEnv* env,
-            jobject receiverObj, const sp<InputChannel>& inputChannel,
+            jobject receiverWeak, const sp<InputChannel>& inputChannel,
             const sp<MessageQueue>& messageQueue);
 
     status_t initialize();
@@ -59,7 +61,7 @@ protected:
     virtual ~NativeInputEventReceiver();
 
 private:
-    jobject mReceiverObjGlobal;
+    jobject mReceiverWeakGlobal;
     InputConsumer mInputConsumer;
     sp<MessageQueue> mMessageQueue;
     PreallocatedInputEventFactory mInputEventFactory;
@@ -74,9 +76,9 @@ private:
 
 
 NativeInputEventReceiver::NativeInputEventReceiver(JNIEnv* env,
-        jobject receiverObj, const sp<InputChannel>& inputChannel,
+        jobject receiverWeak, const sp<InputChannel>& inputChannel,
         const sp<MessageQueue>& messageQueue) :
-        mReceiverObjGlobal(env->NewGlobalRef(receiverObj)),
+        mReceiverWeakGlobal(env->NewGlobalRef(receiverWeak)),
         mInputConsumer(inputChannel), mMessageQueue(messageQueue),
         mBatchedInputEventPending(false) {
 #if DEBUG_DISPATCH_CYCLE
@@ -86,7 +88,7 @@ NativeInputEventReceiver::NativeInputEventReceiver(JNIEnv* env,
 
 NativeInputEventReceiver::~NativeInputEventReceiver() {
     JNIEnv* env = AndroidRuntime::getJNIEnv();
-    env->DeleteGlobalRef(mReceiverObjGlobal);
+    env->DeleteGlobalRef(mReceiverWeakGlobal);
 }
 
 status_t NativeInputEventReceiver::initialize() {
@@ -151,6 +153,7 @@ status_t NativeInputEventReceiver::consumeEvents(JNIEnv* env,
         mBatchedInputEventPending = false;
     }
 
+    ScopedLocalRef<jobject> receiverObj(env, NULL);
     bool skipCallbacks = false;
     for (;;) {
         uint32_t seq;
@@ -162,12 +165,21 @@ status_t NativeInputEventReceiver::consumeEvents(JNIEnv* env,
                 if (!skipCallbacks && !mBatchedInputEventPending
                         && mInputConsumer.hasPendingBatch()) {
                     // There is a pending batch.  Come back later.
+                    if (!receiverObj.get()) {
+                        receiverObj.reset(jniGetReferent(env, mReceiverWeakGlobal));
+                        if (!receiverObj.get()) {
+                            ALOGW("channel '%s' ~ Receiver object was finalized "
+                                    "without being disposed.", getInputChannelName());
+                            return DEAD_OBJECT;
+                        }
+                    }
+
                     mBatchedInputEventPending = true;
 #if DEBUG_DISPATCH_CYCLE
                     ALOGD("channel '%s' ~ Dispatching batched input event pending notification.",
                             getInputChannelName());
 #endif
-                    env->CallVoidMethod(mReceiverObjGlobal,
+                    env->CallVoidMethod(receiverObj.get(),
                             gInputEventReceiverClassInfo.dispatchBatchedInputEventPending);
                     if (env->ExceptionCheck()) {
                         ALOGE("Exception dispatching batched input events.");
@@ -183,6 +195,15 @@ status_t NativeInputEventReceiver::consumeEvents(JNIEnv* env,
         assert(inputEvent);
 
         if (!skipCallbacks) {
+            if (!receiverObj.get()) {
+                receiverObj.reset(jniGetReferent(env, mReceiverWeakGlobal));
+                if (!receiverObj.get()) {
+                    ALOGW("channel '%s' ~ Receiver object was finalized "
+                            "without being disposed.", getInputChannelName());
+                    return DEAD_OBJECT;
+                }
+            }
+
             jobject inputEventObj;
             switch (inputEvent->getType()) {
             case AINPUT_EVENT_TYPE_KEY:
@@ -210,12 +231,13 @@ status_t NativeInputEventReceiver::consumeEvents(JNIEnv* env,
 #if DEBUG_DISPATCH_CYCLE
                 ALOGD("channel '%s' ~ Dispatching input event.", getInputChannelName());
 #endif
-                env->CallVoidMethod(mReceiverObjGlobal,
+                env->CallVoidMethod(receiverObj.get(),
                         gInputEventReceiverClassInfo.dispatchInputEvent, seq, inputEventObj);
                 if (env->ExceptionCheck()) {
                     ALOGE("Exception dispatching input event.");
                     skipCallbacks = true;
                 }
+                env->DeleteLocalRef(inputEventObj);
             } else {
                 ALOGW("channel '%s' ~ Failed to obtain event object.", getInputChannelName());
                 skipCallbacks = true;
@@ -229,7 +251,7 @@ status_t NativeInputEventReceiver::consumeEvents(JNIEnv* env,
 }
 
 
-static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj,
+static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverWeak,
         jobject inputChannelObj, jobject messageQueueObj) {
     sp<InputChannel> inputChannel = android_view_InputChannel_getInputChannel(env,
             inputChannelObj);
@@ -245,7 +267,7 @@ static jint nativeInit(JNIEnv* env, jclass clazz, jobject receiverObj,
     }
 
     sp<NativeInputEventReceiver> receiver = new NativeInputEventReceiver(env,
-            receiverObj, inputChannel, messageQueue);
+            receiverWeak, inputChannel, messageQueue);
     status_t status = receiver->initialize();
     if (status) {
         String8 message;
@@ -293,7 +315,7 @@ static void nativeConsumeBatchedInputEvents(JNIEnv* env, jclass clazz, jint rece
 static JNINativeMethod gMethods[] = {
     /* name, signature, funcPtr */
     { "nativeInit",
-            "(Landroid/view/InputEventReceiver;Landroid/view/InputChannel;Landroid/os/MessageQueue;)I",
+            "(Ljava/lang/ref/WeakReference;Landroid/view/InputChannel;Landroid/os/MessageQueue;)I",
             (void*)nativeInit },
     { "nativeDispose", "(I)V",
             (void*)nativeDispose },
diff --git a/core/jni/android_view_InputEventSender.cpp b/core/jni/android_view_InputEventSender.cpp
index 8a149d8..b46eb4b 100644
--- a/core/jni/android_view_InputEventSender.cpp
+++ b/core/jni/android_view_InputEventSender.cpp
@@ -35,6 +35,8 @@
 #include "android_view_KeyEvent.h"
 #include "android_view_MotionEvent.h"
 
+#include <ScopedLocalRef.h>
+
 namespace android {
 
 static struct {
@@ -47,7 +49,7 @@ static struct {
 class NativeInputEventSender : public LooperCallback {
 public:
     NativeInputEventSender(JNIEnv* env,
-            jobject senderObj, const sp<InputChannel>& inputChannel,
+            jobject senderWeak, const sp<InputChannel>& inputChannel,
             const sp<MessageQueue>& messageQueue);
 
     status_t initialize();
@@ -59,7 +61,7 @@ protected:
     virtual ~NativeInputEventSender();
 
 private:
-    jobject mSenderObjGlobal;
+    jobject mSenderWeakGlobal;
     InputPublisher mInputPublisher;
     sp<MessageQueue> mMessageQueue;
     KeyedVector<uint32_t, uint32_t> mPublishedSeqMap;
@@ -75,9 +77,9 @@ private:
 
 
 NativeInputEventSender::NativeInputEventSender(JNIEnv* env,
-        jobject senderObj, const sp<InputChannel>& inputChannel,
+        jobject senderWeak, const sp<InputChannel>& inputChannel,
         const sp<MessageQueue>& messageQueue) :
-        mSenderObjGlobal(env->NewGlobalRef(senderObj)),
+        mSenderWeakGlobal(env->NewGlobalRef(senderWeak)),
         mInputPublisher(inputChannel), mMessageQueue(messageQueue),
         mNextPublishedSeq(1) {
 #if DEBUG_DISPATCH_CYCLE
@@ -87,7 +89,7 @@ NativeInputEventSender::NativeInputEventSender(JNIEnv* env,
 
 NativeInputEventSender::~NativeInputEventSender() {
     JNIEnv* env = AndroidRuntime::getJNIEnv();
-    env->DeleteGlobalRef(mSenderObjGlobal);
+    env->DeleteGlobalRef(mSenderWeakGlobal);
 }
 
 status_t NativeInputEventSender::initialize() {
@@ -178,6 +180,7 @@ status_t NativeInputEventSender::receiveFinishedSignals(JNIEnv* env) {
     ALOGD("channel '%s' ~ Receiving finished signals.", getInputChannelName());
 #endif
 
+    ScopedLocalRef<jobject> senderObj(env, NULL);
     bool skipCallbacks = false;
     for (;;) {
         uint32_t publishedSeq;
@@ -205,7 +208,16 @@ status_t NativeInputEventSender::receiveFinishedSignals(JNIEnv* env) {
 #endif
 
             if (!skipCallbacks) {
-                env->CallVoidMethod(mSenderObjGlobal,
+                if (!senderObj.get()) {
+                    senderObj.reset(jniGetReferent(env, mSenderWeakGlobal));
+                    if (!senderObj.get()) {
+                        ALOGW("channel '%s' ~ Sender object was finalized "
+                                "without being disposed.", getInputChannelName());
+                        return DEAD_OBJECT;
+                    }
+                }
+
+                env->CallVoidMethod(senderObj.get(),
                         gInputEventSenderClassInfo.dispatchInputEventFinished,
                         jint(seq), jboolean(handled));
                 if (env->ExceptionCheck()) {
@@ -218,7 +230,7 @@ status_t NativeInputEventSender::receiveFinishedSignals(JNIEnv* env) {
 }
 
 
-static jint nativeInit(JNIEnv* env, jclass clazz, jobject senderObj,
+static jint nativeInit(JNIEnv* env, jclass clazz, jobject senderWeak,
         jobject inputChannelObj, jobject messageQueueObj) {
     sp<InputChannel> inputChannel = android_view_InputChannel_getInputChannel(env,
             inputChannelObj);
@@ -234,7 +246,7 @@ static jint nativeInit(JNIEnv* env, jclass clazz, jobject senderObj,
     }
 
     sp<NativeInputEventSender> sender = new NativeInputEventSender(env,
-            senderObj, inputChannel, messageQueue);
+            senderWeak, inputChannel, messageQueue);
     status_t status = sender->initialize();
     if (status) {
         String8 message;
@@ -277,7 +289,7 @@ static jboolean nativeSendMotionEvent(JNIEnv* env, jclass clazz, jint senderPtr,
 static JNINativeMethod gMethods[] = {
     /* name, signature, funcPtr */
     { "nativeInit",
-            "(Landroid/view/InputEventSender;Landroid/view/InputChannel;Landroid/os/MessageQueue;)I",
+            "(Ljava/lang/ref/WeakReference;Landroid/view/InputChannel;Landroid/os/MessageQueue;)I",
             (void*)nativeInit },
     { "nativeDispose", "(I)V",
             (void*)nativeDispose },