Skip to content
Commit 32f37ab0 authored by Robert Carr's avatar Robert Carr
Browse files

ViewRootImpl: More null checks for performTraversals.

Let's first understand how mView could become null. Notice
at the beginning of performTraversals, there is a check that
mView != null, and so it is nulled while we are in the function.
mView is package private and there are only two places which assign
to it, ViewRootImpl#setView and ViewRootImpl#doDie. setView is guarded
by mView == null. But mView was not null (per the check at the beginning
of performTraversals) and so mView is being nulled by doDie().

doDie() only has 3 callpoints:
        1. ViewRootImpl#die(). Here though, calling it is guarded by
        !mIsInTraversal. !mIsInTraversal is unconditionally set at the
        beginning of performTraversals, and so this isn't our caller.
        2. ViewRootHandler, handling MSG_DIE. This runs on the same thread
        as performTraversal, and so it can't be our nuller.
        3. WindowManagerGlobal#addView. This must be our nuller.

We see WindowManagerGlobal#addView will call doDie in the case that
we attempt to add a view which we had previously set to be removed
but deferred removal of. Now we can construct a reasonable sequence
for getting here:
    1. requestLayout(). Perform traversals ends up on handler.
    2. removeView(). MSG_DIE ends up on handler, View ends up in mDyingViews
    3. performTraversals is executed by the handler
    4. From a callback initiated by performTraversals (e.g. measure)
       the client calls WindowManagerGlobal#addView on the view which
       was just removed.
    5. We are still in performTraversals so MSG_DIE hasn't been processed
       yet. This means that WindowManagerGlobal will perform the doDie
       immediately nulling mView.
    6. We return to performTraversals and crash.

We can see shortly after the offending call to doDie, a new ViewRoot will be
constructed and so whatever traversal we are doing on the old one doesn't
seem particularly important. It doesn't seem that we can do any better
than letting it fall through without crashing.

Bug: 38421184
Test: go/wm-smoke. Feed to the monkeys.
Change-Id: I55f310a3533175c9df4a82878be5a60fd01b80c1
parent 7c41ddb9
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment