From: Nick Holland <nick.holland@partner.samsung.com>
Date: Tue, 28 Mar 2017 10:41:04 +0000 (+0100)
Subject: Valgrind detected TextureSet leak and invalid access
X-Git-Tag: dali_1.2.33~2^2
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=5ccda65a296046076deabf99b944f41eba28830a;p=platform%2Fcore%2Fuifw%2Fdali-core.git

Valgrind detected TextureSet leak and invalid access

Memory leak:

If a public API TextureSet gets deleted, it sends a
message to update-manager (RemoveTextureSet).
The scene graph side TextureSet gets removed from
update manager, but it never gets deleted.

Invalid memory access:

Currently when a scene graph TextureSet gets deleted
the renderers using that TextureSet are not notified.

So when a Render gets deleted it would call
TextureSet->RemoveObserver( this );

on an already deleted TextureSet.

DALi however doesn't crash at this point,
because the TextureSet is stored in a memory
 pool so the object is still reachable.

Change-Id: Icf0f5a3e55d3ba7537f40db08accad04ae4440f7
---

diff --git a/dali/internal/update/manager/update-manager.cpp b/dali/internal/update/manager/update-manager.cpp
index 8cd9021..dae7194 100644
--- a/dali/internal/update/manager/update-manager.cpp
+++ b/dali/internal/update/manager/update-manager.cpp
@@ -631,6 +631,9 @@ void UpdateManager::RemoveTextureSet( TextureSet* textureSet )
     if( textureSet == mImpl->textureSets[i] )
     {
       mImpl->textureSets.Remove( mImpl->textureSets.Begin() + i );
+
+      // Update manager has ownership of the TextureSet
+      delete textureSet;
       return;
     }
   }
diff --git a/dali/internal/update/rendering/scene-graph-renderer.cpp b/dali/internal/update/rendering/scene-graph-renderer.cpp
index af7d93e..732ca23 100644
--- a/dali/internal/update/rendering/scene-graph-renderer.cpp
+++ b/dali/internal/update/rendering/scene-graph-renderer.cpp
@@ -656,6 +656,12 @@ void Renderer::TextureSetChanged()
   mResendFlag |= RESEND_DATA_PROVIDER;
 }
 
+void Renderer::TextureSetDeleted()
+{
+  mTextureSet = NULL;
+
+  mResendFlag |= RESEND_DATA_PROVIDER;
+}
 void Renderer::ConnectionsChanged( PropertyOwner& object )
 {
   // One of our child objects has changed it's connections. Ensure the uniform
@@ -679,11 +685,7 @@ void Renderer::UniformMappingsChanged( const UniformMap& mappings )
 
 void Renderer::ObservedObjectDestroyed(PropertyOwner& owner)
 {
-  if( reinterpret_cast<PropertyOwner*>(mTextureSet) == &owner )
-  {
-    mTextureSet = NULL;
-  }
-  else if( reinterpret_cast<PropertyOwner*>(mShader) == &owner )
+  if( reinterpret_cast<PropertyOwner*>(mShader) == &owner )
   {
     mShader = NULL;
   }
diff --git a/dali/internal/update/rendering/scene-graph-renderer.h b/dali/internal/update/rendering/scene-graph-renderer.h
index aa530e1..ea1308b 100644
--- a/dali/internal/update/rendering/scene-graph-renderer.h
+++ b/dali/internal/update/rendering/scene-graph-renderer.h
@@ -277,6 +277,11 @@ public:
    */
   void TextureSetChanged();
 
+  /**
+   * Called by the TextureSet to notify to the renderer that it is about to be deleted
+   */
+  void TextureSetDeleted();
+
 public: // Implementation of ObjectOwnerContainer template methods
   /**
    * Connect the object to the scene graph
diff --git a/dali/internal/update/rendering/scene-graph-texture-set.cpp b/dali/internal/update/rendering/scene-graph-texture-set.cpp
index 65aa913..a5e7df8 100644
--- a/dali/internal/update/rendering/scene-graph-texture-set.cpp
+++ b/dali/internal/update/rendering/scene-graph-texture-set.cpp
@@ -51,6 +51,11 @@ TextureSet::TextureSet()
 
 TextureSet::~TextureSet()
 {
+  size_t rendererCount = mRenderers.Size();
+  for( size_t i(0); i<rendererCount; ++i )
+  {
+    mRenderers[i]->TextureSetDeleted();
+  }
 }
 
 void TextureSet::operator delete( void* ptr )