Android 10 SurfaceView Crash: signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4

Recently, it was found that SurfaceView crashed during the test on Android 10 system. The system tests before 10 will crash, but it is normal on devices after Android 11.

Error log

Native error message

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x4
Cause: null pointer dereference
    eax 00000000  ebx f017756c  ecx 00000001  edx 00017af6
    edi 00017af6  esi edbe9140
    ebp c4c67938  esp c4c67920  eip f0169a71

backtrace:
      #00 pc 0000fa71  /system/lib/libutils.so (android::RefBase::incStrong(void const*) const+33) (BuildId: 288ba3aff5b46dbd7e74be954af88b83)
      #01 pc 00114620  /system/lib/libandroid_runtime.so (android::nativeDeferTransactionUntilSurface(_JNIEnv*, _jclass*, long long, long long, long long, long long)+96) (BuildId: 3643bee2c4fb7899d7781c565843060b)
      #02 pc 002a2325  /system/framework/x86/boot-framework.oat (art_jni_trampoline+213) (BuildId: 38176ebc9c3cce5f657a723b08d40d487952c484)
      #03 pc 0204d399  /memfd:/jit-cache (deleted) (android.view.SurfaceControl.access$2700+89)
      #04 pc 020421aa  /memfd:/jit-cache (deleted) (android.view.SurfaceControl$Transaction.deferTransactionUntilSurface+202)
      #05 pc 02036db1  /memfd:/jit-cache (deleted) (android.view.SurfaceView.applySurfaceTransforms+177)
      #06 pc 0204979f  /memfd:/jit-cache (deleted) (android.view.SurfaceView.setParentSpaceRectangle+95)
      #07 pc 020546cb  /memfd:/jit-cache (deleted) (android.view.SurfaceView.access$200+59)
      #08 pc 0204ae12  /memfd:/jit-cache (deleted) (android.view.SurfaceView$3.positionChanged+434)
      #09 pc 0203c004  /memfd:/jit-cache (deleted) (android.graphics.RenderNode$CompositePositionUpdateListener.positionChanged+148)

Error messages displayed in the application layer log

Fatal signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 0x10 in tid 6711 (hwuiTask0), pid 6669 (.crash)
Exception from repositionChild
    java.lang.NullPointerException: mNativeObject is null. Have you called release() already?
            at android.view.SurfaceControl.checkNotReleased(SurfaceControl.java:945)
            at android.view.SurfaceControl.access$800(SurfaceControl.java:77)
            at android.view.SurfaceControl$Transaction.setPosition(SurfaceControl.java:2170)
            at android.view.SurfaceView.applySurfaceTransforms(SurfaceView.java:1025)
            at android.view.SurfaceView.setParentSpaceRectangle(SurfaceView.java:1038)
            at android.view.SurfaceView.access$200(SurfaceView.java:99)
            at android.view.SurfaceView$1.positionChanged(SurfaceView.java:1081)

Find a relevant Google patch on the Internet according to the error information

commit	005c63e8c6e6e5b312dc9c4631c57fb12224df30	[log] [tgz]
author	Robert Carr <[email protected]>	Fri Sep 20 14:31:24 2019 -0700
committer	Robert Carr <[email protected]>	Fri Sep 20 14:48:40 2019 -0700
tree	939eaeca0159a44bca7b5abf44f41f5096c1fb90
parent	c48da419931a3a9272910f2631f815cecf05c9ba [diff]
SurfaceView: Destroy SurfaceControl from RenderThread

Currently there is a race where the UI thread can destroy
the SurfaceControl (free the C++ object) while the RenderThread
still has pending operations on it. To fix have the positionLost callback handle the destruction.

Bug: 139111930
Bug: 136808018
Test: go/wm-smoke
Change-Id: Id5225e1e47046d928f8298de38ff61662debe360
diff --git a/core/java/android/view/SurfaceView.java b/core/java/android/view/SurfaceView.java
index 90e69f3..262b9e5 100644
--- a/core/java/android/view/SurfaceView.java
+++ b/core/java/android/view/SurfaceView.java
@@ -136,6 +136,7 @@
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     boolean mIsCreating = false;
     private volatile boolean mRtHandlingPositionUpdates = false;
+    private volatile boolean mRtReleaseSurfaces = false;
 
     private final ViewTreeObserver.OnScrollChangedListener mScrollChangedListener =
             this::updateSurface;
