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

Commit 722ff89f authored by Wale Ogunwale's avatar Wale Ogunwale
Browse files

Protect against surfaceController and hasSurface getting out of sync. 

WindowStateAnimator.mSurfaceController is set to null whenever a
surface is destroyed and WindowState.mHasSurface is set to false
shortly after that. However, it is possible for them to get out
of sync in a couple of places due to exceptions or duplicate destroy.
Consolidated the call to set WindowState.mHasSurface inside a finally
block in WindowStateAnimator.destroySurface
Also, cleaned up the code a little to that it is more obvious what is
going on.

Bug: 27235356
Change-Id: I7e6d0c1fb015531c393ac86dcaebebd134fad612
parent 40d8823d

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

+25 −28
Original line number Diff line number Diff line
@@ -8984,11 +8984,10 @@ public class WindowManagerService extends IWindowManager.Stub 
        EventLog.writeEvent(EventLogTags.WM_NO_SURFACE_MEMORY, winAnimator.mWin.toString(),

                winAnimator.mSession.mPid, operation);



        long callingIdentity = Binder.clearCallingIdentity();

        final long callingIdentity = Binder.clearCallingIdentity();

        try {

            // There was some problem...   first, do a sanity check of the

            // window list to make sure we haven't left any dangling surfaces

            // around.

            // There was some problem...   first, do a sanity check of the window list to make sure

            // we haven't left any dangling surfaces around.



            Slog.i(TAG_WM, "Out of memory for surface!  Looking for leaks...");

            final int numDisplays = mDisplayContents.size();
@@ -8997,8 +8996,10 @@ public class WindowManagerService extends IWindowManager.Stub 
                final int numWindows = windows.size();

                for (int winNdx = 0; winNdx < numWindows; ++winNdx) {

                    final WindowState ws = windows.get(winNdx);

                    WindowStateAnimator wsa = ws.mWinAnimator;

                    if (wsa.mSurfaceController != null) {

                    final WindowStateAnimator wsa = ws.mWinAnimator;

                    if (wsa.mSurfaceController == null) {

                        continue;

                    }

                    if (!mSessions.contains(wsa.mSession)) {

                        Slog.w(TAG_WM, "LEAKED SURFACE (session doesn't exist): "

                                + ws + " surface=" + wsa.mSurfaceController
@@ -9006,7 +9007,6 @@ public class WindowManagerService extends IWindowManager.Stub 
                                + " pid=" + ws.mSession.mPid

                                + " uid=" + ws.mSession.mUid);

                        wsa.destroySurface();

                            ws.setHasSurface(false);

                        mForceRemoves.add(ws);

                        leakedSurface = true;

                    } else if (ws.mAppToken != null && ws.mAppToken.clientHidden) {
@@ -9016,12 +9016,10 @@ public class WindowManagerService extends IWindowManager.Stub 
                                + " saved=" + ws.mAppToken.hasSavedSurface());

                        if (SHOW_TRANSACTIONS) logSurface(ws, "LEAK DESTROY", false);

                        wsa.destroySurface();

                            ws.setHasSurface(false);

                        leakedSurface = true;

                    }

                }

            }

            }



            if (!leakedSurface) {

                Slog.w(TAG_WM, "No leaked surfaces; killing applicatons!");
@@ -9061,8 +9059,7 @@ public class WindowManagerService extends IWindowManager.Stub 
                if (surfaceController != null) {

                    if (SHOW_TRANSACTIONS || SHOW_SURFACE_ALLOC) logSurface(winAnimator.mWin,

                            "RECOVER DESTROY", false);

                    surfaceController.destroyInTransaction();

                    winAnimator.mWin.setHasSurface(false);

                    winAnimator.destroySurface();

                    scheduleRemoveStartingWindowLocked(winAnimator.mWin.mAppToken);

                }

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

+153 −144
Original line number Diff line number Diff line
@@ -600,9 +600,15 @@ class WindowStateAnimator { 
            return mSurfaceController;

        }



        if (mSurfaceController == null) {

        if (mSurfaceController != null) {

            return mSurfaceController;

        }



        w.setHasSurface(false);



        if (DEBUG_ANIM || DEBUG_ORIENTATION) Slog.i(TAG,

                "createSurface " + this + ": mDrawState=DRAW_PENDING");



        mDrawState = DRAW_PENDING;

        if (w.mAppToken != null) {

            if (w.mAppToken.mAppAnimator.animation == null) {
@@ -677,23 +683,19 @@ class WindowStateAnimator { 
                        + " / " + this);

            }

        } catch (OutOfResourcesException e) {

                w.setHasSurface(false);

            Slog.w(TAG, "OutOfResourcesException creating surface");

            mService.reclaimSomeSurfaceMemoryLocked(this, "create", true);

            mDrawState = NO_SURFACE;

            return null;

        } catch (Exception e) {

                w.setHasSurface(false);

            Slog.e(TAG, "Exception creating surface", e);

            mDrawState = NO_SURFACE;

            return null;

        }



            if (WindowManagerService.localLOGV) {

                Slog.v(TAG, "Got surface: " + mSurfaceController

        if (WindowManagerService.localLOGV) Slog.v(TAG, "Got surface: " + mSurfaceController

                + ", set left=" + w.mFrame.left + " top=" + w.mFrame.top

                + ", animLayer=" + mAnimLayer);

            }



        if (SHOW_LIGHT_TRANSACTIONS) {

            Slog.i(TAG, ">>> OPEN TRANSACTION createSurfaceLocked");
@@ -706,13 +708,10 @@ class WindowStateAnimator { 
        final int layerStack = w.getDisplayContent().getDisplay().getLayerStack();

        if (SHOW_TRANSACTIONS) WindowManagerService.logSurface(w,

                "POS " + mTmpSize.left + ", " + mTmpSize.top, false);

            mSurfaceController.setPositionAndLayer(mTmpSize.left, mTmpSize.top, layerStack,

                    mAnimLayer);

        mSurfaceController.setPositionAndLayer(mTmpSize.left, mTmpSize.top, layerStack, mAnimLayer);

        mLastHidden = true;



            if (WindowManagerService.localLOGV) Slog.v(

                    TAG, "Created surface " + this);

        }

        if (WindowManagerService.localLOGV) Slog.v(TAG, "Created surface " + this);

        return mSurfaceController;

    }
@@ -783,13 +782,15 @@ class WindowStateAnimator { 


        mWin.mSurfaceSaved = false;



        if (mSurfaceController != null) {

        if (mSurfaceController == null) {

            return;

        }



        int i = mWin.mChildWindows.size();

            // When destroying a surface we want to make sure child windows

            // are hidden. If we are preserving the surface until redraw though

            // we intend to swap it out with another surface for resizing. In this case

            // the window always remains visible to the user and the child windows

            // should likewise remain visable.

        // When destroying a surface we want to make sure child windows are hidden. If we are

        // preserving the surface until redraw though we intend to swap it out with another surface

        // for resizing. In this case the window always remains visible to the user and the child

        // windows should likewise remain visible.

        while (!mDestroyPreservedSurfaceUponRedraw && i > 0) {

            i--;

            WindowState c = mWin.mChildWindows.get(i);
@@ -822,8 +823,7 @@ class WindowStateAnimator { 
            }

        } catch (RuntimeException e) {

            Slog.w(TAG, "Exception thrown when destroying Window " + this

                    + " surface " + mSurfaceController + " session " + mSession

                    + ": " + e.toString());

                + " surface " + mSurfaceController + " session " + mSession + ": " + e.toString());

        }



        // Whether the surface was preserved (and copied to mPendingDestroySurface) or not, it
@@ -836,7 +836,6 @@ class WindowStateAnimator { 
        mSurfaceController = null;

        mDrawState = NO_SURFACE;

    }

    }



    void destroyDeferredSurfaceLocked() {

        try {
@@ -1746,8 +1745,18 @@ class WindowStateAnimator { 
    }



    void destroySurface() {

        try {

            if (mSurfaceController != null) {

                mSurfaceController.destroyInTransaction();

            }

        } catch (RuntimeException e) {

            Slog.w(TAG, "Exception thrown when destroying surface " + this

                    + " surface " + mSurfaceController + " session " + mSession + ": " + e);

        } finally {

            mWin.setHasSurface(false);

            mSurfaceController = null;

            mDrawState = NO_SURFACE;

        }

    }



    void setMoveAnimation(int left, int top) {

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

+7 −3
Original line number Diff line number Diff line
@@ -16,6 +16,7 @@ 


package com.android.server.wm;



import static com.android.server.wm.WindowManagerDebugConfig.SHOW_SURFACE_ALLOC;

import static com.android.server.wm.WindowManagerDebugConfig.SHOW_TRANSACTIONS;

import static com.android.server.wm.WindowManagerDebugConfig.SHOW_LIGHT_TRANSACTIONS;

import static com.android.server.wm.WindowManagerDebugConfig.DEBUG_SURFACE_TRACE;
@@ -133,11 +134,14 @@ class WindowSurfaceController { 
        Slog.i(TAG, "Destroying surface " + this + " called by " + Debug.getCallers(4));

        //        }

        try {

            if (mSurfaceControl != null) {

                mSurfaceControl.destroy();

            mSurfaceShown = false;

            mSurfaceControl = null;

            }

        } catch (RuntimeException e) {

            Slog.w(TAG, "Error destroying surface in: " + this, e);

        } finally {

            mSurfaceShown = false;

            mSurfaceControl = null;

        }

    }