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

Commit 8c68a4d9 authored by John Wu's avatar John Wu
Browse files

Do not do heapSweep on message removal 

When push or quit is called between a heapSweap call and a removeMessage
call, the existing implementation is broken because it will call
heapSweap, making the removed message being re-added back to
MessageHeap, causing duplicates.

Also, stop comparing m against mTopValue, and instead directly check if
m.prev is null to determine whether an add/remove race occurred, since
the existing implementation does not take into account when pushes or
quits are called after the previous heapSweap.

Bug: 433901686
Test: atest FrameworksCoreTestsRavenwood:android.os.MessageStackTest
Flag: EXEMPT new data structure isn't used yet; usages will be flagged
Change-Id: Icd315ec277d72ada38b671757e6cce3886ea3ce9
parent abdefbe6

core/java/android/os/MessageStack.java

+19 −5
Original line number Diff line number Diff line
@@ -251,6 +251,17 @@ public final class MessageStack { 
        return m;

    }



    /**

     * Create all missing backlinks in the stack.

     */

    private void createBackLinks() {

        Message current = (Message) sTop.getAcquire(this);

        while (current != null && current.next != null && current.next.prev == null) {

            current.next.prev = current;

            current = current.next;

        }

    }



    /**

     * Remove a message from the stack.

     *
@@ -275,8 +286,10 @@ public final class MessageStack { 
                mLooperProcessed = mLooperProcessed.next;

            } while (mLooperProcessed != null && mLooperProcessed.isRemoved());

        }

        // If this is the top, attempt to CAS the top to the next item.

        if (m == mTopValue) {



        // If prev is null, m was the top at the time the previous heapSweep was called.

        if (m.prev == null) {

            // Check whether m is still the top. If so, we can just unlink it.

            // Since only the looper thread can pop or drain the freelist, if this CAS fails, it

            // can only be due to a push or quit.

            if (sTop.compareAndSet(this, m, m.next)) {
@@ -284,9 +297,10 @@ public final class MessageStack { 
                m.prev = null;

                return;

            }

            // If the CAS failed, this is no longer the top, and we must find m's predecessor and

            // create backlinks before continuing to remove the message the normal way.

            heapSweep();

            // New messages are pushed to the stack between the previous heapSweep and now.

            // We must find m's predecessor and create backlinks before continuing to

            // remove the message the normal way.

            createBackLinks();

        }

        unlinkFromNext(m);

        unlinkFromPrev(m);