@@ -658,7 +659,14 @@
     }
 
     private void releaseSurfaces() {
+        mSurfaceAlpha = 1f;
+
         synchronized (mSurfaceControlLock) {
+            if (mRtHandlingPositionUpdates) {
+                mRtReleaseSurfaces = true;
+                return;
+            }
+
             if (mSurfaceControl != null) {
                 mTmpTransaction.remove(mSurfaceControl);
                 mSurfaceControl = null;
@@ -669,7 +677,6 @@
             }
             mTmpTransaction.apply();
         }
-        mSurfaceAlpha = 1f;
     }
 
     /** @hide */
@@ -1084,7 +1091,9 @@
             // the synchronization would violate the rule that RT must never block
             // on the UI thread which would open up potential deadlocks. The risk of
             // a single-frame desync is therefore preferable for now.
-            mRtHandlingPositionUpdates = true;
+            synchronized(mSurfaceControlLock) {
+                mRtHandlingPositionUpdates = true;
+            }
             if (mRTLastReportedPosition.left == left
                     && mRTLastReportedPosition.top == top
                     && mRTLastReportedPosition.right == right
@@ -1126,6 +1135,18 @@
                         frameNumber);
             }
             mRtTransaction.hide(mSurfaceControl);
+
+            synchronized (mSurfaceControlLock) {
+                if (mRtReleaseSurfaces) {
+                    mRtReleaseSurfaces = false;
+                    mRtTransaction.remove(mSurfaceControl);
+                    mRtTransaction.remove(mBackgroundControl);
+                    mSurfaceControl = null;
+                    mBackgroundControl = null;
+                }
+                mRtHandlingPositionUpdates = false;
+            }
+
             mRtTransaction.apply();
         }
     };

After updating, the SurfaceView getViewRootImpl() will always be empty. No patches for other updates were found
Because it is normal to test on Android 11, Android 11 solves this problem, so directly copy the code of Android 11’s SurfaceView. Because Android 11 has updated a lot of things, select relevant modifications to update.

diff --git a/frameworks/base/core/java/android/view/SurfaceView.java b/frameworks/base/core/java/android/view/SurfaceView.java
index d11548d687..3d1a5a3549 100644
--- a/frameworks/base/core/java/android/view/SurfaceView.java
+++ b/frameworks/base/core/java/android/view/SurfaceView.java
@@ -118,8 +118,7 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
     boolean mDrawFinished = false;

     final Rect mScreenRect = new Rect();
-    SurfaceSession mSurfaceSession;
-
+    private final SurfaceSession mSurfaceSession = new SurfaceSession();
     SurfaceControl mSurfaceControl;
     // In the case of format changes we switch out the surface in-place
     // we need to preserve the old one until the new one has drawn.
@@ -135,6 +134,7 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
     @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.P, trackingBug = 115609023)
     boolean mIsCreating = false;
     private volatile boolean mRtHandlingPositionUpdates = false;
+    private volatile boolean mRtReleaseSurfaces = false;

     private final ViewTreeObserver.OnScrollChangedListener mScrollChangedListener =
             this::updateSurface;
@@ -428,9 +428,16 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
     }

     private void performDrawFinished() {
+        if (mDeferredDestroySurfaceControl != null) {
+            synchronized (mSurfaceControlLock) {
+                mTmpTransaction.remove(mDeferredDestroySurfaceControl).apply();
+                mDeferredDestroySurfaceControl = null;
+            }
+        }
         if (mPendingReportDraws > 0) {
             mDrawFinished = true;
             if (mAttachedToWindow) {
+                mParent.requestTransparentRegion(SurfaceView.this);
                 notifyDrawFinished();
                 invalidate();
             }
@@ -658,7 +665,15 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
     }

     private void releaseSurfaces() {
+        mSurfaceAlpha = 1f;
+
         synchronized (mSurfaceControlLock) {
+            mSurface.release();
+            if (mRtHandlingPositionUpdates) {
+                mRtReleaseSurfaces = true;
+                return;
+            }
+
             if (mSurfaceControl != null) {
                 mTmpTransaction.remove(mSurfaceControl);
                 mSurfaceControl = null;
@@ -669,7 +684,6 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
             }
             mTmpTransaction.apply();
         }
-        mSurfaceAlpha = 1f;
     }

     /** @hide */
@@ -680,12 +694,21 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
             }
             return;
         }
