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

Commit 3743222f authored by Shai Barack's avatar Shai Barack
Browse files

Prevent race between nativeWake and dispose, and potential deadlock

Fixes: 407098224
Fixes: 409572723
Flag: build.RELEASE_PACKAGE_MESSAGEQUEUE_IMPLEMENTATION
Test: MessageQueueTest#testStressQuit
Change-Id: Ib109f0cfa0991121c8fb8c1694bafa60d964e99a
parent 0d2f511b
Loading
Loading
Loading
Loading
+105 −39
Original line number Diff line number Diff line
@@ -73,6 +73,16 @@ public final class MessageQueue {
    @UnsupportedAppUsage
    private final boolean mQuitAllowed;

    /**
     * Used by all native methods.
     *
     * <p>In legacy mode, usage of this field (directly, or indirectly via native method
     * invocations) must be guarded with the lock.
     *
     * <p>In concurrent mode, the Looper thread may access freely, but other threads must first call
     * {@link #incrementMptrRefs()}, check the result, and if true then access the native
     * object, followed by a call to {@link #decrementMptrRefs()}.
     */
    @UnsupportedAppUsage
    @SuppressWarnings("unused")
    private long mPtr; // used by native code
@@ -99,6 +109,7 @@ public final class MessageQueue {
    private int mAsyncMessageCount;

    private final AtomicLong mMessageCount = new AtomicLong();
    private final Thread mLooperThread;
    private final String mThreadName;
    private final long mTid;

@@ -148,7 +159,8 @@ public final class MessageQueue {
        mUseConcurrent = sIsProcessAllowedToUseConcurrent;
        mQuitAllowed = quitAllowed;
        mPtr = nativeInit();
        mThreadName = Thread.currentThread().getName();
        mLooperThread = Thread.currentThread();
        mThreadName = mLooperThread.getName();
        mTid = Process.myTid();
    }

@@ -402,15 +414,15 @@ public final class MessageQueue {

    private boolean isPollingConcurrent() {
        // If the loop is quitting then it must not be idling.
        if (!incrementQuittingState()) {
            return false;
        }
        if (incrementMptrRefs()) {
            try {
                return nativeIsPolling(mPtr);
            } finally {
            decrementQuittingState();
                decrementMptrRefs();
            }
        }
        return false;
    }

    private boolean isPollingLegacy() {
        synchronized (this) {
@@ -565,11 +577,12 @@ public final class MessageQueue {
                record.mEvents = events;
                record.mSeq += 1;
            }
            nativeSetFileDescriptorEvents(mPtr, fdNum, events);
            setFileDescriptorEvents(fdNum, events);
        } else if (record != null) {
            record.mEvents = 0;
            mFileDescriptorRecords.removeAt(index);
            nativeSetFileDescriptorEvents(mPtr, fdNum, 0);

            setFileDescriptorEvents(fdNum, 0);
        }
    }

@@ -786,8 +799,12 @@ public final class MessageQueue {
             */
            StateNode nextOp = sStackStateActive;
            if (found == null) {
                if (next == null) {
                    /* No message to deliver, sleep indefinitely */
                if (getQuitting()) {
                    mNextPollTimeoutMillis = 0;
                    // State change will be Active->Active, so can immediately return here.
                    return null;
                } else if (next == null) {
                    /* No message to deliver, sleep indefinitely, unless quitting */
                    mNextPollTimeoutMillis = -1;
                    nextOp = sStackStateParked;
                    if (DEBUG) {
@@ -1053,11 +1070,8 @@ public final class MessageQueue {
        }

        if (mUseConcurrent) {
            if (!incrementQuittingState()) {
                return;
            }
            if (incrementMptrRefsAndSetQuitting()) {
                try {
                if (setQuitting()) {
                    if (safe) {
                        removeAllFutureMessages();
                    } else {
@@ -1066,9 +1080,9 @@ public final class MessageQueue {

                    // We can assume mPtr != 0 while we hold a ref on our quitting state
                    nativeWake(mPtr);
                }
                } finally {
                decrementQuittingState();
                    decrementMptrRefs();
                }
            }
        } else {
            synchronized (this) {
@@ -1232,7 +1246,7 @@ public final class MessageQueue {
            Message m = first.mMessage;
            if (m.target == null && m.arg1 == token) {
                /* Wake up next() in case it was sleeping on this barrier. */
                nativeWake(mPtr);
                concurrentWake();
            }
        } else if (!removed) {
            throw new IllegalStateException("The specified message queue synchronization "
@@ -2662,45 +2676,97 @@ public final class MessageQueue {
    }

    // Use MSB to indicate quitting state. Lower 63 bits hold ref count.
    private static final long QUITTING_MASK = ~(-1L >>> 1);
    private static final long QUITTING_MASK = 1L << 63;

    private boolean incrementQuittingState() {
        long oldVal = (long)sQuittingRefCount.getAndAdd(this, 1);
    /**
     * Increment the mPtr ref count.
     *
     * If this method returns true then the caller may use mPtr until they call
     * {@link #decrementMptrRefs()}.
     * If this method returns false then the caller must not use mPtr, and must
     * instead assume that the MessageQueue is quitting or has already quit and
     * act accordingly.
     */
    private boolean incrementMptrRefs() {
        while (true) {
            final long oldVal = mQuittingRefCountValue;
            if ((oldVal & QUITTING_MASK) != 0) {
            // If we're quitting we need to drop our ref and indicate to the caller
            sQuittingRefCount.getAndAdd(this, -1);
                // If we're quitting then we're not allowed to increment the ref count.
                return false;
            }
            if (sQuittingRefCount.compareAndSet(this, oldVal, oldVal + 1)) {
                // Successfully incremented the ref count without quitting.
                return true;
            }
        }
    }

    private void decrementQuittingState() {
    /**
     * Decrement the mPtr ref count.
     *
     * Call after {@link #incrementMptrRefs()} to release the ref on mPtr.
     */
    private void decrementMptrRefs() {
        long oldVal = (long)sQuittingRefCount.getAndAdd(this, -1);
        // If quitting and we were the last ref, wake up looper thread
        if ((oldVal & QUITTING_MASK) != 0 && (oldVal & ~QUITTING_MASK) == 1L) {
        if (oldVal - 1 == QUITTING_MASK) {
            LockSupport.unpark(mLooperThread);
        }
    }

    private boolean setQuitting() {
        long oldVal = (long)sQuittingRefCount.getAndBitwiseOr(this, QUITTING_MASK);
    private boolean incrementMptrRefsAndSetQuitting() {
        while (true) {
            final long oldVal = mQuittingRefCountValue;
            if ((oldVal & QUITTING_MASK) != 0) {
                // If we're quitting then we're not allowed to increment the ref count.
                return false;
            }
            if (sQuittingRefCount.compareAndSet(this, oldVal, (oldVal + 1) | QUITTING_MASK)) {
                // Successfully incremented the ref count and set quitting.
                return true;
            }
        }
    }

    /**
     * Wake the looper thread.
     *
     * {@link #nativeWake(long)} may be called directly only by the looper thread.
     * Otherwise, call this method to ensure safe access to mPtr.
     */
    private void concurrentWake() {
        if (incrementMptrRefs()) {
            try {
                nativeWake(mPtr);
            } finally {
                decrementMptrRefs();
            }
        }
    }

    private void setFileDescriptorEvents(int fdNum, int events) {
        if (mUseConcurrent) {
            if (incrementMptrRefs()) {
                try {
                    nativeSetFileDescriptorEvents(mPtr, fdNum, events);
                } finally {
                    decrementMptrRefs();
                }
            }
        } else {
            nativeSetFileDescriptorEvents(mPtr, fdNum, events);
        }
    }

    private boolean getQuitting() {
        return (mQuittingRefCountValue & QUITTING_MASK) != 0;
    }

    private volatile Thread mLooperThread = null;
    // Must only be called from looper thread
    private boolean checkQuittingAndWaitForRefsToDrop() {
        if (!getQuitting()) {
            return false;
        }
        mLooperThread = Thread.currentThread();
        boolean wasInterrupted = false;
        try {
            while ((mQuittingRefCountValue & ~QUITTING_MASK) != 0) {
@@ -2903,7 +2969,7 @@ public final class MessageQueue {
            if (sState.compareAndSet(this, old, node)) {
                if (inactive) {
                    if (wakeNeeded) {
                        nativeWake(mPtr);
                        concurrentWake();
                    } else {
                        mMessageCounts.incrementQueued();
                    }
@@ -2956,7 +3022,7 @@ public final class MessageQueue {
                        p.mMessage.recycleUnchecked();
                        decAndTraceMessageCount();
                        if (mMessageCounts.incrementCancelled()) {
                            nativeWake(mPtr);
                            concurrentWake();
                        }
                    }
                } else {
+94 −41
Original line number Diff line number Diff line
@@ -242,39 +242,88 @@ public final class MessageQueue {
    }

    // Use MSB to indicate quitting state. Lower 63 bits hold ref count.
    private static final long QUITTING_MASK = ~(-1L >>> 1);
    private static final long QUITTING_MASK = 1L << 63;

    private boolean incrementQuittingState() {
        long oldVal = (long)sQuittingRefCount.getAndAdd(this, 1);
    /**
     * Increment the mPtr ref count.
     *
     * If this method returns true then the caller may use mPtr until they call
     * {@link #decrementMptrRefs()}.
     * If this method returns false then the caller must not use mPtr, and must
     * instead assume that the MessageQueue is quitting or has already quit and
     * act accordingly.
     */
    private boolean incrementMptrRefs() {
        while (true) {
            final long oldVal = mQuittingRefCountValue;
            if ((oldVal & QUITTING_MASK) != 0) {
            // If we're quitting we need to drop our ref and indicate to the caller
            sQuittingRefCount.getAndAdd(this, -1);
                // If we're quitting then we're not allowed to increment the ref count.
                return false;
            }
            if (sQuittingRefCount.compareAndSet(this, oldVal, oldVal + 1)) {
                // Successfully incremented the ref count without quitting.
                return true;
            }
        }
    }

    private void decrementQuittingState() {
    /**
     * Decrement the mPtr ref count.
     *
     * Call after {@link #incrementMptrRefs()} to release the ref on mPtr.
     */
    private void decrementMptrRefs() {
        long oldVal = (long)sQuittingRefCount.getAndAdd(this, -1);
        // If quitting and we were the last ref, wake up looper thread
        if ((oldVal & QUITTING_MASK) != 0 && (oldVal & ~QUITTING_MASK) == 1L) {
        if (oldVal - 1 == QUITTING_MASK) {
            LockSupport.unpark(mLooperThread);
        }
    }

    private boolean setQuitting() {
        long oldVal = (long)sQuittingRefCount.getAndBitwiseOr(this, QUITTING_MASK);
    private boolean incrementMptrRefsAndSetQuitting() {
        while (true) {
            final long oldVal = mQuittingRefCountValue;
            if ((oldVal & QUITTING_MASK) != 0) {
                // If we're quitting then we're not allowed to increment the ref count.
                return false;
            }
            if (sQuittingRefCount.compareAndSet(this, oldVal, (oldVal + 1) | QUITTING_MASK)) {
                // Successfully incremented the ref count and set quitting.
                return true;
            }
        }
    }

    /**
     * Wake the looper thread.
     *
     * {@link #nativeWake(long)} may be called directly only by the looper thread.
     * Otherwise, call this method to ensure safe access to mPtr.
     */
    private void concurrentWake() {
        if (incrementMptrRefs()) {
            try {
                nativeWake(mPtr);
            } finally {
                decrementMptrRefs();
            }
        }
    }

    private void setFileDescriptorEvents(int fdNum, int events) {
        if (incrementMptrRefs()) {
            try {
                nativeSetFileDescriptorEvents(mPtr, fdNum, events);
            } finally {
                decrementMptrRefs();
            }
        }
    }

    private boolean getQuitting() {
        return (mQuittingRefCountValue & QUITTING_MASK) != 0;
    }

    private volatile Thread mLooperThread = null;
    // Must only be called from looper thread
    private boolean checkQuittingAndWaitForRefsToDrop() {
        if (!getQuitting()) {
@@ -379,6 +428,7 @@ public final class MessageQueue {
    }

    private final MessageCounts mMessageCounts = new MessageCounts();
    private final Thread mLooperThread;

    private final Object mIdleHandlersLock = new Object();
    @GuardedBy("mIdleHandlersLock")
@@ -421,6 +471,7 @@ public final class MessageQueue {
    MessageQueue(boolean quitAllowed) {
        mQuitAllowed = quitAllowed;
        mPtr = nativeInit();
        mLooperThread = Thread.currentThread();
    }

    @android.ravenwood.annotation.RavenwoodReplace
@@ -565,15 +616,15 @@ public final class MessageQueue {
     */
    public boolean isPolling() {
        // If the loop is quitting then it must not be idling.
        if (!incrementQuittingState()) {
            return false;
        }
        if (incrementMptrRefs()) {
            try {
                return nativeIsPolling(mPtr);
            } finally {
            decrementQuittingState();
                decrementMptrRefs();
            }
        }
        return false;
    }

    /* Helper to choose the correct queue to insert into. */
    private void insertIntoPriorityQueue(MessageNode msgNode) {
@@ -778,7 +829,11 @@ public final class MessageQueue {
             */
            StateNode nextOp = sStackStateActive;
            if (found == null) {
                if (next == null) {
                if (getQuitting()) {
                    mNextPollTimeoutMillis = 0;
                    // State change will be Active->Active, so can immediately return here.
                    return null;
                } else if (next == null) {
                    /* No message to deliver, sleep indefinitely */
                    mNextPollTimeoutMillis = -1;
                    nextOp = sStackStateParked;
@@ -911,11 +966,10 @@ public final class MessageQueue {
            throw new IllegalStateException("Main thread not allowed to quit.");
        }

        if (!incrementQuittingState()) {
        if (!incrementMptrRefsAndSetQuitting()) {
            return;
        }
        try {
            if (setQuitting()) {
            if (safe) {
                removeAllFutureMessages();
            } else {
@@ -924,9 +978,8 @@ public final class MessageQueue {

            // We can assume mPtr != 0 while we hold a ref on our quitting state
            nativeWake(mPtr);
            }
        } finally {
            decrementQuittingState();
            decrementMptrRefs();
        }
    }

@@ -1027,7 +1080,7 @@ public final class MessageQueue {
            if (sState.compareAndSet(this, old, node)) {
                if (inactive) {
                    if (wakeNeeded) {
                        nativeWake(mPtr);
                        concurrentWake();
                    } else {
                        mMessageCounts.incrementQueued();
                    }
@@ -1128,7 +1181,7 @@ public final class MessageQueue {
            Message m = first.mMessage;
            if (m.target == null && m.arg1 == token) {
                /* Wake up next() in case it was sleeping on this barrier. */
                nativeWake(mPtr);
                concurrentWake();
            }
        } else if (!removed) {
            throw new IllegalStateException("The specified message queue synchronization "
@@ -1251,7 +1304,7 @@ public final class MessageQueue {
                    if (p.removeFromStack()) {
                        p.mMessage.recycleUnchecked();
                        if (mMessageCounts.incrementCancelled()) {
                            nativeWake(mPtr);
                            concurrentWake();
                        }
                    }
                } else {
@@ -1691,11 +1744,11 @@ public final class MessageQueue {
                record.mEvents = events;
                record.mSeq += 1;
            }
            nativeSetFileDescriptorEvents(mPtr, fdNum, events);
            setFileDescriptorEvents(fdNum, events);
        } else if (record != null) {
            record.mEvents = 0;
            mFileDescriptorRecords.removeAt(index);
            nativeSetFileDescriptorEvents(mPtr, fdNum, 0);
            setFileDescriptorEvents(fdNum, 0);
        }
    }