From 013cf847bcfd2828d34dced60adf2d3dd98021dc Mon Sep 17 00:00:00 2001
From: Jeff Brown <jeffbrown@google.com>
Date: Fri, 6 Sep 2013 16:24:36 -0700
Subject: Fix native crash when message queue quits.

Fix a race when quitting the looper's message queue that could
cause the mPtr field to be zeroed out and the native object to
be destroyed while still in use.

This happened due to an optimization that was intended to release
the native looper's file descriptor as soon as the last message
was processed rather than waiting for the finalizer to run.

Bug: 9726217
Change-Id: I695a9a657acfdb3ce65a5737ff20cd11113d15fa
---
 core/java/android/os/MessageQueue.java | 47 ++++++++++++++++++++++------------
 1 file changed, 31 insertions(+), 16 deletions(-)

(limited to 'core/java/android/os')

diff --git a/core/java/android/os/MessageQueue.java b/core/java/android/os/MessageQueue.java
index 1e8983e..d1b8213 100644
--- a/core/java/android/os/MessageQueue.java
+++ b/core/java/android/os/MessageQueue.java
@@ -39,7 +39,7 @@ public final class MessageQueue {
     Message mMessages;
     private final ArrayList<IdleHandler> mIdleHandlers = new ArrayList<IdleHandler>();
     private IdleHandler[] mPendingIdleHandlers;
-    private boolean mQuiting;
+    private boolean mQuitting;
 
     // Indicates whether next() is blocked waiting in pollOnce() with a non-zero timeout.
     private boolean mBlocked;
@@ -115,6 +115,8 @@ public final class MessageQueue {
         }
     }
 
+    // Disposes of the underlying message queue.
+    // Must only be called on the looper thread or the finalizer.
     private void dispose() {
         if (mPtr != 0) {
             nativeDestroy(mPtr);
@@ -125,11 +127,13 @@ public final class MessageQueue {
     Message next() {
         int pendingIdleHandlerCount = -1; // -1 only during first iteration
         int nextPollTimeoutMillis = 0;
-
         for (;;) {
             if (nextPollTimeoutMillis != 0) {
                 Binder.flushPendingCommands();
             }
+
+            // We can assume mPtr != 0 because the loop is obviously still running.
+            // The looper will not call this method after the loop quits.
             nativePollOnce(mPtr, nextPollTimeoutMillis);
 
             synchronized (this) {
@@ -167,7 +171,7 @@ public final class MessageQueue {
                 }
 
                 // Process the quit message now that all pending messages have been handled.
-                if (mQuiting) {
+                if (mQuitting) {
                     dispose();
                     return null;
                 }
@@ -226,18 +230,20 @@ public final class MessageQueue {
         }
 
         synchronized (this) {
-            if (mQuiting) {
+            if (mQuitting) {
                 return;
             }
-            mQuiting = true;
+            mQuitting = true;
 
             if (safe) {
                 removeAllFutureMessagesLocked();
             } else {
                 removeAllMessagesLocked();
             }
+
+            // We can assume mPtr != 0 because mQuitting was previously false.
+            nativeWake(mPtr);
         }
-        nativeWake(mPtr);
     }
 
     int enqueueSyncBarrier(long when) {
@@ -270,7 +276,6 @@ public final class MessageQueue {
     void removeSyncBarrier(int token) {
         // Remove a sync barrier token from the queue.
         // If the queue is no longer stalled by a barrier then wake it.
-        final boolean needWake;
         synchronized (this) {
             Message prev = null;
             Message p = mMessages;
@@ -282,6 +287,7 @@ public final class MessageQueue {
                 throw new IllegalStateException("The specified message queue synchronization "
                         + " barrier token has not been posted or has already been removed.");
             }
+            final boolean needWake;
             if (prev != null) {
                 prev.next = p.next;
                 needWake = false;
@@ -290,9 +296,12 @@ public final class MessageQueue {
                 needWake = mMessages == null || mMessages.target != null;
             }
             p.recycle();
-        }
-        if (needWake) {
-            nativeWake(mPtr);
+
+            // If the loop is quitting then it is already awake.
+            // We can assume mPtr != 0 when mQuitting is false.
+            if (needWake && !mQuitting) {
+                nativeWake(mPtr);
+            }
         }
     }
 
@@ -304,9 +313,8 @@ public final class MessageQueue {
             throw new AndroidRuntimeException("Message must have a target.");
         }
 
-        boolean needWake;
         synchronized (this) {
-            if (mQuiting) {
+            if (mQuitting) {
                 RuntimeException e = new RuntimeException(
                         msg.target + " sending message to a Handler on a dead thread");
                 Log.w("MessageQueue", e.getMessage(), e);
@@ -315,6 +323,7 @@ public final class MessageQueue {
 
             msg.when = when;
             Message p = mMessages;
+            boolean needWake;
             if (p == null || when == 0 || when < p.when) {
                 // New head, wake up the event queue if blocked.
                 msg.next = p;
@@ -339,9 +348,11 @@ public final class MessageQueue {
                 msg.next = p; // invariant: p == prev.next
                 prev.next = msg;
             }
-        }
-        if (needWake) {
-            nativeWake(mPtr);
+
+            // We can assume mPtr != 0 because mQuitting is false.
+            if (needWake) {
+                nativeWake(mPtr);
+            }
         }
         return true;
     }
@@ -381,7 +392,11 @@ public final class MessageQueue {
     }
 
     boolean isIdling() {
-        return nativeIsIdling(mPtr);
+        synchronized (this) {
+            // If the loop is quitting then it must not be idling.
+            // We can assume mPtr != 0 when mQuitting is false.
+            return !mQuitting && nativeIsIdling(mPtr);
+        }
     }
 
     void removeMessages(Handler h, int what, Object object) {
-- 
cgit v1.1