-        ViewRootImpl viewRoot = getViewRootImpl();
-        if (viewRoot == null || viewRoot.mSurface == null || !viewRoot.mSurface.isValid()) {
+        final ViewRootImpl viewRoot = getViewRootImpl();
+        if (viewRoot == null) {
+            if (DEBUG) {
+                Log.d(TAG, System.identityHashCode(this)
+                        + " updateSurface: viewRoot null");
+            }
+            return;
+        }
+        if (viewRoot.mSurface == null || !viewRoot.mSurface.isValid()) {
             if (DEBUG) {
                 Log.d(TAG, System.identityHashCode(this)
                         + " updateSurface: no valid surface");
             }
+            notifySurfaceDestroyed();
+            releaseSurfaces();
             return;
         }

@@ -708,9 +731,14 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
         final boolean sizeChanged = mSurfaceWidth != myWidth || mSurfaceHeight != myHeight;
         final boolean windowVisibleChanged = mWindowVisibility != mLastWindowVisibility;
         boolean redrawNeeded = false;
-
-        if (creating || formatChanged || sizeChanged || visibleChanged || (mUseAlpha
-                && alphaChanged) || windowVisibleChanged) {
+        getLocationInSurface(mLocation);
+        final boolean positionChanged = mWindowSpaceLeft != mLocation[0]
+            || mWindowSpaceTop != mLocation[1];
+        final boolean layoutSizeChanged = getWidth() != mScreenRect.width()
+            || getHeight() != mScreenRect.height();
+        if (creating || formatChanged || sizeChanged || visibleChanged ||
+                (mUseAlpha && alphaChanged) || windowVisibleChanged ||
+                positionChanged || layoutSizeChanged) {
             getLocationInWindow(mLocation);

             if (DEBUG) Log.i(TAG, System.identityHashCode(this) + " "
@@ -744,7 +772,6 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb

                 if (creating) {
                     viewRoot.createBoundsSurface(mSubLayer);
-                    mSurfaceSession = new SurfaceSession();
                     mDeferredDestroySurfaceControl = mSurfaceControl;

                     updateOpaqueFlag();
@@ -853,28 +880,7 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
                     final boolean surfaceChanged = creating;
                     if (mSurfaceCreated && (surfaceChanged || (!visible && visibleChanged))) {
                         mSurfaceCreated = false;
-                        if (mSurface.isValid()) {
-                            if (DEBUG) Log.i(TAG, System.identityHashCode(this) + " "
-                                    + "visibleChanged -- surfaceDestroyed");
-                            callbacks = getSurfaceCallbacks();
-                            for (SurfaceHolder.Callback c : callbacks) {
-                                c.surfaceDestroyed(mSurfaceHolder);
-                            }
-                            // Since Android N the same surface may be reused and given to us
-                            // again by the system server at a later point. However
-                            // as we didn't do this in previous releases, clients weren't
-                            // necessarily required to clean up properly in
-                            // surfaceDestroyed. This leads to problems for example when
-                            // clients don't destroy their EGL context, and try
-                            // and create a new one on the same surface following reuse.
-                            // Since there is no valid use of the surface in-between
-                            // surfaceDestroyed and surfaceCreated, we force a disconnect,
-                            // so the next connect will always work if we end up reusing
-                            // the surface.
-                            if (mSurface.isValid()) {
-                                mSurface.forceScopedDisconnect();
-                            }
-                        }
+                        notifySurfaceDestroyed();
                     }

                     if (creating) {
@@ -933,7 +939,6 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
                 } finally {
                     mIsCreating = false;
                     if (mSurfaceControl != null && !mSurfaceCreated) {
-                        mSurface.release();
                         releaseSurfaces();
                     }
                 }
@@ -944,48 +949,6 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
                 TAG, "Layout: x=" + mScreenRect.left + " y=" + mScreenRect.top
                 + " w=" + mScreenRect.width() + " h=" + mScreenRect.height()
                 + ", frame=" + mSurfaceFrame);
-        } else {
-            // Calculate the window position in case RT loses the window
-            // and we need to fallback to a UI-thread driven position update
-            getLocationInSurface(mLocation);
-            final boolean positionChanged = mWindowSpaceLeft != mLocation[0]
-                    || mWindowSpaceTop != mLocation[1];
-            final boolean layoutSizeChanged = getWidth() != mScreenRect.width()
-                    || getHeight() != mScreenRect.height();
-            if (positionChanged || layoutSizeChanged) { // Only the position has changed
-                mWindowSpaceLeft = mLocation[0];
-                mWindowSpaceTop = mLocation[1];
-                // For our size changed check, we keep mScreenRect.width() and mScreenRect.height()
-                // in view local space.
-                mLocation[0] = getWidth();
-                mLocation[1] = getHeight();
-
-                mScreenRect.set(mWindowSpaceLeft, mWindowSpaceTop,
-                        mWindowSpaceLeft + mLocation[0], mWindowSpaceTop + mLocation[1]);
-
-                if (translator != null) {
-                    translator.translateRectInAppWindowToScreen(mScreenRect);
-                }
-
-                if (mSurfaceControl == null) {
-                    return;
-                }
-
-                if (!isHardwareAccelerated() || !mRtHandlingPositionUpdates) {
-                    try {
-                        if (DEBUG_POSITION) {
-                            Log.d(TAG, String.format("%d updateSurfacePosition UI, "
-                                            + "position = [%d, %d, %d, %d]",
-                                    System.identityHashCode(this),
-                                    mScreenRect.left, mScreenRect.top,
-                                    mScreenRect.right, mScreenRect.bottom));
-                        }
-                        setParentSpaceRectangle(mScreenRect, -1);
-                    } catch (Exception ex) {
-                        Log.e(TAG, "Exception configuring surface", ex);
-                    }
-                }
-            }
         }
     }

@@ -994,12 +957,6 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
             Log.i(TAG, System.identityHashCode(this) + " "
                     + "finishedDrawing");
         }
-
-        if (mDeferredDestroySurfaceControl != null) {
-            mTmpTransaction.remove(mDeferredDestroySurfaceControl).apply();
-            mDeferredDestroySurfaceControl = null;
-        }
-
         runOnUiThread(this::performDrawFinished);
     }

