From 6083d81ce4d67ec632962270fda64ebb9db0d5b1 Mon Sep 17 00:00:00 2001 From: Wink Saville Date: Sat, 21 Aug 2010 15:32:19 -0700 Subject: [PATCH] Allow reliable detection of a message that is in use. Because the standard Looper.loop code calls Message#recycle it is imperative that Handler#handleMessage code not attempt to resue a message it receives. If allowed to do so it will cause bugs that could be difficult to diagnois. This change adds Message#flags and uses one bit to reliably detect a message is in use and throws an error in MessageQueue#enqueueMessage. This allows early detection of this bug. Note: This is not new functionality, but the current implementation does not detect messages that are in use because it uses Message#when != 0 as the detection mechanism. The problem is that a Message#when value of 0 is valid value used to place a message at the front of the queue and is thus unreliable. Another option is to change the setting of Message#when in Message#enqueueMessage so that it is never 0, although that does change subtly a publicly accessible field. Yet another option would be to use other fields but all candidates have similar problems as when in that they are publicly accessible or even settable such as Message#target. Change-Id: I2df600537700a3fe206678f38bcae7329751c4e5 --- core/java/android/os/Message.java | 15 ++++++++++++++- core/java/android/os/MessageQueue.java | 3 ++- .../src/android/os/MessageQueueTest.java | 5 ++++- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/core/java/android/os/Message.java b/core/java/android/os/Message.java index 49b72fee22d3f..6d8be77a2c6a6 100644 --- a/core/java/android/os/Message.java +++ b/core/java/android/os/Message.java @@ -74,6 +74,10 @@ public final class Message implements Parcelable { */ public Messenger replyTo; + /*package*/ static final int FLAG_IN_USE = 1; + + /*package*/ int flags; + /*package*/ long when; /*package*/ Bundle data; @@ -253,6 +257,7 @@ public final class Message implements Parcelable { * target/callback of the original message. */ public void copyFrom(Message o) { + this.flags = o.flags; this.what = o.what; this.arg1 = o.arg1; this.arg2 = o.arg2; @@ -350,6 +355,7 @@ public final class Message implements Parcelable { } /*package*/ void clearForRecycle() { + flags = 0; what = 0; arg1 = 0; arg2 = 0; @@ -361,6 +367,14 @@ public final class Message implements Parcelable { data = null; } + /*package*/ boolean isInUse() { + return ((flags & FLAG_IN_USE) == FLAG_IN_USE); + } + + /*package*/ void markInUse() { + flags |= FLAG_IN_USE; + } + /** Constructor (but the preferred way to get a Message is to call {@link #obtain() Message.obtain()}). */ public Message() { @@ -453,4 +467,3 @@ public final class Message implements Parcelable { replyTo = Messenger.readMessengerOrNullFromParcel(source); } } - diff --git a/core/java/android/os/MessageQueue.java b/core/java/android/os/MessageQueue.java index adb11c82f9878..75dfdd2549605 100644 --- a/core/java/android/os/MessageQueue.java +++ b/core/java/android/os/MessageQueue.java @@ -120,6 +120,7 @@ public class MessageQueue { now = SystemClock.uptimeMillis(); Message msg = pullNextLocked(now); if (msg != null) { + msg.markInUse(); return msg; } @@ -192,7 +193,7 @@ public class MessageQueue { } final boolean enqueueMessage(Message msg, long when) { - if (msg.when != 0) { + if (msg.isInUse()) { throw new AndroidRuntimeException(msg + " This message is already in use."); } diff --git a/core/tests/coretests/src/android/os/MessageQueueTest.java b/core/tests/coretests/src/android/os/MessageQueueTest.java index b7c2d1f1c1495..12931ec39dc39 100644 --- a/core/tests/coretests/src/android/os/MessageQueueTest.java +++ b/core/tests/coretests/src/android/os/MessageQueueTest.java @@ -41,6 +41,10 @@ public class MessageQueueTest extends TestCase { } public void handleMessage(Message msg) { + if (!msg.isInUse()) { + failure(new RuntimeException( + "msg.isInuse is false, should always be true, #" + msg.what)); + } if (mCount <= mLastMessage) { if (msg.what != mCount) { failure(new RuntimeException( @@ -100,4 +104,3 @@ public class MessageQueueTest extends TestCase { tester.doTest(1000); } } -