Fix spurious ANRs in native activities.

Some native activities experienced ANRs when the input consumer
deferred an input event due to client-side batching.  If the
input channel was fully emptied then the client had no way of
knowing that it should wake up to handle the deferred input event.

This patch also fixes some lock issues in the native activity
input queue implementation.  In at least one error case, it
was possible for a function to exit without releasing the lock.

Bug: 6051176
Change-Id: I4d9d843237e69b9834f8d8b360031b677fcab8c3

6 files changed, 135 insertions, 66 deletions

diff --git a/core/jni/android_app_NativeActivity.cpp b/core/jni/android_app_NativeActivity.cpp
index 088062a..655a834 100644
--- a/core/jni/android_app_NativeActivity.cpp
+++ b/core/jni/android_app_NativeActivity.cpp
@@ -162,12 +162,12 @@ int32_t AInputQueue::hasEvents() {
 int32_t AInputQueue::getEvent(AInputEvent** outEvent) {
     *outEvent = NULL;
 
-    bool finishNow = false;
-
     char byteread;
     ssize_t nRead = read(mDispatchKeyRead, &byteread, 1);
+
+    Mutex::Autolock _l(mLock);
+
     if (nRead == 1) {
-        mLock.lock();
         if (mDispatchingKeys.size() > 0) {
             KeyEvent* kevent = mDispatchingKeys[0];
             *outEvent = kevent;
@@ -178,6 +178,8 @@ int32_t AInputQueue::getEvent(AInputEvent** outEvent) {
             inflight.finishSeq = 0;
             mInFlightEvents.push(inflight);
         }
+
+        bool finishNow = false;
         if (mFinishPreDispatches.size() > 0) {
             finish_pre_dispatch finish(mFinishPreDispatches[0]);
             mFinishPreDispatches.removeAt(0);
@@ -193,7 +195,6 @@ int32_t AInputQueue::getEvent(AInputEvent** outEvent) {
                 ALOGW("getEvent couldn't find inflight for seq %d", finish.seq);
             }
         }
-        mLock.unlock();
 
         if (finishNow) {
             finishEvent(*outEvent, true, false);
@@ -206,7 +207,8 @@ int32_t AInputQueue::getEvent(AInputEvent** outEvent) {
 
     uint32_t consumerSeq;
     InputEvent* myEvent = NULL;
-    status_t res = mConsumer.consume(this, true /*consumeBatches*/, &consumerSeq, &myEvent);
+    status_t res = mConsumer.consume(&mPooledInputEventFactory, true /*consumeBatches*/,
+            &consumerSeq, &myEvent);
     if (res != android::OK) {
         if (res != android::WOULD_BLOCK) {
             ALOGW("channel '%s' ~ Failed to consume input event.  status=%d",
@@ -215,6 +217,10 @@ int32_t AInputQueue::getEvent(AInputEvent** outEvent) {
         return -1;
     }
 
+    if (mConsumer.hasDeferredEvent()) {
+        wakeupDispatchLocked();
+    }
+
     in_flight_event inflight;
     inflight.event = myEvent;
     inflight.seq = -1;
@@ -255,7 +261,8 @@ void AInputQueue::finishEvent(AInputEvent* event, bool handled, bool didDefaultH
         return;
     }
 
-    mLock.lock();
+    Mutex::Autolock _l(mLock);
+
     const size_t N = mInFlightEvents.size();
     for (size_t i=0; i<N; i++) {
         const in_flight_event& inflight(mInFlightEvents[i]);
@@ -267,111 +274,82 @@ void AInputQueue::finishEvent(AInputEvent* event, bool handled, bool didDefaultH
                             mConsumer.getChannel()->getName().string(), res);
                 }
             }
-            if (static_cast<InputEvent*>(event)->getType() == AINPUT_EVENT_TYPE_KEY) {
-                mAvailKeyEvents.push(static_cast<KeyEvent*>(event));
-            } else {
-                mAvailMotionEvents.push(static_cast<MotionEvent*>(event));
-            }
+            mPooledInputEventFactory.recycle(static_cast<InputEvent*>(event));
             mInFlightEvents.removeAt(i);
-            mLock.unlock();
             return;
         }
     }
-    mLock.unlock();
-    
+
     ALOGW("finishEvent called for unknown event: %p", event);
 }
 
 void AInputQueue::dispatchEvent(android::KeyEvent* event) {
-    mLock.lock();
+    Mutex::Autolock _l(mLock);
+
     LOG_TRACE("dispatchEvent: dispatching=%d write=%d\n", mDispatchingKeys.size(),
             mDispatchKeyWrite);
     mDispatchingKeys.add(event);
-    wakeupDispatch();
-    mLock.unlock();
+    wakeupDispatchLocked();
 }
 
 void AInputQueue::finishPreDispatch(int seq, bool handled) {
-    mLock.lock();
+    Mutex::Autolock _l(mLock);
+
     LOG_TRACE("finishPreDispatch: seq=%d handled=%d\n", seq, handled ? 1 : 0);
     finish_pre_dispatch finish;
     finish.seq = seq;
     finish.handled = handled;
     mFinishPreDispatches.add(finish);
-    wakeupDispatch();
-    mLock.unlock();
+    wakeupDispatchLocked();
 }
 
 KeyEvent* AInputQueue::consumeUnhandledEvent() {
-    KeyEvent* event = NULL;
+    Mutex::Autolock _l(mLock);
 
-    mLock.lock();
+    KeyEvent* event = NULL;
     if (mUnhandledKeys.size() > 0) {
         event = mUnhandledKeys[0];
         mUnhandledKeys.removeAt(0);
     }
-    mLock.unlock();
 
     LOG_TRACE("consumeUnhandledEvent: KeyEvent=%p", event);
-
     return event;
 }
 
 KeyEvent* AInputQueue::consumePreDispatchingEvent(int* outSeq) {
-    KeyEvent* event = NULL;
+    Mutex::Autolock _l(mLock);
 
-    mLock.lock();
+    KeyEvent* event = NULL;
     if (mPreDispatchingKeys.size() > 0) {
         const in_flight_event& inflight(mPreDispatchingKeys[0]);
         event = static_cast<KeyEvent*>(inflight.event);
         *outSeq = inflight.seq;
         mPreDispatchingKeys.removeAt(0);
     }
-    mLock.unlock();
 
     LOG_TRACE("consumePreDispatchingEvent: KeyEvent=%p", event);
-
     return event;
 }
 
 KeyEvent* AInputQueue::createKeyEvent() {
-    mLock.lock();
-    KeyEvent* event;
-    if (mAvailKeyEvents.size() <= 0) {
-        event = new KeyEvent();
-    } else {
-        event = mAvailKeyEvents.top();
-        mAvailKeyEvents.pop();
-    }
-    mLock.unlock();
-    return event;
-}
+    Mutex::Autolock _l(mLock);
 
-MotionEvent* AInputQueue::createMotionEvent() {
-    mLock.lock();
-    MotionEvent* event;
-    if (mAvailMotionEvents.size() <= 0) {
-        event = new MotionEvent();
-    } else {
-        event = mAvailMotionEvents.top();
-        mAvailMotionEvents.pop();
-    }
-    mLock.unlock();
-    return event;
+    return mPooledInputEventFactory.createKeyEvent();
 }
 
 void AInputQueue::doUnhandledKey(KeyEvent* keyEvent) {
-    mLock.lock();
+    Mutex::Autolock _l(mLock);
+
     LOG_TRACE("Unhandled key: pending=%d write=%d\n", mUnhandledKeys.size(), mWorkWrite);
     if (mUnhandledKeys.size() <= 0 && mWorkWrite >= 0) {
         write_work(mWorkWrite, CMD_DEF_KEY);
     }
     mUnhandledKeys.add(keyEvent);
-    mLock.unlock();
 }
 
 bool AInputQueue::preDispatchKey(KeyEvent* keyEvent) {
-    mLock.lock();
+    Mutex::Autolock _l(mLock);
+
     LOG_TRACE("preDispatch key: pending=%d write=%d\n", mPreDispatchingKeys.size(), mWorkWrite);
     const size_t N = mInFlightEvents.size();
     for (size_t i=0; i<N; i++) {
@@ -380,7 +358,6 @@ bool AInputQueue::preDispatchKey(KeyEvent* keyEvent) {
             if (inflight.seq >= 0) {
                 // This event has already been pre-dispatched!
                 LOG_TRACE("Event already pre-dispatched!");
-                mLock.unlock();
                 return false;
             }
             mSeq++;
@@ -391,7 +368,6 @@ bool AInputQueue::preDispatchKey(KeyEvent* keyEvent) {
                 write_work(mWorkWrite, CMD_DEF_KEY);
             }
             mPreDispatchingKeys.add(inflight);
-            mLock.unlock();
             return true;
         }
     }
@@ -400,7 +376,7 @@ bool AInputQueue::preDispatchKey(KeyEvent* keyEvent) {
     return false;
 }
 
-void AInputQueue::wakeupDispatch() {
+void AInputQueue::wakeupDispatchLocked() {
 restart:
     char dummy = 0;
     int res = write(mDispatchKeyWrite, &dummy, sizeof(dummy));
diff --git a/include/android_runtime/android_app_NativeActivity.h b/include/android_runtime/android_app_NativeActivity.h
index a59677a..7977c23 100644
--- a/include/android_runtime/android_app_NativeActivity.h
+++ b/include/android_runtime/android_app_NativeActivity.h
@@ -65,7 +65,7 @@ extern void android_NativeActivity_hideSoftInput(
  *      b. Java sends event through default key handler.
  *      c. event is finished.
  */
-struct AInputQueue : public android::InputEventFactoryInterface {
+struct AInputQueue {
 public:
     /* Creates a consumer associated with an input channel. */
     explicit AInputQueue(const android::sp<android::InputChannel>& channel, int workWrite);
@@ -96,16 +96,16 @@ public:
     android::KeyEvent* consumeUnhandledEvent();
     android::KeyEvent* consumePreDispatchingEvent(int* outSeq);
 
-    virtual android::KeyEvent* createKeyEvent();
-    virtual android::MotionEvent* createMotionEvent();
+    android::KeyEvent* createKeyEvent();
 
     int mWorkWrite;
 
 private:
     void doUnhandledKey(android::KeyEvent* keyEvent);
     bool preDispatchKey(android::KeyEvent* keyEvent);
-    void wakeupDispatch();
+    void wakeupDispatchLocked();
 
+    android::PooledInputEventFactory mPooledInputEventFactory;
     android::InputConsumer mConsumer;
     android::sp<android::Looper> mLooper;
 
@@ -127,11 +127,6 @@ private:
 
     int mSeq;
 
-    // Cache of previously allocated key events.
-    android::Vector<android::KeyEvent*> mAvailKeyEvents;
-    // Cache of previously allocated motion events.
-    android::Vector<android::MotionEvent*> mAvailMotionEvents;
-
     // All input events that are actively being processed.
     android::Vector<in_flight_event> mInFlightEvents;
 
diff --git a/include/androidfw/Input.h b/include/androidfw/Input.h
index f5db6e2..a4ebd95 100644
--- a/include/androidfw/Input.h
+++ b/include/androidfw/Input.h
@@ -616,6 +616,26 @@ private:
 };
 
 /*
+ * An input event factory implementation that maintains a pool of input events.
+ */
+class PooledInputEventFactory : public InputEventFactoryInterface {
+public:
+    PooledInputEventFactory(size_t maxPoolSize = 20);
+    virtual ~PooledInputEventFactory();
+
+    virtual KeyEvent* createKeyEvent();
+    virtual MotionEvent* createMotionEvent();
+
+    void recycle(InputEvent* event);
+
+private:
+    const size_t mMaxPoolSize;
+
+    Vector<KeyEvent*> mKeyEventPool;
+    Vector<MotionEvent*> mMotionEventPool;
+};
+
+/*
  * Calculates the velocity of pointer movements over time.
  */
 class VelocityTracker {
diff --git a/include/androidfw/InputTransport.h b/include/androidfw/InputTransport.h
index a846e65..29c296e 100644
--- a/include/androidfw/InputTransport.h
+++ b/include/androidfw/InputTransport.h
@@ -292,7 +292,29 @@ public:
      */
     status_t sendFinishedSignal(uint32_t seq, bool handled);
 
-    /* Returns true if there is a pending batch. */
+    /* Returns true if there is a deferred event waiting.
+     *
+     * Should be called after calling consume() to determine whether the consumer
+     * has a deferred event to be processed.  Deferred events are somewhat special in
+     * that they have already been removed from the input channel.  If the input channel
+     * becomes empty, the client may need to do extra work to ensure that it processes
+     * the deferred event despite the fact that the inptu channel's file descriptor
+     * is not readable.
+     *
+     * One option is simply to call consume() in a loop until it returns WOULD_BLOCK.
+     * This guarantees that all deferred events will be processed.
+     *
+     * Alternately, the caller can call hasDeferredEvent() to determine whether there is
+     * a deferred event waiting and then ensure that its event loop wakes up at least
+     * one more time to consume the deferred event.
+     */
+    bool hasDeferredEvent() const;
+
+    /* Returns true if there is a pending batch.
+     *
+     * Should be called after calling consume() with consumeBatches == false to determine
+     * whether consume() should be called again later on with consumeBatches == true.
+     */
     bool hasPendingBatch() const;
 
 private:
diff --git a/libs/androidfw/Input.cpp b/libs/androidfw/Input.cpp
index ca09caf..da57839 100644
--- a/libs/androidfw/Input.cpp
+++ b/libs/androidfw/Input.cpp
@@ -683,6 +683,58 @@ bool MotionEvent::isTouchEvent(int32_t source, int32_t action) {
 }
 
 
+// --- PooledInputEventFactory ---
+
+PooledInputEventFactory::PooledInputEventFactory(size_t maxPoolSize) :
+        mMaxPoolSize(maxPoolSize) {
+}
+
+PooledInputEventFactory::~PooledInputEventFactory() {
+    for (size_t i = 0; i < mKeyEventPool.size(); i++) {
+        delete mKeyEventPool.itemAt(i);
+    }
+    for (size_t i = 0; i < mMotionEventPool.size(); i++) {
+        delete mMotionEventPool.itemAt(i);
+    }
+}
+
+KeyEvent* PooledInputEventFactory::createKeyEvent() {
+    if (!mKeyEventPool.isEmpty()) {
+        KeyEvent* event = mKeyEventPool.top();
+        mKeyEventPool.pop();
+        return event;
+    }
+    return new KeyEvent();
+}
+
+MotionEvent* PooledInputEventFactory::createMotionEvent() {
+    if (!mMotionEventPool.isEmpty()) {
+        MotionEvent* event = mMotionEventPool.top();
+        mMotionEventPool.pop();
+        return event;
+    }
+    return new MotionEvent();
+}
+
+void PooledInputEventFactory::recycle(InputEvent* event) {
+    switch (event->getType()) {
+    case AINPUT_EVENT_TYPE_KEY:
+        if (mKeyEventPool.size() < mMaxPoolSize) {
+            mKeyEventPool.push(static_cast<KeyEvent*>(event));
+            return;
+        }
+        break;
+    case AINPUT_EVENT_TYPE_MOTION:
+        if (mMotionEventPool.size() < mMaxPoolSize) {
+            mMotionEventPool.push(static_cast<MotionEvent*>(event));
+            return;
+        }
+        break;
+    }
+    delete event;
+}
+
+
 // --- VelocityTracker ---
 
 const uint32_t VelocityTracker::DEFAULT_DEGREE;
diff --git a/libs/androidfw/InputTransport.cpp b/libs/androidfw/InputTransport.cpp
index 1ebd75c..294236f 100644
--- a/libs/androidfw/InputTransport.cpp
+++ b/libs/androidfw/InputTransport.cpp
@@ -527,6 +527,10 @@ status_t InputConsumer::sendUnchainedFinishedSignal(uint32_t seq, bool handled)
     return mChannel->sendMessage(&msg);
 }
 
+bool InputConsumer::hasDeferredEvent() const {
+    return mMsgDeferred;
+}
+
 bool InputConsumer::hasPendingBatch() const {
     return !mBatches.isEmpty();
 }