@@ -1015,9 +972,8 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
     }

     private void applySurfaceTransforms(SurfaceControl surface, Rect position, long frameNumber) {
-        if (frameNumber > 0) {
-            final ViewRootImpl viewRoot = getViewRootImpl();
-
+        final ViewRootImpl viewRoot = getViewRootImpl();
+        if (frameNumber > 0 && viewRoot != null ) {
             mRtTransaction.deferTransactionUntilSurface(surface, viewRoot.mSurface,
                     frameNumber);
         }
@@ -1036,10 +992,10 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
         final ViewRootImpl viewRoot = getViewRootImpl();

         applySurfaceTransforms(mSurfaceControl, position, frameNumber);
-
-        applyChildSurfaceTransaction_renderWorker(mRtTransaction, viewRoot.mSurface,
+        if (frameNumber > 0 && viewRoot != null ) {
+            applyChildSurfaceTransaction_renderWorker(mRtTransaction, viewRoot.mSurface,
                 frameNumber);
-
+        }
         mRtTransaction.apply();
     }

@@ -1062,7 +1018,9 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
             // the synchronization would violate the rule that RT must never block
             // on the UI thread which would open up potential deadlocks. The risk of
             // a single-frame desync is therefore preferable for now.
-            mRtHandlingPositionUpdates = true;
+            synchronized(mSurfaceControlLock) {
+                mRtHandlingPositionUpdates = true;
+            }
             if (mRTLastReportedPosition.left == left
                     && mRTLastReportedPosition.top == top
                     && mRTLastReportedPosition.right == right
@@ -1096,14 +1054,27 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
             if (mSurfaceControl == null) {
                 return;
             }
+            final ViewRootImpl viewRoot = getViewRootImpl();

-            if (frameNumber > 0) {
-                final ViewRootImpl viewRoot = getViewRootImpl();

-                mRtTransaction.deferTransactionUntilSurface(mSurfaceControl, viewRoot.mSurface,
-                        frameNumber);
+            synchronized (mSurfaceControlLock) {
+                if (frameNumber > 0 && viewRoot != null) {
+                    if (viewRoot.mSurface.isValid()) {
+                        mRtTransaction.deferTransactionUntilSurface(mSurfaceControl, viewRoot.mSurface,
+                                frameNumber);
+                                       }
+                }
+                mRtTransaction.hide(mSurfaceControl);
+                if (mRtReleaseSurfaces) {
+                    mRtReleaseSurfaces = false;
+                    mRtTransaction.remove(mSurfaceControl);
+                    mRtTransaction.remove(mBackgroundControl);
+                    mSurfaceControl = null;
+                    mBackgroundControl = null;
+                }
+                mRtHandlingPositionUpdates = false;
             }
-            mRtTransaction.hide(mSurfaceControl);
+
             mRtTransaction.apply();
         }
     };
@@ -1348,4 +1319,29 @@ public class SurfaceView extends View implements ViewRootImpl.WindowStoppedCallb
     public SurfaceControl getSurfaceControl() {
         return mSurfaceControl;
     }
+
+    private void notifySurfaceDestroyed() {
+        if (mSurface.isValid()) {
+            if (DEBUG) Log.i(TAG, System.identityHashCode(this) + " "
+                    + "surfaceDestroyed");
+            SurfaceHolder.Callback[] callbacks = getSurfaceCallbacks();
+            for (SurfaceHolder.Callback c : callbacks) {
+                c.surfaceDestroyed(mSurfaceHolder);
+            }
+            // Since Android N the same surface may be reused and given to us
+            // again by the system server at a later point. However
+            // as we didn't do this in previous releases, clients weren't
+            // necessarily required to clean up properly in
+            // surfaceDestroyed. This leads to problems for example when
+            // clients don't destroy their EGL context, and try
+            // and create a new one on the same surface following reuse.
+            // Since there is no valid use of the surface in-between
+            // surfaceDestroyed and surfaceCreated, we force a disconnect,
+            // so the next connect will always work if we end up reusing
+            // the surface.
+            if (mSurface.isValid()) {
+                mSurface.forceScopedDisconnect();
+            }
+        }
+    }
 }

After modification, the test is normal, and the application will not crash, but an error log message will pop up from time to time
E/Layer: [SurfaceView – com. crash/com. crash. MainActivity # 3] No local sync point found

Check Layer.cpp when encountering this exception is to skip the logic, so it does not care, it is not clear if there are any other problems with this change.

 741 bool Layer::applyPendingStates(State* stateToCommit) {
 742     bool stateUpdateAvailable = false;
 743     while (!mPendingStates.empty()) {
 744         if (mPendingStates[0].barrierLayer_legacy != nullptr) {
 745             if (mRemoteSyncPoints.empty()) {
 746                 // If we don't have a sync point for this, apply it anyway. It
 747                 // will be visually wrong, but it should keep us from getting
 748                 // into too much trouble.
 749                 ALOGE("[%s] No local sync point found", mName.string());
 750                 popPendingState(stateToCommit);
 751                 stateUpdateAvailable = true;
 752                 continue;
 753             }
 754
 755             if (mRemoteSyncPoints.front()->getFrameNumber() !=
 756                 mPendingStates[0].frameNumber_legacy) {
 757                 ALOGE("[%s] Unexpected sync point frame number found", mName.string());
 758
 759                 // Signal our end of the sync point and then dispose of it
 760                 mRemoteSyncPoints.front()->setTransactionApplied();
 761                 mRemoteSyncPoints.pop_front();
 762                 continue;
 763             }
 764
 765             if (mRemoteSyncPoints.front()->frameIsAvailable()) {
 766                 ATRACE_NAME("frameIsAvailable");
 767                 // Apply the state update
 768                 popPendingState(stateToCommit);
 769                 stateUpdateAvailable = true;
 770
 771                 // Signal our end of the sync point and then dispose of it
 772                 mRemoteSyncPoints.front()->setTransactionApplied();
 773                 mRemoteSyncPoints.pop_front();
 774             } else {
 775                 ATRACE_NAME("!frameIsAvailable");
 776                 break;
 777             }
 778         } else {
 779             popPendingState(stateToCommit);
 780             stateUpdateAvailable = true;
 781         }
 782     }
 783
 784     // If we still have pending updates, wake SurfaceFlinger back up and point
 785     // it at this layer so we can process them
 786     if (!mPendingStates.empty()) {
 787         setTransactionFlags(eTransactionNeeded);
 788         mFlinger->setTransactionFlags(eTraversalNeeded);
 789     }
 790
 791     mCurrentState.modified = false;
 792     return stateUpdateAvailable;
 793 }

Read More: