Avoid ImageWallpaper IllegalStateException
It's possible that WallpaperService calls surface.release() between
lockCanvas and unlockCanvasAndPost, which gives the exception from
b/337287154 (IllegalStateException: surface has already been released).
To avoid that, we use a second lock that synchronizes the drawing and
the onSurfaceDestroyed method. This makes sure that we are done drawing
(or haven't started drawing) when we exit onSurfaceDestroyed().
Flag: aconfig com.android.systemui.fix_image_wallpaper_crash_surface_already_released DEVELOPMENT
Bug: 337287154
Test: treehugger
Test: crash cluster will be monitored
Change-Id: I3579e928fa9a115ecad7fa1e9fc4f15a0dc5ea68
diff --git a/packages/SystemUI/aconfig/systemui.aconfig b/packages/SystemUI/aconfig/systemui.aconfig
index 582b6a1..7a02b4f 100644
--- a/packages/SystemUI/aconfig/systemui.aconfig
+++ b/packages/SystemUI/aconfig/systemui.aconfig
@@ -398,6 +398,16 @@
}
flag {
+ name: "fix_image_wallpaper_crash_surface_already_released"
+ namespace: "systemui"
+ description: "Make sure ImageWallpaper doesn't return from OnSurfaceDestroyed until any drawing is finished"
+ bug: "337287154"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
+
+flag {
name: "activity_transition_use_largest_window"
namespace: "systemui"
description: "Target largest opening window during activity transitions."
diff --git a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java
index 1568e8c0..2e29bbd 100644
--- a/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java
+++ b/packages/SystemUI/src/com/android/systemui/wallpapers/ImageWallpaper.java
@@ -20,6 +20,7 @@
import static android.app.WallpaperManager.FLAG_SYSTEM;
import static android.app.WallpaperManager.SetWallpaperFlags;
+import static com.android.systemui.Flags.fixImageWallpaperCrashSurfaceAlreadyReleased;
import static com.android.window.flags.Flags.offloadColorExtraction;
import android.annotation.Nullable;
@@ -128,8 +129,17 @@
* and if the count is 0, unload the bitmap
*/
private int mBitmapUsages = 0;
+
+ /**
+ * Main lock for long operations (loading the bitmap or processing colors).
+ */
private final Object mLock = new Object();
+ /**
+ * Lock for SurfaceHolder operations. Should only be acquired after the main lock.
+ */
+ private final Object mSurfaceLock = new Object();
+
CanvasEngine() {
super();
setFixedSizeAllowed(true);
@@ -223,6 +233,12 @@
if (DEBUG) {
Log.i(TAG, "onSurfaceDestroyed");
}
+ if (fixImageWallpaperCrashSurfaceAlreadyReleased()) {
+ synchronized (mSurfaceLock) {
+ mSurfaceHolder = null;
+ }
+ return;
+ }
mLongExecutor.execute(this::onSurfaceDestroyedSynchronized);
}
@@ -259,7 +275,7 @@
}
private void drawFrameInternal() {
- if (mSurfaceHolder == null) {
+ if (mSurfaceHolder == null && !fixImageWallpaperCrashSurfaceAlreadyReleased()) {
Log.i(TAG, "attempt to draw a frame without a valid surface");
return;
}
@@ -268,6 +284,19 @@
if (!isBitmapLoaded()) {
loadWallpaperAndDrawFrameInternal();
} else {
+ if (fixImageWallpaperCrashSurfaceAlreadyReleased()) {
+ synchronized (mSurfaceLock) {
+ if (mSurfaceHolder == null) {
+ Log.i(TAG, "Surface released before the image could be drawn");
+ return;
+ }
+ mBitmapUsages++;
+ drawFrameOnCanvas(mBitmap);
+ reportEngineShown(false);
+ unloadBitmapIfNotUsedInternal();
+ return;
+ }
+ }
mBitmapUsages++;
drawFrameOnCanvas(mBitmap);
reportEngineShown(false);
@@ -328,9 +357,14 @@
mBitmap.recycle();
}
mBitmap = null;
-
- final Surface surface = getSurfaceHolder().getSurface();
- surface.hwuiDestroy();
+ if (fixImageWallpaperCrashSurfaceAlreadyReleased()) {
+ synchronized (mSurfaceLock) {
+ if (mSurfaceHolder != null) mSurfaceHolder.getSurface().hwuiDestroy();
+ }
+ } else {
+ final Surface surface = getSurfaceHolder().getSurface();
+ surface.hwuiDestroy();
+ }
mWallpaperManager.forgetLoadedWallpaper();
Trace.endSection();
}