Fix memory leaks detected by Valgrind 15/117015/3
authorNick Holland <nick.holland@partner.samsung.com>
Tue, 7 Mar 2017 16:03:00 +0000 (16:03 +0000)
committerNick Holland <nick.holland@partner.samsung.com>
Tue, 7 Mar 2017 16:10:02 +0000 (16:10 +0000)
Fixes the following memory leaks reported by Valgrind
that can occur when messages with raw pointers are sent
from event -> update (and update -> render)
and dali shutdowns before update or render has had chance
to process those messages

- AddSampler ( sampler is lost )
- AddPropertyBuffer ( property buffer is lost )
- SetPropertyBufferFormat( property buffer format is lost )
- AddGeometry ( geometry data  is lost )
- AddTexture ( texture data is lost )

event -> update
- AddCameraMessage ( Camera is lost )
- AddObjectMessage ( Object is lost )
- AddPropertyNotificationMessage ( PropertyNotification is lost )
- AddGestureMessage ( PanGesture is lost )
- AddSamplerMessage( sampler is lost )
- AddPropertyBuffer( property buffer is lost )
- SetPropertyBufferFormat( property buffer format is lost )
- SetPropertyBufferData( property data is lost )
- AddGeometry( geometry is lost)

Also removes PropertyBuffer data and format from being duplicated
on the event side.

Change-Id: Iac6d446bd2b5372cea7ec101e2e289ef3f284b4c

dali/internal/event/common/property-buffer-impl.cpp
dali/internal/event/common/property-buffer-impl.h
dali/internal/update/controllers/render-message-dispatcher.cpp
dali/internal/update/manager/update-manager.cpp
dali/internal/update/manager/update-manager.h
dali/internal/update/render-tasks/scene-graph-render-task-list.h

index 9c3c438..d281d84 100644 (file)
@@ -132,21 +132,22 @@ PropertyBufferPtr PropertyBuffer::New( Dali::Property::Map& format )
 
 void PropertyBuffer::SetData( const void* data, std::size_t size )
 {
-  DALI_ASSERT_DEBUG( mFormat.Count() && "Format must be set before setting the data." );
+  mSize = size; // size is the number of elements
 
-  mSize = size;
+  unsigned int bufferSize = mBufferFormatSize * mSize;
 
-  // Check if format and size have been set yet
-  if( mBufferFormat != NULL )
-  {
-    unsigned int bufferSize = mBufferFormat->size * mSize;
-    mBuffer.Resize( bufferSize );
-  }
+  // create a new DALi vector to store the buffer data
+  // the heap allocated vector will end up being owned by Render::PropertyBuffer
+  Dali::Vector<char>* bufferCopy = new Dali::Vector<char>();
+  bufferCopy->Resize( bufferSize );
 
+  // copy the data
   const char* source = static_cast<const char*>( data );
-  std::copy( source, source + mBuffer.Size(), &mBuffer[0] );
+  char *destination = &((*bufferCopy)[0]);
+  std::copy( source, source + bufferSize, destination );
 
-  SceneGraph::SetPropertyBufferData( mEventThreadServices.GetUpdateManager(), *mRenderObject, new Dali::Vector<char>( mBuffer ), mSize );
+  // Ownership of the bufferCopy is passed to the message ( uses an owner pointer )
+  SceneGraph::SetPropertyBufferData( mEventThreadServices.GetUpdateManager(), *mRenderObject, bufferCopy, mSize );
 }
 
 std::size_t PropertyBuffer::GetSize() const
@@ -170,7 +171,7 @@ PropertyBuffer::~PropertyBuffer()
 PropertyBuffer::PropertyBuffer()
 : mEventThreadServices( *Stage::GetCurrent() ),
   mRenderObject( NULL ),
-  mBufferFormat( NULL ),
+  mBufferFormatSize( 0 ),
   mSize( 0 )
 {
 }
@@ -180,12 +181,9 @@ void PropertyBuffer::Initialize( Dali::Property::Map& formatMap )
   mRenderObject = new Render::PropertyBuffer();
   SceneGraph::AddPropertyBuffer(mEventThreadServices.GetUpdateManager(), *mRenderObject );
 
-  mFormat = formatMap;
-
-  size_t numComponents = mFormat.Count();
+  size_t numComponents = formatMap.Count();
 
   // Create the format
