Skip to content

Commit 603f6de

Browse files
chethaasechethaase
committed
Fix occasional crash bug with layers
Launcher occasionally crashes with a stack trace indicating that the memory of a Layer object is corrupt. It is possible for us to delete a Layer structure and then, briefly, use it to draw a DisplayList again before that DisplayList gets recreated (without the layer that got deleted). When this happens, if the memory got corrupted, it's possible to crash. The fix is to add Layer to the other objects which we currently refcount (bitmaps, shaders, etc.). Then instead of deleting a Layer, we decrement the refcount. We increment when creating it, then increment it again when it's referenced from a DisplayList. Then we decrement the refcount instead of deleting it, and decrement when we clear a DisplayList that refers to it. Then when the refcount reaches 0, we delete it. Issue #6994632 Native crash in launcher when trying to launch all apps screen Change-Id: I0627be8d49bb2f9ba8d158a84b764bb4e7df934c
1 parent cc5dd18 commit 603f6de

16 files changed

+119
-57
lines changed

‎core/java/android/view/GLES20Canvas.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ protected void finalize() throws Throwable {
150150

151151
static native int nCreateTextureLayer(boolean opaque, int[] layerInfo);
152152
static native int nCreateLayer(int width, int height, boolean isOpaque, int[] layerInfo);
153-
static native void nResizeLayer(int layerId, int width, int height, int[] layerInfo);
153+
static native boolean nResizeLayer(int layerId, int width, int height, int[] layerInfo);
154154
static native void nSetOpaqueLayer(int layerId, boolean isOpaque);
155155
static native void nSetLayerPaint(int layerId, int nativePaint);
156156
static native void nSetLayerColorFilter(int layerId, int nativeColorFilter);

‎core/java/android/view/GLES20RenderLayer.java‎

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,20 +54,26 @@ boolean isValid() {
5454
}
5555

5656
@Override
57-
void resize(int width, int height) {
58-
if (!isValid() || width <= 0 || height <= 0) return;
57+
boolean resize(int width, int height) {
58+
if (!isValid() || width <= 0 || height <= 0) return false;
5959

6060
mWidth = width;
6161
mHeight = height;
6262

6363
if (width != mLayerWidth || height != mLayerHeight) {
6464
int[] layerInfo = new int[2];
6565

66-
GLES20Canvas.nResizeLayer(mLayer, width, height, layerInfo);
67-
68-
mLayerWidth = layerInfo[0];
69-
mLayerHeight = layerInfo[1];
66+
if (GLES20Canvas.nResizeLayer(mLayer, width, height, layerInfo)) {
67+
mLayerWidth = layerInfo[0];
68+
mLayerHeight = layerInfo[1];
69+
} else {
70+
// Failure: not enough GPU resources for requested size
71+
mLayer = 0;
72+
mLayerWidth = 0;
73+
mLayerHeight = 0;
74+
}
7075
}
76+
return isValid();
7177
}
7278

7379
@Override

‎core/java/android/view/GLES20TextureLayer.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,8 @@ boolean isValid() {
4848
}
4949

5050
@Override
51-
void resize(int width, int height) {
51+
boolean resize(int width, int height) {
52+
return isValid();
5253
}
5354

5455
@Override

‎core/java/android/view/HardwareLayer.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,8 +135,9 @@ boolean isOpaque() {
135135
*
136136
* @param width The new desired minimum width for this layer
137137
* @param height The new desired minimum height for this layer
138+
* @return True if the resulting layer is valid, false otherwise
138139
*/
139-
abstract void resize(int width, int height);
140+
abstract boolean resize(int width, int height);
140141

141142
/**
142143
* Returns a hardware canvas that can be used to render onto

‎core/java/android/view/View.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12213,8 +12213,9 @@ HardwareLayer getHardwareLayer() {
1221312213
mLocalDirtyRect.set(0, 0, width, height);
1221412214
} else {
1221512215
if (mHardwareLayer.getWidth() != width || mHardwareLayer.getHeight() != height) {
12216-
mHardwareLayer.resize(width, height);
12217-
mLocalDirtyRect.set(0, 0, width, height);
12216+
if (mHardwareLayer.resize(width, height)) {
12217+
mLocalDirtyRect.set(0, 0, width, height);
12218+
}
1221812219
}
1221912220

1222012221
// This should not be necessary but applications that change
@@ -12225,7 +12226,7 @@ HardwareLayer getHardwareLayer() {
1222512226
computeOpaqueFlags();
1222612227

1222712228
final boolean opaque = isOpaque();
12228-
if (mHardwareLayer.isOpaque() != opaque) {
12229+
if (mHardwareLayer.isValid() && mHardwareLayer.isOpaque() != opaque) {
1222912230
mHardwareLayer.setOpaque(opaque);
1223012231
mLocalDirtyRect.set(0, 0, width, height);
1223112232
}

‎core/java/android/view/ViewRootImpl.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1402,6 +1402,7 @@ private void performTraversals() {
14021402
mResizeBuffer.getHeight() != mHeight) {
14031403
mResizeBuffer.resize(mWidth, mHeight);
14041404
}
1405+
// TODO: should handle create/resize failure
14051406
layerCanvas = mResizeBuffer.start(hwRendererCanvas);
14061407
layerCanvas.setViewport(mWidth, mHeight);
14071408
layerCanvas.onPreDraw(null);

‎core/jni/android_view_GLES20Canvas.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -766,14 +766,16 @@ static Layer* android_view_GLES20Canvas_createLayer(JNIEnv* env, jobject clazz,
766766
return layer;
767767
}
768768

769-
static void android_view_GLES20Canvas_resizeLayer(JNIEnv* env, jobject clazz,
769+
static bool android_view_GLES20Canvas_resizeLayer(JNIEnv* env, jobject clazz,
770770
Layer* layer, jint width, jint height, jintArray layerInfo) {
771-
LayerRenderer::resizeLayer(layer, width, height);
772-
773-
jint* storage = env->GetIntArrayElements(layerInfo, NULL);
774-
storage[0] = layer->getWidth();
775-
storage[1] = layer->getHeight();
776-
env->ReleaseIntArrayElements(layerInfo, storage, 0);
771+
if (LayerRenderer::resizeLayer(layer, width, height)) {
772+
jint* storage = env->GetIntArrayElements(layerInfo, NULL);
773+
storage[0] = layer->getWidth();
774+
storage[1] = layer->getHeight();
775+
env->ReleaseIntArrayElements(layerInfo, storage, 0);
776+
return true;
777+
}
778+
return false;
777779
}
778780

779781
static void android_view_GLES20Canvas_setLayerPaint(JNIEnv* env, jobject clazz,
@@ -992,7 +994,7 @@ static JNINativeMethod gMethods[] = {
992994

993995
{ "nCreateLayerRenderer", "(I)I", (void*) android_view_GLES20Canvas_createLayerRenderer },
994996
{ "nCreateLayer", "(IIZ[I)I", (void*) android_view_GLES20Canvas_createLayer },
995-
{ "nResizeLayer", "(III[I)V" , (void*) android_view_GLES20Canvas_resizeLayer },
997+
{ "nResizeLayer", "(III[I)Z" , (void*) android_view_GLES20Canvas_resizeLayer },
996998
{ "nSetLayerPaint", "(II)V", (void*) android_view_GLES20Canvas_setLayerPaint },
997999
{ "nSetLayerColorFilter", "(II)V", (void*) android_view_GLES20Canvas_setLayerColorFilter },
9981000
{ "nSetOpaqueLayer", "(IZ)V", (void*) android_view_GLES20Canvas_setOpaqueLayer },

‎libs/hwui/DisplayListRenderer.cpp‎

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,10 @@ void DisplayList::clearResources() {
182182
caches.resourceCache.decrementRefcountLocked(mSourcePaths.itemAt(i));
183183
}
184184

185+
for (size_t i = 0; i < mLayers.size(); i++) {
186+
caches.resourceCache.decrementRefcountLocked(mLayers.itemAt(i));
187+
}
188+
185189
caches.resourceCache.unlock();
186190

187191
for (size_t i = 0; i < mPaints.size(); i++) {
@@ -206,6 +210,7 @@ void DisplayList::clearResources() {
206210
mPaints.clear();
207211
mPaths.clear();
208212
mMatrices.clear();
213+
mLayers.clear();
209214
}
210215

211216
void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorder, bool reusing) {
@@ -264,6 +269,12 @@ void DisplayList::initFromDisplayListRenderer(const DisplayListRenderer& recorde
264269
caches.resourceCache.incrementRefcountLocked(sourcePaths.itemAt(i));
265270
}
266271

272+
const Vector<Layer*>& layers = recorder.getLayers();
273+
for (size_t i = 0; i < layers.size(); i++) {
274+
mLayers.add(layers.itemAt(i));
275+
caches.resourceCache.incrementRefcountLocked(layers.itemAt(i));
276+
}
277+
267278
caches.resourceCache.unlock();
268279

269280
const Vector<SkPaint*>& paints = recorder.getPaints();
@@ -1361,6 +1372,10 @@ void DisplayListRenderer::reset() {
13611372
mCaches.resourceCache.decrementRefcountLocked(mSourcePaths.itemAt(i));
13621373
}
13631374

1375+
for (size_t i = 0; i < mLayers.size(); i++) {
1376+
mCaches.resourceCache.decrementRefcountLocked(mLayers.itemAt(i));
1377+
}
1378+
13641379
mCaches.resourceCache.unlock();
13651380

13661381
mBitmapResources.clear();
@@ -1379,6 +1394,8 @@ void DisplayListRenderer::reset() {
13791394

13801395
mMatrices.clear();
13811396

1397+
mLayers.clear();
1398+
13821399
mHasDrawOps = false;
13831400
}
13841401

@@ -1539,7 +1556,7 @@ status_t DisplayListRenderer::drawDisplayList(DisplayList* displayList,
15391556

15401557
status_t DisplayListRenderer::drawLayer(Layer* layer, float x, float y, SkPaint* paint) {
15411558
addOp(DisplayList::DrawLayer);
1542-
addInt((int) layer);
1559+
addLayer(layer);
15431560
addPoint(x, y);
15441561
addPaint(paint);
15451562
return DrawGlInfo::kStatusDone;

‎libs/hwui/DisplayListRenderer.h‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -496,6 +496,7 @@ class DisplayList {
496496
SortedVector<SkPath*> mSourcePaths;
497497
Vector<SkMatrix*> mMatrices;
498498
Vector<SkiaShader*> mShaders;
499+
Vector<Layer*> mLayers;
499500

500501
mutable SkFlattenableReadBuffer mReader;
501502

@@ -652,6 +653,10 @@ class DisplayListRenderer: public OpenGLRenderer {
652653
return mSourcePaths;
653654
}
654655

656+
const Vector<Layer*>& getLayers() const {
657+
return mLayers;
658+
}
659+
655660
const Vector<SkMatrix*>& getMatrices() const {
656661
return mMatrices;
657662
}
@@ -804,6 +809,12 @@ class DisplayListRenderer: public OpenGLRenderer {
804809
mMatrices.add(copy);
805810
}
806811

812+
inline void addLayer(Layer* layer) {
813+
addInt((int) layer);
814+
mLayers.add(layer);
815+
mCaches.resourceCache.incrementRefcount(layer);
816+
}
817+
807818
inline void addBitmap(SkBitmap* bitmap) {
808819
// Note that this assumes the bitmap is immutable. There are cases this won't handle
809820
// correctly, such as creating the bitmap from scratch, drawing with it, changing its
@@ -862,6 +873,8 @@ class DisplayListRenderer: public OpenGLRenderer {
862873

863874
Vector<SkMatrix*> mMatrices;
864875

876+
Vector<Layer*> mLayers;
877+
865878
uint32_t mBufferSize;
866879

867880
int mRestoreSaveCount;

‎libs/hwui/Layer.cpp‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,10 +25,29 @@
2525
namespace android {
2626
namespace uirenderer {
2727

28+
Layer::Layer(const uint32_t layerWidth, const uint32_t layerHeight) {
29+
mesh = NULL;
30+
meshIndices = NULL;
31+
meshElementCount = 0;
32+
cacheable = true;
33+
textureLayer = false;
34+
renderTarget = GL_TEXTURE_2D;
35+
texture.width = layerWidth;
36+
texture.height = layerHeight;
37+
colorFilter = NULL;
38+
deferredUpdateScheduled = false;
39+
renderer = NULL;
40+
displayList = NULL;
41+
fbo = 0;
42+
Caches::getInstance().resourceCache.incrementRefcount(this);
43+
}
44+
2845
Layer::~Layer() {
2946
if (mesh) delete mesh;
3047
if (meshIndices) delete meshIndices;
3148
if (colorFilter) Caches::getInstance().resourceCache.decrementRefcount(colorFilter);
49+
if (fbo) Caches::getInstance().fboCache.put(fbo);
50+
deleteTexture();
3251
}
3352

3453
void Layer::setPaint(SkPaint* paint) {

0 commit comments

Comments
 (0)