Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

SurfaceView: Ensure the Surface remains valid with SurfaceHolder#lockCanvas

Browse Source 
Fixes an issue where the surface was destroyed while the caller had a
valid Surface. The SurfaceHolder#lockCanvas API states the
Surface remains valid until the caller unlocks the canvas. This cl
ensures the validity by acquiring the mSurfaceLock before changing
the state of the Surface.

Test: app in b/234006724 does not ANR
Test: app in b/235188096 does not crash
Test: atest SurfaceViewTest
Bug: 235188096
Change-Id: I67ae2b98b83cc148128bb7c70cf2180c927c0b04
This commit is contained in:
Vishnu Nair
2022-07-19 08:14:11 -07:00
parent 838cd583ba
commit 38876847fe
1 changed files with 31 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
core/java/android/view/SurfaceView.java
View File

@@ -119,6 +119,10 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
final int[] mLocation = new int[2];
@UnsupportedAppUsage
// Used to ensure the Surface remains valid between SurfaceHolder#lockCanvas and
// SurfaceHolder#unlockCanvasAndPost calls. This prevents SurfaceView from destroying or
// invalidating the Surface. This means this lock should be acquired when destroying the
// BlastBufferQueue.
final ReentrantLock mSurfaceLock = new ReentrantLock();
@UnsupportedAppUsage
final Surface mSurface = new Surface(); // Current surface in use
@@ -724,14 +728,18 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
private void releaseSurfaces(boolean releaseSurfacePackage) {
mSurfaceAlpha = 1f;
mSurface.destroy();
synchronized (mSurfaceControlLock) {
try {
mSurfaceLock.lock();
mSurface.destroy();
if (mBlastBufferQueue != null) {
mBlastBufferQueue.destroy();
mBlastBufferQueue = null;
}
} finally {
mSurfaceLock.unlock();
}
synchronized (mSurfaceControlLock) {
final Transaction transaction = new Transaction();
if (mSurfaceControl != null) {
transaction.remove(mSurfaceControl);
@@ -1232,16 +1240,21 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
.build();
}
// Always recreate the IGBP for compatibility. This can be optimized in the future but
// the behavior change will need to be gated by SDK version.
if (mBlastBufferQueue != null) {
mBlastBufferQueue.destroy();
}
mTransformHint = viewRoot.getBufferTransformHint();
mBlastSurfaceControl.setTransformHint(mTransformHint);
mBlastBufferQueue = new BLASTBufferQueue(name, false /* updateDestinationFrame */);
mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, mFormat);
// Always recreate the IGBP for compatibility. This can be optimized in the future but
// the behavior change will need to be gated by SDK version.
try {
mSurfaceLock.lock();
if (mBlastBufferQueue != null) {
mBlastBufferQueue.destroy();
}
mBlastBufferQueue = new BLASTBufferQueue(name, false /* updateDestinationFrame */);
mBlastBufferQueue.update(mBlastSurfaceControl, mSurfaceWidth, mSurfaceHeight, mFormat);
} finally {
mSurfaceLock.unlock();
}
mBlastBufferQueue.setTransactionHangCallback(ViewRootImpl.sTransactionHangCallback);
}
@@ -1814,7 +1827,14 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
// so the next connect will always work if we end up reusing
// the surface.
if (mSurface.isValid()) {
mSurface.forceScopedDisconnect();
// We need to grab this lock since mSurface.forceScopedDisconnect
// will free buffers from the queue.
try {
mSurfaceLock.lock();
mSurface.forceScopedDisconnect();
} finally {
mSurfaceLock.unlock();
}
}
}
}