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

Commit 6cf0f7cd authored by wilsonshih's avatar wilsonshih
Browse files

Fix getTaskSnapshot returning a stale snapshot when loading from disk. 

In commit 3dfdc4c2, when a client asks for a low-resolution snapshot,
the system will convert a high-resolution snapshot from the cache if
one exists.
This can cause a race condition from a second calls getTaskSnapshot
requesting the high-resolution version. Since the cache now holds the
low-resolution snapshot, the system attempts to load the
high-resolution version from disk. However, because the persistence
task may not have completed yet, the version on disk can be stale,
causing the client to receive an old snapshot.
To fix this, the low-resolution snapshot is now only updated in the cache
after the persistence task is complete.
Additionally, if the corresponding StoreWriteQueueItem exists, the
pre-converted low-resolution snapshot will be put into it, which avoids
performing the conversion again after processing the item.

Flag: com.android.window.flags.respect_requested_task_snapshot_resolution
Bug: 238206323
Test: Verified that calling getTasksnapshot first for low-resolution and
then for high-resolution returns the same, up-to-date snapshot ID.

Change-Id: I045cd3c1934adf3e003a4c4014d48112eb76963d
parent dad7f3c8

core/java/android/window/TaskSnapshot.java

+7 −1
Original line number Diff line number Diff line
@@ -103,6 +103,8 @@ public class TaskSnapshot implements Parcelable { 
    public static final int REFERENCE_WRITE_TO_PARCEL = 1 << 4;

    /** This snapshot object is being used to convert resolution . */

    public static final int REFERENCE_CONVERT_RESOLUTION = 1 << 5;

    /** This snapshot object should be update to cache at some point. */

    public static final int REFERENCE_WILL_UPDATE_TO_CACHE = 1 << 6;



    @IntDef(flag = true, prefix = { "REFERENCE_" }, value = {

            REFERENCE_NONE,
@@ -111,7 +113,8 @@ public class TaskSnapshot implements Parcelable { 
            REFERENCE_PERSIST,

            REFERENCE_CONTENT_SUGGESTION,

            REFERENCE_WRITE_TO_PARCEL,

            REFERENCE_CONVERT_RESOLUTION

            REFERENCE_CONVERT_RESOLUTION,

            REFERENCE_WILL_UPDATE_TO_CACHE

    })

    @Retention(RetentionPolicy.SOURCE)

    public @interface ReferenceFlags {}
@@ -514,6 +517,9 @@ public class TaskSnapshot implements Parcelable { 
            mWriteToParcelCount++;

        }

        mInternalReferences |= usage;

        if (usage == REFERENCE_CACHE) {

            mInternalReferences &= ~REFERENCE_WILL_UPDATE_TO_CACHE;

        }

    }



    /**

services/core/java/com/android/server/wm/SnapshotController.java

+8 −14
Original line number Diff line number Diff line
@@ -312,7 +312,7 @@ class SnapshotController { 


        if (convertToLow) {

            final TaskSnapshot convertLowResSnapshot =

                    convertToLowResSnapshot(task, inCacheSnapshot, true /* updateCache */);

                    convertToLowResSnapshot(taskId, inCacheSnapshot);

            if (convertLowResSnapshot != null) {

                convertLowResSnapshot.addReference(TaskSnapshot.REFERENCE_WRITE_TO_PARCEL);

                return convertLowResSnapshot;
@@ -332,26 +332,20 @@ class SnapshotController { 
     * </p>

     *

     * @param snapshot The high resolution snapshot.

     * @param updateCache If true, update the converted low-resolution snapshot to cache.

     */

    TaskSnapshot convertToLowResSnapshot(Task task, TaskSnapshot snapshot, boolean updateCache) {

    TaskSnapshot convertToLowResSnapshot(int taskId, TaskSnapshot snapshot) {

        try {

            Trace.traceBegin(TRACE_TAG_WINDOW_MANAGER, "createLowResSnapshot");

            final TaskSnapshot lowSnapshot = mTaskSnapshotController.createLowResSnapshot(snapshot);

            if (updateCache && lowSnapshot != null) {

                synchronized (mService.mGlobalLock) {

                    if (task.isAttached()) {

                        mTaskSnapshotController.updateCacheWithLowResSnapshotIfNeeded(

                                task, lowSnapshot);

                    }

                }

                return lowSnapshot;

            final TaskSnapshot lowResSnapshot = mTaskSnapshotController

                    .createLowResSnapshot(snapshot);

            if (lowResSnapshot != null) {

                mSnapshotPersistQueue.updateKnownLowResSnapshotIfPossible(taskId, lowResSnapshot);

            }

            return lowResSnapshot;

        } finally {

            snapshot.removeReference(TaskSnapshot.REFERENCE_CONVERT_RESOLUTION);

            Trace.traceEnd(TRACE_TAG_WINDOW_MANAGER);

        }

        return null;

    }



    void notifySnapshotChanged(int taskId, TaskSnapshot snapshot) {
@@ -427,7 +421,7 @@ class SnapshotController { 
                }

                if (convertToLow) {

                    final TaskSnapshot convert = SnapshotController.this

                            .convertToLowResSnapshot(task, freshSnapshot, updateCache);

                            .convertToLowResSnapshot(taskId, freshSnapshot);

                    if (convert != null) {

                        convert.addReference(TaskSnapshot.REFERENCE_WRITE_TO_PARCEL);

                        return convert;

services/core/java/com/android/server/wm/SnapshotPersistQueue.java

+35 −0
Original line number Diff line number Diff line
@@ -344,10 +344,21 @@ class SnapshotPersistQueue { 
        return new StoreWriteQueueItem(id, userId, snapshot, provider, lowResSnapshotConsumer);

    }



    void updateKnownLowResSnapshotIfPossible(int id, TaskSnapshot lowResSnapshot) {

        synchronized (mLock) {

            for (StoreWriteQueueItem item : mStoreQueueItems) {

                if (item.mId == id && item.updateKnownLowResSnapshotIfPossible(lowResSnapshot)) {

                    break;

                }

            }

        }

    }



    class StoreWriteQueueItem extends WriteQueueItem {

        private final int mId;

        private final TaskSnapshot mSnapshot;

        private final Consumer<LowResSnapshotSupplier> mLowResSnapshotConsumer;

        private TaskSnapshot mKnownLowResSnapshot;



        StoreWriteQueueItem(int id, int userId, TaskSnapshot snapshot,

                PersistInfoProvider provider,
@@ -366,6 +377,7 @@ class SnapshotPersistQueue { 
            mStoreQueueItems.removeIf(item -> {

                if (item.equals(this) && item.mSnapshot != mSnapshot) {

                    item.mSnapshot.removeReference(TaskSnapshot.REFERENCE_PERSIST);

                    item.removeKnownLowResSnapshot();

                    return true;

                }

                return false;
@@ -379,6 +391,22 @@ class SnapshotPersistQueue { 
            mStoreQueueItems.remove(this);

        }



        boolean updateKnownLowResSnapshotIfPossible(TaskSnapshot lowResSnapshot) {

            if (mSnapshot.getId() == lowResSnapshot.getId() && mKnownLowResSnapshot == null) {

                mKnownLowResSnapshot = lowResSnapshot;

                mKnownLowResSnapshot.addReference(TaskSnapshot.REFERENCE_WILL_UPDATE_TO_CACHE);

                return true;

            }

            return false;

        }



        private void removeKnownLowResSnapshot() {

            if (mKnownLowResSnapshot != null) {

                mKnownLowResSnapshot.removeReference(

                        TaskSnapshot.REFERENCE_WILL_UPDATE_TO_CACHE);

            }

        }



        @Override

        void write() {

            if (Trace.isTagEnabled(TRACE_TAG_WINDOW_MANAGER)) {
@@ -490,6 +518,10 @@ class SnapshotPersistQueue { 
                mLowResSnapshotConsumer.accept(new LowResSnapshotSupplier() {

                    @Override

                    public TaskSnapshot getLowResSnapshot() {

                        if (mKnownLowResSnapshot != null) {

                            lowResBitmap.recycle();

                            return mKnownLowResSnapshot;

                        }

                        final TaskSnapshot result = TaskSnapshotConvertUtil

                                .convertLowResSnapshot(mSnapshot, lowResBitmap);

                        lowResBitmap.recycle();
@@ -499,10 +531,12 @@ class SnapshotPersistQueue { 
                    @Override

                    public void abort() {

                        lowResBitmap.recycle();

                        removeKnownLowResSnapshot();

                    }

                });

            } else {

                lowResBitmap.recycle();

                removeKnownLowResSnapshot();

            }



            return true;
@@ -520,6 +554,7 @@ class SnapshotPersistQueue { 
        void onRemovedFromWriteQueue() {

            mStoreQueueItems.remove(this);

            mSnapshot.removeReference(TaskSnapshot.REFERENCE_PERSIST);

            removeKnownLowResSnapshot();

        }



        @Override