-  DALI_ASSERT_DEBUG( mBufferFormat == NULL && "PropertyFormat should not be set yet" );
   Render::PropertyBuffer::Format* format = new Render::PropertyBuffer::Format();
   format->components.resize( numComponents );
 
@@ -194,7 +192,7 @@ void PropertyBuffer::Initialize( Dali::Property::Map& formatMap )
 
   for( size_t i = 0u; i < numComponents; ++i )
   {
-    KeyValuePair component = mFormat.GetKeyValue( i );
+    KeyValuePair component = formatMap.GetKeyValue( i );
 
     // Get the name
     if(component.first.type == Property::Key::INDEX)
@@ -253,7 +251,7 @@ void PropertyBuffer::Initialize( Dali::Property::Map& formatMap )
   // Set the format size
   format->size = currentAlignment;
 
-  mBufferFormat = format;
+  mBufferFormatSize = format->size;
 
   SceneGraph::SetPropertyBufferFormat(mEventThreadServices.GetUpdateManager(), *mRenderObject, format );
 }
index 8631f34..aeac543 100644 (file)
@@ -89,12 +89,9 @@ private: // unimplemented methods
 
 private: // data
   EventThreadServices& mEventThreadServices;    ///<Used to send messages to the render thread via update thread
-  Render::PropertyBuffer* mRenderObject; ///<Render side object
-
-  Property::Map mFormat;  ///< Format of the property buffer
-  const Render::PropertyBuffer::Format* mBufferFormat;  ///< Metadata for the format of the property buffer
+  Render::PropertyBuffer* mRenderObject;        ///<Render side object
+  unsigned int mBufferFormatSize;
   unsigned int mSize; ///< Number of elements in the buffer
