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

Commit d34dd718 authored by Chet Haase's avatar Chet Haase
Browse files

Fix hang/crash in native path code 

An optimization for paths is to only create a texture for the original native
Path object, and have all copies of that object use that texture. This works in
most cases, but sometimes that original path object may get destroyed (when the
SDK path object is finalized) while we are still referencing and using that object
in the DisplayList code. This causes undefined errors such as crashes and hanging
as we iterate through the operations of a destroyed (and garbage-filled) path object.

The fix is to use the existing ResourceCache to refcount the original path until
we are done with it.

Issue #6414050 Analytics Dogfood App crashes reliably on Jellybean

Change-Id: I5dbec5c069f7d6a1e68c13424f454976a7d188e9
parent 5380cdc2

libs/hwui/DisplayListRenderer.cpp

+16 −0
Original line number Diff line number Diff line
@@ -184,6 +184,11 @@ void DisplayList::clearResources() { 
    }

    mPaths.clear();



    for (size_t i = 0; i < mSourcePaths.size(); i++) {

        caches.resourceCache.decrementRefcount(mSourcePaths.itemAt(i));

    }

    mSourcePaths.clear();



    for (size_t i = 0; i < mMatrices.size(); i++) {

        delete mMatrices.itemAt(i);

    }
@@ -242,6 +247,12 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde 
        mPaths.add(paths.itemAt(i));

    }



    const SortedVector<SkPath*> &sourcePaths = recorder.getSourcePaths();

    for (size_t i = 0; i < sourcePaths.size(); i++) {

        mSourcePaths.add(sourcePaths.itemAt(i));

        caches.resourceCache.incrementRefcount(sourcePaths.itemAt(i));

    }



    const Vector<SkMatrix*> &matrices = recorder.getMatrices();

    for (size_t i = 0; i < matrices.size(); i++) {

        mMatrices.add(matrices.itemAt(i));
@@ -1273,6 +1284,11 @@ void DisplayListRenderer::reset() { 
    mShaders.clear();

    mShaderMap.clear();



    for (size_t i = 0; i < mSourcePaths.size(); i++) {

        caches.resourceCache.decrementRefcount(mSourcePaths.itemAt(i));

    }

    mSourcePaths.clear();



    mPaints.clear();

    mPaintMap.clear();

libs/hwui/DisplayListRenderer.h

+11 −0
Original line number Diff line number Diff line
@@ -487,6 +487,7 @@ private: 


    Vector<SkPaint*> mPaints;

    Vector<SkPath*> mPaths;

    SortedVector<SkPath*> mSourcePaths;

    Vector<SkMatrix*> mMatrices;

    Vector<SkiaShader*> mShaders;
@@ -634,6 +635,10 @@ public: 
        return mPaths;

    }



    const SortedVector<SkPath*>& getSourcePaths() const {

        return mSourcePaths;

    }



    const Vector<SkMatrix*>& getMatrices() const {

        return mMatrices;

    }
@@ -750,6 +755,10 @@ private: 
            mPathMap.replaceValueFor(path, pathCopy);

            mPaths.add(pathCopy);

        }

        if (mSourcePaths.indexOf(path) < 0) {

            Caches::getInstance().resourceCache.incrementRefcount(path);

            mSourcePaths.add(path);

        }



        addInt((int) pathCopy);

    }
@@ -830,6 +839,8 @@ private: 
    Vector<SkPath*> mPaths;

    DefaultKeyedVector<SkPath*, SkPath*> mPathMap;



    SortedVector<SkPath*> mSourcePaths;



    Vector<SkiaShader*> mShaders;

    DefaultKeyedVector<SkiaShader*, SkiaShader*> mShaderMap;

tests/HwAccelerationTest/AndroidManifest.xml

+9 −0
Original line number Diff line number Diff line
@@ -657,6 +657,15 @@ 
            </intent-filter>

        </activity>



        <activity

                android:name="PathDestructionActivity"

                android:label="_PathDestruction">

            <intent-filter>

                <action android:name="android.intent.action.MAIN" />

                <category android:name="android.intent.category.LAUNCHER" />

            </intent-filter>

        </activity>



        <activity

                android:name="TransformsAndAnimationsActivity"

                android:label="_TransformAnim">

tests/HwAccelerationTest/src/com/android/test/hwui/PathDestructionActivity.java

0 → 100644
+101 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2012 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.test.hwui;



import android.app.Activity;

import android.content.Context;

import android.graphics.Canvas;

import android.graphics.Paint;

import android.graphics.Path;

import android.os.Bundle;

import android.util.MathUtils;

import android.view.View;



/**

 * The point of this test is to ensure that we can cause many paths to be created, drawn,

 * and destroyed without causing hangs or crashes. This tests the native reference counting

 * scheme in particular, because we should be able to have the Java-level path finalized

 * without destroying the underlying native path object until we are done referencing it

 * in pending DisplayLists.

 */

public class PathDestructionActivity extends Activity {



    private static final int MIN_SIZE = 20;

    @Override

    protected void onCreate(Bundle savedInstanceState) {

        super.onCreate(savedInstanceState);



        MyView view = new MyView(this);

        setContentView(view);

    }



    private static class MyView extends View {

        Paint strokePaint = new Paint(Paint.ANTI_ALIAS_FLAG);

        Paint fillPaint = new Paint(Paint.ANTI_ALIAS_FLAG);

        Paint fillAndStrokePaint = new Paint(Paint.ANTI_ALIAS_FLAG);



        private MyView(Context context) {

            super(context);

            strokePaint.setStyle(Paint.Style.STROKE);

            fillPaint.setStyle(Paint.Style.FILL);

            fillAndStrokePaint.setStyle(Paint.Style.FILL_AND_STROKE);

        }



        private Path getRandomPath() {

            float left, top, right, bottom;

            left = MathUtils.random(getWidth() - MIN_SIZE);

            top = MathUtils.random(getHeight() - MIN_SIZE);

            right = left + MathUtils.random(getWidth() - left);

            bottom = top + MathUtils.random(getHeight() - top);

            Path path = new Path();

            path.moveTo(left, top);

            path.lineTo(right, top);

            path.lineTo(right, bottom);

            path.lineTo(left, bottom);

            path.close();

            return path;

        }



        private int getRandomColor() {

            int red = MathUtils.random(255);

            int green = MathUtils.random(255);

            int blue = MathUtils.random(255);

            return 0xff000000 | red << 16 | green << 8 | blue;

        }



        @Override

        protected void onDraw(Canvas canvas) {

            Path path;

            for (int i = 0; i < 15; ++i) {

                path = getRandomPath();

                strokePaint.setColor(getRandomColor());

                canvas.drawPath(path, strokePaint);

                path = null;

                path = getRandomPath();

                fillPaint.setColor(getRandomColor());

                canvas.drawPath(path, fillPaint);

                path = null;

                path = getRandomPath();

                fillAndStrokePaint.setColor(getRandomColor());

                canvas.drawPath(path, fillAndStrokePaint);

                path = null;

            }



            invalidate();

        }

    }

}