From 17035b0211a3c9d45ea46a99217a6acbe76e8fbe Mon Sep 17 00:00:00 2001 From: John Reck Date: Wed, 3 Sep 2014 07:39:53 -0700 Subject: [PATCH] Have destroy call freePrefetchedLayers Bug: 17208461 There's a potential race condition between HardwareRenderer.destroy() being called (which calls destroyCanvasAndSurface()) and the renderer being finalized (which is what calls freePrefetchedLayers), during which time it's possible we get a TRIM_MEMORY_COMPLETE and destroy the EGL context. Fix this race condition by moving stopDrawing() and freePrefetchedLayers() into destroyCanvasAndSurface() where they should have been in the first place. Also, if we hit the assertion failure, dump the current state of Caches to try and provide more context for the failure. Change-Id: Ife0ba3562041e8b08e87e3e13640472b3004eed6 --- core/java/android/view/ThreadedRenderer.java | 4 ++-- core/jni/android_view_ThreadedRenderer.cpp | 6 +++--- libs/hwui/Caches.cpp | 10 +++++++--- libs/hwui/Caches.h | 5 +++++ libs/hwui/LayerCache.cpp | 4 ++++ libs/hwui/LayerCache.h | 2 ++ libs/hwui/RenderState.cpp | 6 +++++- libs/hwui/RenderState.h | 1 + libs/hwui/renderthread/CanvasContext.cpp | 9 +++++---- libs/hwui/renderthread/CanvasContext.h | 2 +- libs/hwui/renderthread/RenderProxy.cpp | 8 ++++---- libs/hwui/renderthread/RenderProxy.h | 2 +- 12 files changed, 40 insertions(+), 19 deletions(-) diff --git a/core/java/android/view/ThreadedRenderer.java b/core/java/android/view/ThreadedRenderer.java index ca08eccc53656..5d2822d895aec 100644 --- a/core/java/android/view/ThreadedRenderer.java +++ b/core/java/android/view/ThreadedRenderer.java @@ -126,7 +126,7 @@ public class ThreadedRenderer extends HardwareRenderer { void destroy() { mInitialized = false; updateEnabledState(null); - nDestroyCanvasAndSurface(mNativeProxy); + nDestroy(mNativeProxy); } private void updateEnabledState(Surface surface) { @@ -488,7 +488,7 @@ public class ThreadedRenderer extends HardwareRenderer { private static native void nSetOpaque(long nativeProxy, boolean opaque); private static native int nSyncAndDrawFrame(long nativeProxy, long frameTimeNanos, long recordDuration, float density); - private static native void nDestroyCanvasAndSurface(long nativeProxy); + private static native void nDestroy(long nativeProxy); private static native void nRegisterAnimatingRenderNode(long rootRenderNode, long animatingNode); private static native void nInvokeFunctor(long functor, boolean waitForCompletion); diff --git a/core/jni/android_view_ThreadedRenderer.cpp b/core/jni/android_view_ThreadedRenderer.cpp index 99babacecec44..6ec6b006fb5a0 100644 --- a/core/jni/android_view_ThreadedRenderer.cpp +++ b/core/jni/android_view_ThreadedRenderer.cpp @@ -291,10 +291,10 @@ static int android_view_ThreadedRenderer_syncAndDrawFrame(JNIEnv* env, jobject c return proxy->syncAndDrawFrame(frameTimeNanos, recordDuration, density); } -static void android_view_ThreadedRenderer_destroyCanvasAndSurface(JNIEnv* env, jobject clazz, +static void android_view_ThreadedRenderer_destroy(JNIEnv* env, jobject clazz, jlong proxyPtr) { RenderProxy* proxy = reinterpret_cast(proxyPtr); - proxy->destroyCanvasAndSurface(); + proxy->destroy(); } static void android_view_ThreadedRenderer_registerAnimatingRenderNode(JNIEnv* env, jobject clazz, @@ -430,7 +430,7 @@ static JNINativeMethod gMethods[] = { { "nSetup", "(JIIFFFFII)V", (void*) android_view_ThreadedRenderer_setup }, { "nSetOpaque", "(JZ)V", (void*) android_view_ThreadedRenderer_setOpaque }, { "nSyncAndDrawFrame", "(JJJF)I", (void*) android_view_ThreadedRenderer_syncAndDrawFrame }, - { "nDestroyCanvasAndSurface", "(J)V", (void*) android_view_ThreadedRenderer_destroyCanvasAndSurface }, + { "nDestroy", "(J)V", (void*) android_view_ThreadedRenderer_destroy }, { "nRegisterAnimatingRenderNode", "(JJ)V", (void*) android_view_ThreadedRenderer_registerAnimatingRenderNode }, { "nInvokeFunctor", "(JZ)V", (void*) android_view_ThreadedRenderer_invokeFunctor }, { "nCreateDisplayListLayer", "(JII)J", (void*) android_view_ThreadedRenderer_createDisplayListLayer }, diff --git a/libs/hwui/Caches.cpp b/libs/hwui/Caches.cpp index 5689e170f5fd2..9855f718930dd 100644 --- a/libs/hwui/Caches.cpp +++ b/libs/hwui/Caches.cpp @@ -24,6 +24,7 @@ #include "Properties.h" #include "LayerRenderer.h" #include "ShadowTessellator.h" +#include "RenderState.h" namespace android { @@ -49,7 +50,7 @@ namespace uirenderer { /////////////////////////////////////////////////////////////////////////////// Caches::Caches(): Singleton(), - mExtensions(Extensions::getInstance()), mInitialized(false) { + mExtensions(Extensions::getInstance()), mInitialized(false), mRenderState(NULL) { init(); initFont(); initConstraints(); @@ -267,8 +268,11 @@ void Caches::dumpMemoryUsage(String8 &log) { log.appendFormat("Current memory usage / total memory usage (bytes):\n"); log.appendFormat(" TextureCache %8d / %8d\n", textureCache.getSize(), textureCache.getMaxSize()); - log.appendFormat(" LayerCache %8d / %8d\n", - layerCache.getSize(), layerCache.getMaxSize()); + log.appendFormat(" LayerCache %8d / %8d (numLayers = %zu)\n", + layerCache.getSize(), layerCache.getMaxSize(), layerCache.getCount()); + log.appendFormat(" Garbage layers %8zu\n", mLayerGarbage.size()); + log.appendFormat(" Active layers %8zu\n", + mRenderState ? mRenderState->mActiveLayers.size() : 0); log.appendFormat(" RenderBufferCache %8d / %8d\n", renderBufferCache.getSize(), renderBufferCache.getMaxSize()); log.appendFormat(" GradientCache %8d / %8d\n", diff --git a/libs/hwui/Caches.h b/libs/hwui/Caches.h index 0482430f404aa..726b74da42f86 100644 --- a/libs/hwui/Caches.h +++ b/libs/hwui/Caches.h @@ -107,6 +107,7 @@ struct CacheLogger { /////////////////////////////////////////////////////////////////////////////// class RenderNode; +class RenderState; class ANDROID_API Caches: public Singleton { Caches(); @@ -132,6 +133,8 @@ public: */ bool initProperties(); + void setRenderState(RenderState* renderState) { mRenderState = renderState; } + /** * Flush the cache. * @@ -431,6 +434,8 @@ private: GLuint mBoundTextures[REQUIRED_TEXTURE_UNITS_COUNT]; OverdrawColorSet mOverdrawDebugColorSet; + + RenderState* mRenderState; }; // class Caches }; // namespace uirenderer diff --git a/libs/hwui/LayerCache.cpp b/libs/hwui/LayerCache.cpp index 13869aadea168..efa30ac36a6ee 100644 --- a/libs/hwui/LayerCache.cpp +++ b/libs/hwui/LayerCache.cpp @@ -49,6 +49,10 @@ LayerCache::~LayerCache() { // Size management /////////////////////////////////////////////////////////////////////////////// +size_t LayerCache::getCount() { + return mCache.size(); +} + uint32_t LayerCache::getSize() { return mSize; } diff --git a/libs/hwui/LayerCache.h b/libs/hwui/LayerCache.h index 1b0fc2cfd1d7a..6b93e8fc86c5f 100644 --- a/libs/hwui/LayerCache.h +++ b/libs/hwui/LayerCache.h @@ -87,6 +87,8 @@ public: */ uint32_t getSize(); + size_t getCount(); + /** * Prints out the content of the cache. */ diff --git a/libs/hwui/RenderState.cpp b/libs/hwui/RenderState.cpp index 50b8f395011c9..9948b44ac703d 100644 --- a/libs/hwui/RenderState.cpp +++ b/libs/hwui/RenderState.cpp @@ -32,10 +32,14 @@ void RenderState::onGLContextCreated() { // This is delayed because the first access of Caches makes GL calls mCaches = &Caches::getInstance(); mCaches->init(); + mCaches->setRenderState(this); } void RenderState::onGLContextDestroyed() { - LOG_ALWAYS_FATAL_IF(!mActiveLayers.empty(), "layers have survived gl context destruction"); + if (CC_UNLIKELY(!mActiveLayers.empty())) { + mCaches->dumpMemoryUsage(); + LOG_ALWAYS_FATAL("layers have survived gl context destruction"); + } } void RenderState::setViewport(GLsizei width, GLsizei height) { diff --git a/libs/hwui/RenderState.h b/libs/hwui/RenderState.h index cd71c73291435..3915fb5f3f83f 100644 --- a/libs/hwui/RenderState.h +++ b/libs/hwui/RenderState.h @@ -59,6 +59,7 @@ public: private: friend class renderthread::RenderThread; + friend class Caches; void interruptForFunctorInvoke(); void resumeFromFunctorInvoke(); diff --git a/libs/hwui/renderthread/CanvasContext.cpp b/libs/hwui/renderthread/CanvasContext.cpp index 0a17e132eed60..967cb6ffcde5e 100644 --- a/libs/hwui/renderthread/CanvasContext.cpp +++ b/libs/hwui/renderthread/CanvasContext.cpp @@ -51,13 +51,14 @@ CanvasContext::CanvasContext(RenderThread& thread, bool translucent, } CanvasContext::~CanvasContext() { - destroyCanvasAndSurface(); - mRenderThread.removeFrameCallback(this); + destroy(); delete mAnimationContext; - freePrefetechedLayers(); } -void CanvasContext::destroyCanvasAndSurface() { +void CanvasContext::destroy() { + stopDrawing(); + freePrefetechedLayers(); + destroyHardwareResources(); if (mCanvas) { delete mCanvas; mCanvas = 0; diff --git a/libs/hwui/renderthread/CanvasContext.h b/libs/hwui/renderthread/CanvasContext.h index 7c27190b137b2..5984fd0950a27 100644 --- a/libs/hwui/renderthread/CanvasContext.h +++ b/libs/hwui/renderthread/CanvasContext.h @@ -66,7 +66,7 @@ public: void processLayerUpdate(DeferredLayerUpdater* layerUpdater); void prepareTree(TreeInfo& info); void draw(); - void destroyCanvasAndSurface(); + void destroy(); // IFrameCallback, Chroreographer-driven frame callback entry point virtual void doFrame(); diff --git a/libs/hwui/renderthread/RenderProxy.cpp b/libs/hwui/renderthread/RenderProxy.cpp index 3d043169682fb..9528874b92d6e 100644 --- a/libs/hwui/renderthread/RenderProxy.cpp +++ b/libs/hwui/renderthread/RenderProxy.cpp @@ -194,13 +194,13 @@ int RenderProxy::syncAndDrawFrame(nsecs_t frameTimeNanos, nsecs_t recordDuration return mDrawFrameTask.drawFrame(frameTimeNanos, recordDurationNanos); } -CREATE_BRIDGE1(destroyCanvasAndSurface, CanvasContext* context) { - args->context->destroyCanvasAndSurface(); +CREATE_BRIDGE1(destroy, CanvasContext* context) { + args->context->destroy(); return NULL; } -void RenderProxy::destroyCanvasAndSurface() { - SETUP_TASK(destroyCanvasAndSurface); +void RenderProxy::destroy() { + SETUP_TASK(destroy); args->context = mContext; // destroyCanvasAndSurface() needs a fence as when it returns the // underlying BufferQueue is going to be released from under diff --git a/libs/hwui/renderthread/RenderProxy.h b/libs/hwui/renderthread/RenderProxy.h index 9e6bcf51a7298..8b8d99c0ad7b5 100644 --- a/libs/hwui/renderthread/RenderProxy.h +++ b/libs/hwui/renderthread/RenderProxy.h @@ -73,7 +73,7 @@ public: ANDROID_API void setOpaque(bool opaque); ANDROID_API int syncAndDrawFrame(nsecs_t frameTimeNanos, nsecs_t recordDurationNanos, float density); - ANDROID_API void destroyCanvasAndSurface(); + ANDROID_API void destroy(); ANDROID_API static void invokeFunctor(Functor* functor, bool waitForCompletion);