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

Merge "SurfaceView: Fix unsafe transaction accesses" into sc-v2-dev

Browse Source
This commit is contained in:
Vishnu Nair
2022-01-10 23:43:41 +00:00
committed by Android (Google) Code Review
parent a70be49294 c10beab1e2
commit 830bf8bd34
1 changed files with 49 additions and 56 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -138,20 +138,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
private boolean mDisableBackgroundLayer = false; private boolean mDisableBackgroundLayer = false;
/** /**
* We use this lock in SOME cases when reading or writing SurfaceControl, * We use this lock to protect access to mSurfaceControl and
* but use the following model so that the RenderThread can run locklessly * SurfaceViewPositionUpdateListener#mPositionChangedTransaction. Both are accessed on the UI
* in the position up-date case. * thread and the render thread.
*
* 1. UI Thread can read from mSurfaceControl (use in Transactions) without
* holding the lock.
* 2. UI Thread will hold the lock when writing to mSurfaceControl (calling release
* or remove).
* 3. Render thread will also hold the lock when writing to mSurfaceControl (e.g.
* calling release from positionLost).
* 3. RenderNode.PositionUpdateListener::positionChanged will only be called
* when the UI thread is paused (blocked on the Render thread).
* 4. positionChanged thus will not be required to hold the lock as the
* UI thread is blocked, and the other writer is the RT itself.
*/ */
final Object mSurfaceControlLock = new Object(); final Object mSurfaceControlLock = new Object();
final Rect mTmpRect = new Rect(); final Rect mTmpRect = new Rect();
@@ -945,7 +934,9 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
Transaction geometryTransaction) { Transaction geometryTransaction) {
if (mPositionListener != null) { if (mPositionListener != null) {
mRenderNode.removePositionUpdateListener(mPositionListener); mRenderNode.removePositionUpdateListener(mPositionListener);
geometryTransaction = mPositionListener.getTransaction().merge(geometryTransaction); synchronized (mSurfaceControlLock) {
geometryTransaction = mPositionListener.getTransaction().merge(geometryTransaction);
}
} }
mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight, mPositionListener = new SurfaceViewPositionUpdateListener(surfaceWidth, surfaceHeight,
geometryTransaction); geometryTransaction);
@@ -1467,43 +1458,45 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
if (mSurfaceControl == null) { if (mSurfaceControl == null) {
return; return;
} }
} if (mRTLastReportedPosition.left == left
if (mRTLastReportedPosition.left == left && mRTLastReportedPosition.top == top
&& mRTLastReportedPosition.top == top && mRTLastReportedPosition.right == right
&& mRTLastReportedPosition.right == right && mRTLastReportedPosition.bottom == bottom
&& mRTLastReportedPosition.bottom == bottom && mRTLastReportedSurfaceSize.x == mRtSurfaceWidth
&& mRTLastReportedSurfaceSize.x == mRtSurfaceWidth && mRTLastReportedSurfaceSize.y == mRtSurfaceHeight
&& mRTLastReportedSurfaceSize.y == mRtSurfaceHeight && !mPendingTransaction) {
&& !mPendingTransaction) { return;
return;
}
try {
if (DEBUG_POSITION) {
Log.d(TAG, String.format(
"%d updateSurfacePosition RenderWorker, frameNr = %d, "
+ "position = [%d, %d, %d, %d] surfaceSize = %dx%d",
System.identityHashCode(SurfaceView.this), frameNumber,
left, top, right, bottom, mRtSurfaceWidth, mRtSurfaceHeight));
} }
mRTLastReportedPosition.set(left, top, right, bottom); try {
mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight); if (DEBUG_POSITION) {
onSetSurfacePositionAndScaleRT(mPositionChangedTransaction, mSurfaceControl, Log.d(TAG, String.format(
mRTLastReportedPosition.left /*positionLeft*/, "%d updateSurfacePosition RenderWorker, frameNr = %d, "
mRTLastReportedPosition.top /*positionTop*/, + "position = [%d, %d, %d, %d] surfaceSize = %dx%d",
mRTLastReportedPosition.width() / (float) mRtSurfaceWidth /*postScaleX*/, System.identityHashCode(SurfaceView.this), frameNumber,
mRTLastReportedPosition.height() / (float) mRtSurfaceHeight /*postScaleY*/); left, top, right, bottom, mRtSurfaceWidth, mRtSurfaceHeight));
if (mViewVisibility) { }
mPositionChangedTransaction.show(mSurfaceControl); mRTLastReportedPosition.set(left, top, right, bottom);
mRTLastReportedSurfaceSize.set(mRtSurfaceWidth, mRtSurfaceHeight);
onSetSurfacePositionAndScaleRT(mPositionChangedTransaction, mSurfaceControl,
mRTLastReportedPosition.left /*positionLeft*/,
mRTLastReportedPosition.top /*positionTop*/,
mRTLastReportedPosition.width()
/ (float) mRtSurfaceWidth /*postScaleX*/,
mRTLastReportedPosition.height()
/ (float) mRtSurfaceHeight /*postScaleY*/);
if (mViewVisibility) {
mPositionChangedTransaction.show(mSurfaceControl);
}
final ViewRootImpl viewRoot = getViewRootImpl();
if (viewRoot != null) {
applyChildSurfaceTransaction_renderWorker(mPositionChangedTransaction,
viewRoot.mSurface, frameNumber);
}
applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);
mPendingTransaction = false;
} catch (Exception ex) {
Log.e(TAG, "Exception from repositionChild", ex);
} }
final ViewRootImpl viewRoot = getViewRootImpl();
if (viewRoot != null) {
applyChildSurfaceTransaction_renderWorker(mPositionChangedTransaction,
viewRoot.mSurface, frameNumber);
}
applyOrMergeTransaction(mPositionChangedTransaction, frameNumber);
mPendingTransaction = false;
} catch (Exception ex) {
Log.e(TAG, "Exception from repositionChild", ex);
} }
} }
@@ -1526,18 +1519,18 @@ public class SurfaceView extends View implements ViewRootImpl.SurfaceChangedCall
} }
mRTLastReportedPosition.setEmpty(); mRTLastReportedPosition.setEmpty();
mRTLastReportedSurfaceSize.set(-1, -1); mRTLastReportedSurfaceSize.set(-1, -1);
if (mPendingTransaction) {
Log.w(TAG, System.identityHashCode(SurfaceView.this)
+ "Pending transaction cleared.");
mPositionChangedTransaction.clear();
mPendingTransaction = false;
}
/** /**
* positionLost can be called while UI thread is un-paused so we * positionLost can be called while UI thread is un-paused so we
* need to hold the lock here. * need to hold the lock here.
*/ */
synchronized (mSurfaceControlLock) { synchronized (mSurfaceControlLock) {
if (mPendingTransaction) {
Log.w(TAG, System.identityHashCode(SurfaceView.this)
+ "Pending transaction cleared.");
mPositionChangedTransaction.clear();
mPendingTransaction = false;
}
if (mSurfaceControl == null) { if (mSurfaceControl == null) {
return; return;
} }