From 1b0114f5db78c46b1f4c6a83e6d219bbe1e838e4 Mon Sep 17 00:00:00 2001
From: Mathias Agopian <mathias@google.com>
Date: Tue, 15 Feb 2011 19:01:06 -0800
Subject: [PATCH] fix a surface leak in SurfaceFlinger

SF kept a strong reference to ISurface until the
window manager removed the surface from the screen.
This fell appart when running standalone tests, that is
when the window manager wasn't involved.

When the window manager is around, it would clean-up surfaces
even when an application died.

with this change, SF is able to do its own cleanup without
relying on the window manager.

the change is very simple, we simply don't keep a reference
to ISurface and make sure no more than one of them can
be created.

Change-Id: I61f2d7473bf8d4aa651549a846c34cdbb0d0c85a
---
 services/surfaceflinger/Layer.cpp     |  7 ++-----
 services/surfaceflinger/Layer.h       |  1 -
 services/surfaceflinger/LayerBase.cpp | 17 ++++++++++-------
 services/surfaceflinger/LayerBase.h   |  2 +-
 4 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/services/surfaceflinger/Layer.cpp b/services/surfaceflinger/Layer.cpp
index fde8e677dfcf..0f7d6392e496 100644
--- a/services/surfaceflinger/Layer.cpp
+++ b/services/surfaceflinger/Layer.cpp
@@ -142,7 +142,8 @@ void Layer::onRemoved()
 
 sp<LayerBaseClient::Surface> Layer::createSurface() const
 {
-    return mSurface;
+    sp<Surface> sur(new SurfaceLayer(mFlinger, const_cast<Layer *>(this)));
+    return sur;
 }
 
 status_t Layer::ditch()
@@ -152,9 +153,6 @@ status_t Layer::ditch()
     // the layer is not on screen anymore. free as much resources as possible
     mFreezeLock.clear();
 
-    // Free our own reference to ISurface
-    mSurface.clear();
-
     Mutex::Autolock _l(mLock);
     mWidth = mHeight = 0;
     return NO_ERROR;
@@ -202,7 +200,6 @@ status_t Layer::setBuffers( uint32_t w, uint32_t h,
     int layerRedsize = info.getSize(PixelFormatInfo::INDEX_RED);
     mNeedsDithering = layerRedsize > displayRedSize;
 
-    mSurface = new SurfaceLayer(mFlinger, this);
     return NO_ERROR;
 }
 
diff --git a/services/surfaceflinger/Layer.h b/services/surfaceflinger/Layer.h
index d9a8be334fc4..2b3841466d70 100644
--- a/services/surfaceflinger/Layer.h
+++ b/services/surfaceflinger/Layer.h
@@ -213,7 +213,6 @@ private:
     ClientRef mUserClientRef;
 
     // constants
-    sp<Surface> mSurface;
     PixelFormat mFormat;
     const GLExtensions& mGLExtensions;
     bool mNeedsBlending;
diff --git a/services/surfaceflinger/LayerBase.cpp b/services/surfaceflinger/LayerBase.cpp
index 86057f8cd5ed..6025ed4921cb 100644
--- a/services/surfaceflinger/LayerBase.cpp
+++ b/services/surfaceflinger/LayerBase.cpp
@@ -540,7 +540,9 @@ int32_t LayerBaseClient::sIdentity = 1;
 
 LayerBaseClient::LayerBaseClient(SurfaceFlinger* flinger, DisplayID display,
         const sp<Client>& client)
-    : LayerBase(flinger, display), mClientRef(client),
+    : LayerBase(flinger, display),
+      mHasSurface(false),
+      mClientRef(client),
       mIdentity(uint32_t(android_atomic_inc(&sIdentity)))
 {
 }
@@ -557,12 +559,13 @@ sp<LayerBaseClient::Surface> LayerBaseClient::getSurface()
 {
     sp<Surface> s;
     Mutex::Autolock _l(mLock);
-    s = mClientSurface.promote();
-    if (s == 0) {
-        s = createSurface();
-        mClientSurface = s;
-        mClientSurfaceBinder = s->asBinder();
-    }
+
+    LOG_ALWAYS_FATAL_IF(mHasSurface,
+            "LayerBaseClient::getSurface() has already been called");
+
+    mHasSurface = true;
+    s = createSurface();
+    mClientSurfaceBinder = s->asBinder();
     return s;
 }
 
diff --git a/services/surfaceflinger/LayerBase.h b/services/surfaceflinger/LayerBase.h
index 184edd779cff..bfe92e63ddd6 100644
--- a/services/surfaceflinger/LayerBase.h
+++ b/services/surfaceflinger/LayerBase.h
@@ -337,7 +337,7 @@ protected:
 
 private:
     mutable Mutex mLock;
-    mutable wp<Surface> mClientSurface;
+    mutable bool mHasSurface;
     wp<IBinder> mClientSurfaceBinder;
     const wp<Client> mClientRef;
     // only read
-- 
2.11.0