Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 6083d81c authored by Wink Saville's avatar Wink Saville
Browse files

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
parent 5aaeaffd

core/java/android/os/Message.java

+14 −1
Original line number Diff line number Diff line
@@ -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);

    }

}

core/java/android/os/MessageQueue.java

+2 −1
Original line number Diff line number Diff line
@@ -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.");

        }

core/tests/coretests/src/android/os/MessageQueueTest.java

+4 −1
Original line number Diff line number Diff line
@@ -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);

    }

}