From 8b60e4514702edd1eb4b6f2bfc027e04a94369c0 Mon Sep 17 00:00:00 2001 From: Jeff Brown Date: Thu, 18 Apr 2013 15:17:48 -0700 Subject: Fix change of behavior in Looper.quit(). It seems some applications rely on Looper.quit() terminating the loop immediately without processing all messages. Rather than risk breaking them, make the safer behavior optional. Also take care to properly drain the message queue before quitting so that all of the Message instances are recycled. This may help release storage sooner in case the Looper doesn't get GC'd promptly and its remaining queue of undelivered messages sticks around. Improve docs on runWithScissors. Bug: 8596303 Change-Id: I8cbeb6f7a5f6b8e618b5109f87a03defc1486b9f --- core/java/android/os/Handler.java | 15 ++++++---- core/java/android/os/HandlerThread.java | 53 +++++++++++++++++++++++++++++---- core/java/android/os/Looper.java | 33 +++++++++++++++----- core/java/android/os/MessageQueue.java | 46 +++++++++++++++++++++++++++- 4 files changed, 128 insertions(+), 19 deletions(-) (limited to 'core/java/android') diff --git a/core/java/android/os/Handler.java b/core/java/android/os/Handler.java index 94de4487b645..14d8f0765f9d 100644 --- a/core/java/android/os/Handler.java +++ b/core/java/android/os/Handler.java @@ -413,27 +413,32 @@ public class Handler { /** * Runs the specified task synchronously. - * + *

* If the current thread is the same as the handler thread, then the runnable * runs immediately without being enqueued. Otherwise, posts the runnable * to the handler and waits for it to complete before returning. - * + *

* This method is dangerous! Improper use can result in deadlocks. * Never call this method while any locks are held or use it in a * possibly re-entrant manner. - * + *

* This method is occasionally useful in situations where a background thread * must synchronously await completion of a task that must run on the * handler's thread. However, this problem is often a symptom of bad design. * Consider improving the design (if possible) before resorting to this method. - * + *

* One example of where you might want to use this method is when you just * set up a Handler thread and need to perform some initialization steps on * it before continuing execution. - * + *

* If timeout occurs then this method returns false but the runnable * will remain posted on the handler and may already be in progress or * complete at a later time. + *

+ * When using this method, be sure to use {@link Looper#quitSafely} when + * quitting the looper. Otherwise {@link #runWithScissors} may hang indefinitely. + * (TODO: We should fix this by making MessageQueue aware of blocking runnables.) + *

* * @param r The Runnable that will be executed synchronously. * @param timeout The timeout in milliseconds, or 0 to wait indefinitely. diff --git a/core/java/android/os/HandlerThread.java b/core/java/android/os/HandlerThread.java index daf1f5933520..2904105ef782 100644 --- a/core/java/android/os/HandlerThread.java +++ b/core/java/android/os/HandlerThread.java @@ -48,6 +48,7 @@ public class HandlerThread extends Thread { protected void onLooperPrepared() { } + @Override public void run() { mTid = Process.myTid(); Looper.prepare(); @@ -83,12 +84,25 @@ public class HandlerThread extends Thread { } return mLooper; } - + /** - * Ask the currently running looper to quit. If the thread has not - * been started or has finished (that is if {@link #getLooper} returns - * null), then false is returned. Otherwise the looper is asked to - * quit and true is returned. + * Quits the handler thread's looper. + *

+ * Causes the handler thread's looper to terminate without processing any + * more messages in the message queue. + *

+ * Any attempt to post messages to the queue after the looper is asked to quit will fail. + * For example, the {@link Handler#sendMessage(Message)} method will return false. + *

+ * Using this method may be unsafe because some messages may not be delivered + * before the looper terminates. Consider using {@link #quitSafely} instead to ensure + * that all pending work is completed in an orderly manner. + *

+ * + * @return True if the looper looper has been asked to quit or false if the + * thread had not yet started running. + * + * @see #quitSafely */ public boolean quit() { Looper looper = getLooper(); @@ -98,7 +112,34 @@ public class HandlerThread extends Thread { } return false; } - + + /** + * Quits the handler thread's looper safely. + *

+ * Causes the handler thread's looper to terminate as soon as all remaining messages + * in the message queue that are already due to be delivered have been handled. + * Pending delayed messages with due times in the future will not be delivered. + *

+ * Any attempt to post messages to the queue after the looper is asked to quit will fail. + * For example, the {@link Handler#sendMessage(Message)} method will return false. + *

+ * If the thread has not been started or has finished (that is if + * {@link #getLooper} returns null), then false is returned. + * Otherwise the looper is asked to quit and true is returned. + *

+ * + * @return True if the looper looper has been asked to quit or false if the + * thread had not yet started running. + */ + public boolean quitSafely() { + Looper looper = getLooper(); + if (looper != null) { + looper.quitSafely(); + return true; + } + return false; + } + /** * Returns the identifier of this thread. See Process.myTid(). */ diff --git a/core/java/android/os/Looper.java b/core/java/android/os/Looper.java index fa28765206f0..d5cf771a22bc 100644 --- a/core/java/android/os/Looper.java +++ b/core/java/android/os/Looper.java @@ -202,18 +202,37 @@ public final class Looper { /** * Quits the looper. *

+ * Causes the {@link #loop} method to terminate without processing any + * more messages in the message queue. + *

+ * Any attempt to post messages to the queue after the looper is asked to quit will fail. + * For example, the {@link Handler#sendMessage(Message)} method will return false. + *

+ * Using this method may be unsafe because some messages may not be delivered + * before the looper terminates. Consider using {@link #quitSafely} instead to ensure + * that all pending work is completed in an orderly manner. + *

+ * + * @see #quitSafely + */ + public void quit() { + mQueue.quit(false); + } + + /** + * Quits the looper safely. + *

* Causes the {@link #loop} method to terminate as soon as all remaining messages * in the message queue that are already due to be delivered have been handled. - * However delayed messages with due times in the future may not be handled before - * the loop terminates. + * However pending delayed messages with due times in the future will not be + * delivered before the loop terminates. *

- * Any attempt to post messages to the queue after {@link #quit} has been called - * will fail. For example, the {@link Handler#sendMessage(Message)} method will - * return false when the looper is being terminated. + * Any attempt to post messages to the queue after the looper is asked to quit will fail. + * For example, the {@link Handler#sendMessage(Message)} method will return false. *

*/ - public void quit() { - mQueue.quit(); + public void quitSafely() { + mQueue.quit(true); } /** diff --git a/core/java/android/os/MessageQueue.java b/core/java/android/os/MessageQueue.java index c058bfce28e6..bf7e5caf7024 100644 --- a/core/java/android/os/MessageQueue.java +++ b/core/java/android/os/MessageQueue.java @@ -219,7 +219,7 @@ public final class MessageQueue { } } - void quit() { + void quit(boolean safe) { if (!mQuitAllowed) { throw new RuntimeException("Main thread not allowed to quit."); } @@ -229,6 +229,12 @@ public final class MessageQueue { return; } mQuiting = true; + + if (safe) { + removeAllFutureMessagesLocked(); + } else { + removeAllMessagesLocked(); + } } nativeWake(mPtr); } @@ -473,4 +479,42 @@ public final class MessageQueue { } } } + + private void removeAllMessagesLocked() { + Message p = mMessages; + while (p != null) { + Message n = p.next; + p.recycle(); + p = n; + } + mMessages = null; + } + + private void removeAllFutureMessagesLocked() { + final long now = SystemClock.uptimeMillis(); + Message p = mMessages; + if (p != null) { + if (p.when > now) { + removeAllMessagesLocked(); + } else { + Message n; + for (;;) { + n = p.next; + if (n == null) { + return; + } + if (n.when > now) { + break; + } + p = n; + } + p.next = null; + do { + p = n; + n = p.next; + p.recycle(); + } while (n != null); + } + } + } } -- cgit v1.2.3