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

Commit 140c3a50 authored by wilsonshih's avatar wilsonshih
Browse files

Defer close snapshot's hardware buffer after remove snapshot from cache 

The hardware buffer can be closed when either:
1. No transition is collecting/running, or
2. After a transition finish.

This provides a simple protection because we have ensure the snapshot
object is removed from cache, so either it's not used anymore, or it's
used for a collecting transition. The queued transition won't able
to get the snapshot because it was removed from cache.

Flag: com.android.window.flags.safe_release_snapshot_aggressively
Bug: 238206323
Bug: 402277988
Test: atest TaskSnapshotCacheTest TaskSnapshotLowResDisabledTest
TaskSnapshotPersisterLoaderTest ActivitySnapshotControllerTests

Change-Id: I549d9abdb2ca0d18a22cbc74cbf13fcdf3f4bb0a
parent 440faf35

core/java/android/window/TaskSnapshot.java

+19 −1
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ import com.android.window.flags.Flags; 


import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.util.function.Consumer;



/**

 * Represents a task snapshot.
@@ -76,6 +77,7 @@ public class TaskSnapshot implements Parcelable { 
    // Must be one of the named color spaces, otherwise, always use SRGB color space.

    private final ColorSpace mColorSpace;

    private int mInternalReferences;

    private Consumer<HardwareBuffer> mSafeSnapshotReleaser;



    /** Keep in cache, doesn't need reference. */

    public static final int REFERENCE_NONE = 0;
@@ -365,9 +367,25 @@ public class TaskSnapshot implements Parcelable { 
        mInternalReferences &= ~usage;

        if (Flags.releaseSnapshotAggressively() && mInternalReferences == 0 && mSnapshot != null

                && !mSnapshot.isClosed()) {

            if (mSafeSnapshotReleaser != null) {

                mSafeSnapshotReleaser.accept(mSnapshot);

            } else {

                mSnapshot.close();

            }

        }

    }



    /**

     * Register a safe release callback, instead of immediately closing the hardware buffer when

     * no more reference, to let the system server decide when to close it.

     * Only used in core.

     */

    public synchronized void setSafeRelease(Consumer<HardwareBuffer> releaser) {

        if (!Flags.safeReleaseSnapshotAggressively()) {

            return;

        }

        mSafeSnapshotReleaser = releaser;

    }



    public static final @NonNull Creator<TaskSnapshot> CREATOR = new Creator<TaskSnapshot>() {

        public TaskSnapshot createFromParcel(Parcel source) {

core/java/android/window/flags/windowing_frontend.aconfig

+11 −0
Original line number Diff line number Diff line
@@ -515,6 +515,17 @@ flag { 
    }

}



flag {

    name: "safe_release_snapshot_aggressively"

    namespace: "windowing_frontend"

    description: "Protect task snapshot memory from premature release, which can occur when a local variable holds a reference while the snapshot is removed from the cache."

    bug: "238206323"

    is_fixed_read_only: true

    metadata {

        purpose: PURPOSE_BUGFIX

    }

}



flag {

    name: "scramble_snapshot_file_name"

    namespace: "windowing_frontend"

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

+4 −0
Original line number Diff line number Diff line
@@ -131,6 +131,10 @@ abstract class AbsAppSnapshotController<TYPE extends WindowContainer, 
        mCache = cache;

    }



    void setSnapshotReleaser(Consumer<HardwareBuffer> releaser) {

        mCache.setSafeSnapshotReleaser(releaser);

    }



    void setSnapshotEnabled(boolean enabled) {

        mSnapshotEnabled = enabled;

    }

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

+1 −0
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ class ActivitySnapshotCache extends SnapshotCache<ActivityRecord> { 
    void putSnapshot(ActivityRecord ar, TaskSnapshot snapshot) {

        final int hasCode = System.identityHashCode(ar);

        snapshot.addReference(TaskSnapshot.REFERENCE_CACHE);

        snapshot.setSafeRelease(mSafeSnapshotReleaser);

        synchronized (mLock) {

            final CacheEntry entry = mRunningCache.get(hasCode);

            if (entry != null) {

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

+8 −0
Original line number Diff line number Diff line
@@ -16,12 +16,14 @@ 
package com.android.server.wm;



import android.annotation.Nullable;

import android.hardware.HardwareBuffer;

import android.util.ArrayMap;

import android.window.TaskSnapshot;



import com.android.internal.annotations.GuardedBy;



import java.io.PrintWriter;

import java.util.function.Consumer;



/**

 * Base class for an app snapshot cache
@@ -38,12 +40,18 @@ abstract class SnapshotCache<TYPE extends WindowContainer> { 
    @GuardedBy("mLock")

    protected final ArrayMap<Integer, CacheEntry> mRunningCache = new ArrayMap<>();



    protected Consumer<HardwareBuffer> mSafeSnapshotReleaser;



    SnapshotCache(String name) {

        mName = name;

    }



    abstract void putSnapshot(TYPE window, TaskSnapshot snapshot);



    void setSafeSnapshotReleaser(Consumer<HardwareBuffer> safeSnapshotReleaser) {

        mSafeSnapshotReleaser = safeSnapshotReleaser;

    }



    void clearRunningCache() {

        synchronized (mLock) {

            mRunningCache.clear();