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
diff --git a/libs/hwui/DisplayListRenderer.cpp b/libs/hwui/DisplayListRenderer.cpp
index 92207b1..f6ca77c 100644
--- a/libs/hwui/DisplayListRenderer.cpp
+++ b/libs/hwui/DisplayListRenderer.cpp
@@ -184,6 +184,11 @@
}
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 @@
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 @@
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();
diff --git a/libs/hwui/DisplayListRenderer.h b/libs/hwui/DisplayListRenderer.h
index a7fc23a..4edefd5 100644
--- a/libs/hwui/DisplayListRenderer.h
+++ b/libs/hwui/DisplayListRenderer.h
@@ -487,6 +487,7 @@
Vector<SkPaint*> mPaints;
Vector<SkPath*> mPaths;
+ SortedVector<SkPath*> mSourcePaths;
Vector<SkMatrix*> mMatrices;
Vector<SkiaShader*> mShaders;
@@ -634,6 +635,10 @@
return mPaths;
}
+ const SortedVector<SkPath*>& getSourcePaths() const {
+ return mSourcePaths;
+ }
+
const Vector<SkMatrix*>& getMatrices() const {
return mMatrices;
}
@@ -750,6 +755,10 @@
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 @@
Vector<SkPath*> mPaths;
DefaultKeyedVector<SkPath*, SkPath*> mPathMap;
+ SortedVector<SkPath*> mSourcePaths;
+
Vector<SkiaShader*> mShaders;
DefaultKeyedVector<SkiaShader*, SkiaShader*> mShaderMap;
diff --git a/tests/HwAccelerationTest/AndroidManifest.xml b/tests/HwAccelerationTest/AndroidManifest.xml
index 786cba3..9e103ac 100644
--- a/tests/HwAccelerationTest/AndroidManifest.xml
+++ b/tests/HwAccelerationTest/AndroidManifest.xml
@@ -658,6 +658,15 @@
</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">
<intent-filter>
diff --git a/tests/HwAccelerationTest/src/com/android/test/hwui/PathDestructionActivity.java b/tests/HwAccelerationTest/src/com/android/test/hwui/PathDestructionActivity.java
new file mode 100644
index 0000000..4177725
--- /dev/null
+++ b/tests/HwAccelerationTest/src/com/android/test/hwui/PathDestructionActivity.java
@@ -0,0 +1,101 @@
+/*
+ * 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();
+ }
+ }
+}