-  Dali::Vector< char > mBuffer; // Data of the property-buffer
 };
 
 /**
index 1e146d0..cf5937f 100644 (file)
@@ -21,6 +21,7 @@
 // INTERNAL INCLUDES
 #include <dali/internal/render/common/render-manager.h>
 #include <dali/internal/render/queue/render-queue.h>
+#include <dali/internal/render/renderers/render-renderer.h>
 #include <dali/internal/common/message.h>
 
 namespace Dali
@@ -45,7 +46,8 @@ RenderMessageDispatcher::~RenderMessageDispatcher()
 
 void RenderMessageDispatcher::AddRenderer( Render::Renderer& renderer )
 {
-  typedef MessageValue1< RenderManager, Render::Renderer* > DerivedType;
+  // Message has ownership of renderer while in transit from update -> render
+  typedef MessageValue1< RenderManager, OwnerPointer< Render::Renderer > > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mRenderQueue.ReserveMessageSlot( mBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
index 9e7a6fd..8cd9021 100644 (file)
@@ -1107,7 +1107,8 @@ void UpdateManager::SetShaderSaver( ShaderSaver& upstream )
 
 void UpdateManager::AddSampler( Render::Sampler* sampler )
 {
-  typedef MessageValue1< RenderManager, Render::Sampler* > DerivedType;
+  // Message has ownership of Sampler while in transit from update to render
+  typedef MessageValue1< RenderManager, OwnerPointer< Render::Sampler > > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mImpl->renderQueue.ReserveMessageSlot( mSceneGraphBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
@@ -1151,7 +1152,8 @@ void UpdateManager::SetWrapMode( Render::Sampler* sampler, unsigned int rWrapMod
 
 void UpdateManager::AddPropertyBuffer( Render::PropertyBuffer* propertyBuffer )
 {
-  typedef MessageValue1< RenderManager, Render::PropertyBuffer* > DerivedType;
+  // Message has ownership of format while in transit from update -> render
+  typedef MessageValue1< RenderManager, OwnerPointer< Render::PropertyBuffer > > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mImpl->renderQueue.ReserveMessageSlot( mSceneGraphBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
@@ -1173,7 +1175,8 @@ void UpdateManager::RemovePropertyBuffer( Render::PropertyBuffer* propertyBuffer
 
 void UpdateManager::SetPropertyBufferFormat(Render::PropertyBuffer* propertyBuffer, Render::PropertyBuffer::Format* format )
 {
-  typedef MessageValue2< RenderManager, Render::PropertyBuffer*, Render::PropertyBuffer::Format* > DerivedType;
+  // Message has ownership of format while in transit from update -> render
+  typedef MessageValue2< RenderManager, Render::PropertyBuffer*, OwnerPointer< Render::PropertyBuffer::Format > > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mImpl->renderQueue.ReserveMessageSlot( mSceneGraphBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
@@ -1184,7 +1187,8 @@ void UpdateManager::SetPropertyBufferFormat(Render::PropertyBuffer* propertyBuff
 
 void UpdateManager::SetPropertyBufferData( Render::PropertyBuffer* propertyBuffer, Dali::Vector<char>* data, size_t size )
 {
-  typedef MessageValue3< RenderManager, Render::PropertyBuffer*, Dali::Vector<char>*, size_t > DerivedType;
+  // Message has ownership of format while in transit from update -> render
+  typedef MessageValue3< RenderManager, Render::PropertyBuffer*, OwnerPointer< Dali::Vector<char> >, size_t > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mImpl->renderQueue.ReserveMessageSlot( mSceneGraphBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
@@ -1195,7 +1199,8 @@ void UpdateManager::SetPropertyBufferData( Render::PropertyBuffer* propertyBuffe
 
 void UpdateManager::AddGeometry( Render::Geometry* geometry )
 {
-  typedef MessageValue1< RenderManager, Render::Geometry* > DerivedType;
+  // Message has ownership of format while in transit from update -> render
+  typedef MessageValue1< RenderManager, OwnerPointer< Render::Geometry > > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mImpl->renderQueue.ReserveMessageSlot( mSceneGraphBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
@@ -1261,7 +1266,8 @@ void UpdateManager::AddVertexBuffer( Render::Geometry* geometry, Render::Propert
 
 void UpdateManager::AddTexture( Render::Texture* texture )
 {
-  typedef MessageValue1< RenderManager, Render::Texture* > DerivedType;
+  // Message has ownership of Texture while in transit from update -> render
+  typedef MessageValue1< RenderManager, OwnerPointer< Render::Texture > > DerivedType;
 
   // Reserve some memory inside the render queue
   unsigned int* slot = mImpl->renderQueue.ReserveMessageSlot( mSceneGraphBuffers.GetUpdateBufferIndex(), sizeof( DerivedType ) );
index 9b51224..022a08d 100644 (file)
@@ -35,6 +35,8 @@
 #include <dali/internal/update/nodes/node.h>
 #include <dali/internal/update/nodes/scene-graph-layer.h>
 #include <dali/internal/update/rendering/scene-graph-renderer.h>
+#include <dali/internal/update/gestures/scene-graph-pan-gesture.h>
+#include <dali/internal/update/render-tasks/scene-graph-camera.h>
 #include <dali/internal/render/shaders/scene-graph-shader.h>
 #include <dali/internal/render/renderers/render-property-buffer.h>
 #include <dali/internal/event/rendering/texture-impl.h>
@@ -71,14 +73,12 @@ namespace SceneGraph
 
 class Animation;
 class DiscardQueue;
-class PanGesture;
 class RenderManager;
 class RenderTaskList;
 class RenderTaskProcessor;
 class RenderQueue;
 class PropertyBuffer;
 class TextureSet;
-class Camera;
 
 /**
  * UpdateManager maintains a scene graph i.e. a tree of nodes as well as
@@ -721,7 +721,8 @@ inline void DestroyNodeMessage( UpdateManager& manager, const Node& constNode )
 
 inline void AddCameraMessage( UpdateManager& manager, const Camera* constCamera )
 {
-  typedef MessageValue1< UpdateManager, Camera* > LocalType;
+  // Message has ownership of Camera while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< Camera > > LocalType;
 
   Camera* camera = const_cast<Camera*>( constCamera );
   // Reserve some memory inside the message queue
@@ -744,6 +745,7 @@ inline void RemoveCameraMessage( UpdateManager& manager, const Camera* camera )
 
 inline void AddObjectMessage( UpdateManager& manager, PropertyOwner* object )
 {
+  // Message has ownership of object while in transit from event -> update
   typedef MessageValue1< UpdateManager, OwnerPointer<PropertyOwner> > LocalType;
 
   // Reserve some memory inside the message queue
@@ -805,7 +807,8 @@ inline void RemoveAnimationMessage( UpdateManager& manager, const Animation& con
 
 inline void AddPropertyNotificationMessage( UpdateManager& manager, PropertyNotification* propertyNotification )
 {
-  typedef MessageValue1< UpdateManager, PropertyNotification* > LocalType;
+  // Message has ownership of PropertyNotification while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< PropertyNotification > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -934,7 +937,8 @@ inline void SetLayerDepthsMessage( UpdateManager& manager, const std::vector< La
 
 inline void AddGestureMessage( UpdateManager& manager, PanGesture* gesture )
 {
-  typedef MessageValue1< UpdateManager, PanGesture* > LocalType;
+  // Message has ownership of PanGesture while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< PanGesture > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -1002,7 +1006,8 @@ inline void RemoveTextureSetMessage( UpdateManager& manager, TextureSet& texture
 
 inline void AddSamplerMessage( UpdateManager& manager, Render::Sampler& sampler )
 {
-  typedef MessageValue1< UpdateManager, Render::Sampler* > LocalType;
+  // Message has ownership of Sampler while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< Render::Sampler > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -1046,7 +1051,8 @@ inline void SetWrapModeMessage( UpdateManager& manager, Render::Sampler& sampler
 
 inline void AddPropertyBuffer( UpdateManager& manager, Render::PropertyBuffer& propertyBuffer )
 {
-  typedef MessageValue1< UpdateManager, Render::PropertyBuffer*  > LocalType;
+  // Message has ownership of propertyBuffer while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< Render::PropertyBuffer > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -1068,7 +1074,8 @@ inline void RemovePropertyBuffer( UpdateManager& manager, Render::PropertyBuffer
 
 inline void SetPropertyBufferFormat( UpdateManager& manager, Render::PropertyBuffer& propertyBuffer, Render::PropertyBuffer::Format* format )
 {
-  typedef MessageValue2< UpdateManager, Render::PropertyBuffer*, Render::PropertyBuffer::Format*  > LocalType;
+  // Message has ownership of PropertyBuffer::Format while in transit from event -> update
+  typedef MessageValue2< UpdateManager, Render::PropertyBuffer*, OwnerPointer< Render::PropertyBuffer::Format> > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -1079,7 +1086,8 @@ inline void SetPropertyBufferFormat( UpdateManager& manager, Render::PropertyBuf
 
 inline void SetPropertyBufferData( UpdateManager& manager, Render::PropertyBuffer& propertyBuffer, Vector<char>* data, size_t size )
 {
-  typedef MessageValue3< UpdateManager, Render::PropertyBuffer*, Vector<char>*, size_t  > LocalType;
+  // Message has ownership of PropertyBuffer data while in transit from event -> update
+  typedef MessageValue3< UpdateManager, Render::PropertyBuffer*, OwnerPointer< Vector<char> >, size_t  > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -1090,7 +1098,8 @@ inline void SetPropertyBufferData( UpdateManager& manager, Render::PropertyBuffe
 
 inline void AddGeometry( UpdateManager& manager, Render::Geometry& geometry )
 {
-  typedef MessageValue1< UpdateManager, Render::Geometry*  > LocalType;
+  // Message has ownership of Geometry while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< Render::Geometry > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
@@ -1196,7 +1205,8 @@ inline void SetGeometryTypeMessage( UpdateManager& manager, Render::Geometry& ge
 
 inline void AddTexture( UpdateManager& manager, Render::Texture& texture )
 {
-  typedef MessageValue1< UpdateManager, Render::Texture*  > LocalType;
+  // Message has ownership of Texture while in transit from event -> update
+  typedef MessageValue1< UpdateManager, OwnerPointer< Render::Texture > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = manager.ReserveMessageSlot( sizeof( LocalType ) );
index 90b84cf..50542ed 100644 (file)
@@ -111,7 +111,8 @@ private:
 
 inline void AddTaskMessage( EventThreadServices& eventThreadServices, RenderTaskList& list, RenderTask& task )
 {
-  typedef MessageValue1< RenderTaskList, RenderTask* > LocalType;
+  // Message has ownership of the RenderTask while in transit from event -> update
+  typedef MessageValue1< RenderTaskList, OwnerPointer< RenderTask > > LocalType;
 
   // Reserve some memory inside the message queue
   unsigned int* slot = eventThreadServices.ReserveMessageSlot( sizeof( LocalType ) );