(Observers)Fix memory issues during observer iteration

[Issue#]   N/A
[Problem]  Crash could occur when adding a previously removed actor back into the scene
[Cause]    Observer classes were using SceneObjectRemoved callback to remove themselves
from that object's observer list while the object is in process of iterating over the list
[Solution] Mark object as disconnected from scene and remove observer later

Change-Id: I552017a0d8284973f565cd08275f1270fc8e9668
Signed-off-by: David Steele <david.steele@partner.samsung.com>

diff --git a/dali/internal/event/events/gesture-processor.cpp b/dali/internal/event/events/gesture-processor.cpp
index 1858db5..f08ef86 100644 (file)
--- a/dali/internal/event/events/gesture-processor.cpp
+++ b/dali/internal/event/events/gesture-processor.cpp
@@ -30,7 +30,8 @@ namespace Internal
 {
 
 GestureProcessor::GestureProcessor()
-: mCurrentGesturedActor( NULL )
+: mCurrentGesturedActor( NULL ),
+  mGesturedActorDisconnected(false)
 {
 }
 
@@ -156,6 +157,7 @@ void GestureProcessor::SetActor( Dali::Actor actor )
     mCurrentGesturedActor = &GetImplementation( actor );
     mCurrentGesturedActor->AddObserver( *this );
   }
+  mGesturedActorDisconnected = false;
 }
 
 void GestureProcessor::ResetActor()
@@ -164,18 +166,25 @@ void GestureProcessor::ResetActor()
   {
     mCurrentGesturedActor->RemoveObserver( *this );
     mCurrentGesturedActor = NULL;
+    mGesturedActorDisconnected = false;
   }
 }
 
+Actor* GestureProcessor::GetCurrentGesturedActor()
+{
+  return mGesturedActorDisconnected ? NULL : mCurrentGesturedActor;
+}
+
 void GestureProcessor::SceneObjectRemoved(ProxyObject& proxy)
 {
-  if ( mCurrentGesturedActor == &proxy )
+  if ( mCurrentGesturedActor == &proxy &&
+      !mGesturedActorDisconnected )
   {
     // Inform deriving classes.
     OnGesturedActorStageDisconnection();
 
-    proxy.RemoveObserver( *this );
-    mCurrentGesturedActor = NULL;
+    // do not call proxy.RemoveObserver here, proxy is currently iterating through observers... you wouldnt want to upset proxy now would you?
+    mGesturedActorDisconnected = true;
   }
 }
 

diff --git a/dali/internal/event/events/gesture-processor.h b/dali/internal/event/events/gesture-processor.h
index 6ad6977..9441542 100644 (file)
--- a/dali/internal/event/events/gesture-processor.h
+++ b/dali/internal/event/events/gesture-processor.h
@@ -146,6 +146,13 @@ protected: // Construction & Destruction
    */
   void ResetActor();
 
+  /**
+   * Returns the current gestured actor if it is on stage
+   *
+   * @return The current gestured actor
+   */
+  Actor* GetCurrentGesturedActor();
+
   // For derived classes to override
 
   /**
@@ -191,9 +198,10 @@ private:
    */
   void OnStageDisconnection( Dali::Actor actor );
 
-protected: // Data
+private: // Data
 
-  Actor* mCurrentGesturedActor; ///< The current actor that has been gestured.
+  Actor* mCurrentGesturedActor;       ///< The current actor that has been gestured.
+  bool   mGesturedActorDisconnected;  ///< Indicates whether the gestured actor has been disconnected from the scene
 };
 
 } // namespace Internal

diff --git a/dali/internal/event/events/long-press-gesture-processor.cpp b/dali/internal/event/events/long-press-gesture-processor.cpp
index 474b454..1a83a67 100644 (file)
--- a/dali/internal/event/events/long-press-gesture-processor.cpp
+++ b/dali/internal/event/events/long-press-gesture-processor.cpp
@@ -180,12 +180,13 @@ void LongPressGestureProcessor::Process( const Integration::LongPressGestureEven
 
     case Gesture::Started:
     {
-      if ( mCurrentGesturedActor )
+      Actor* currentGesturedActor = GetCurrentGesturedActor();
+      if ( currentGesturedActor )
       {
         HitTestAlgorithm::Results hitTestResults;
         HitTestAlgorithm::HitTest( mStage, longPressEvent.point, hitTestResults );
 
-        if ( hitTestResults.actor && ( mCurrentGesturedActor == &GetImplementation( hitTestResults.actor ) ) )
+        if ( hitTestResults.actor && ( currentGesturedActor == &GetImplementation( hitTestResults.actor ) ) )
         {
           // Record the current render-task for Screen->Actor coordinate conversions
           mCurrentRenderTask = hitTestResults.renderTask;
@@ -212,21 +213,22 @@ void LongPressGestureProcessor::Process( const Integration::LongPressGestureEven
       // Only send subsequent long press gesture signals if we processed the gesture when it started.
       // Check if actor is still touchable.
 
-      if ( mCurrentGesturedActor )
+      Actor* currentGesturedActor = GetCurrentGesturedActor();
+      if ( currentGesturedActor )
       {
-        if ( mCurrentGesturedActor->IsHittable() && !mCurrentEmitters.empty() && mCurrentRenderTask )
+        if ( currentGesturedActor->IsHittable() && !mCurrentEmitters.empty() && mCurrentRenderTask )
         {
           // Ensure actor is still attached to the emitters, if it is not then remove the emitter.
-          LongPressGestureDetectorContainer::iterator endIter = std::remove_if( mCurrentEmitters.begin(), mCurrentEmitters.end(), IsNotAttachedFunctor(mCurrentGesturedActor) );
+          LongPressGestureDetectorContainer::iterator endIter = std::remove_if( mCurrentEmitters.begin(), mCurrentEmitters.end(), IsNotAttachedFunctor(currentGesturedActor) );
           mCurrentEmitters.erase( endIter, mCurrentEmitters.end() );
 
           if ( !mCurrentEmitters.empty() )
           {
             Vector2 actorCoords;
             RenderTask& renderTaskImpl( GetImplementation( mCurrentRenderTask ) );
-            mCurrentGesturedActor->ScreenToLocal( renderTaskImpl, actorCoords.x, actorCoords.y, longPressEvent.point.x, longPressEvent.point.y );
+            currentGesturedActor->ScreenToLocal( renderTaskImpl, actorCoords.x, actorCoords.y, longPressEvent.point.x, longPressEvent.point.y );
 
-            EmitLongPressSignal( Dali::Actor( mCurrentGesturedActor ), mCurrentEmitters, longPressEvent, actorCoords );
+            EmitLongPressSignal( Dali::Actor( currentGesturedActor ), mCurrentEmitters, longPressEvent, actorCoords );
           }
         }
 

diff --git a/dali/internal/event/events/pan-gesture-processor.cpp b/dali/internal/event/events/pan-gesture-processor.cpp
index 1644f7b..0d76ea9 100644 (file)
--- a/dali/internal/event/events/pan-gesture-processor.cpp
+++ b/dali/internal/event/events/pan-gesture-processor.cpp
@@ -256,7 +256,7 @@ void PanGestureProcessor::Process( const Integration::PanGestureEvent& panEvent
 
     case Gesture::Started:
     {
-      if ( mCurrentGesturedActor )
+      if ( GetCurrentGesturedActor() )
       {
         // The pan gesture should only be sent to the gesture detector which first received it so that
         // it can be told when the gesture ends as well.
@@ -264,7 +264,7 @@ void PanGestureProcessor::Process( const Integration::PanGestureEvent& panEvent
         HitTestAlgorithm::Results hitTestResults;
         HitTestAlgorithm::HitTest( mStage, mPossiblePanPosition, hitTestResults ); // Hit test original possible position...
 
-        if ( hitTestResults.actor && ( mCurrentGesturedActor == &GetImplementation( hitTestResults.actor ) ) )
+        if ( hitTestResults.actor && ( GetCurrentGesturedActor() == &GetImplementation( hitTestResults.actor ) ) )
         {
           // Record the current render-task for Screen->Actor coordinate conversions
           mCurrentRenderTask = hitTestResults.renderTask;
@@ -290,27 +290,28 @@ void PanGestureProcessor::Process( const Integration::PanGestureEvent& panEvent
       // Only send subsequent pan gesture signals if we processed the pan gesture when it started.
       // Check if actor is still touchable.
 
-      if ( mCurrentGesturedActor )
+      Actor* currentGesturedActor = GetCurrentGesturedActor();
+      if ( currentGesturedActor )
       {
-        if ( mCurrentGesturedActor->IsHittable() && !mCurrentPanEmitters.empty() && mCurrentRenderTask )
+        if ( currentGesturedActor->IsHittable() && !mCurrentPanEmitters.empty() && mCurrentRenderTask )
         {
           PanGestureDetectorContainer outsideTouchesRangeEmitters;
 
           // Removes emitters that no longer have the actor attached
           // Also remove emitters whose touches are outside the range of the current pan event and add them to outsideTouchesRangeEmitters
           PanGestureDetectorContainer::iterator endIter = std::remove_if( mCurrentPanEmitters.begin(), mCurrentPanEmitters.end(),
-                                                                          IsNotAttachedAndOutsideTouchesRangeFunctor(mCurrentGesturedActor, panEvent.numberOfTouches, outsideTouchesRangeEmitters) );
+                                                                          IsNotAttachedAndOutsideTouchesRangeFunctor(currentGesturedActor, panEvent.numberOfTouches, outsideTouchesRangeEmitters) );
           mCurrentPanEmitters.erase( endIter, mCurrentPanEmitters.end() );
 
           Vector2 actorCoords;
 
           if ( !outsideTouchesRangeEmitters.empty() || !mCurrentPanEmitters.empty() )
           {
-            mCurrentGesturedActor->ScreenToLocal( GetImplementation( mCurrentRenderTask ), actorCoords.x, actorCoords.y, panEvent.currentPosition.x, panEvent.currentPosition.y );
+            currentGesturedActor->ScreenToLocal( GetImplementation( mCurrentRenderTask ), actorCoords.x, actorCoords.y, panEvent.currentPosition.x, panEvent.currentPosition.y );
 
             // EmitPanSignal checks whether we have a valid actor and whether the container we are passing in has emitters before it emits the pan.
-            EmitPanSignal(Dali::Actor(mCurrentGesturedActor), outsideTouchesRangeEmitters, panEvent, actorCoords, Gesture::Finished, mCurrentRenderTask);
-            EmitPanSignal(Dali::Actor(mCurrentGesturedActor), mCurrentPanEmitters, panEvent, actorCoords, panEvent.state, mCurrentRenderTask);
+            EmitPanSignal(Dali::Actor(currentGesturedActor), outsideTouchesRangeEmitters, panEvent, actorCoords, Gesture::Finished, mCurrentRenderTask);
+            EmitPanSignal(Dali::Actor(currentGesturedActor), mCurrentPanEmitters, panEvent, actorCoords, panEvent.state, mCurrentRenderTask);
           }
 
           if ( mCurrentPanEmitters.empty() )

diff --git a/dali/internal/event/events/pinch-gesture-processor.cpp b/dali/internal/event/events/pinch-gesture-processor.cpp
index 6dd9809..de3970e 100644 (file)
--- a/dali/internal/event/events/pinch-gesture-processor.cpp
+++ b/dali/internal/event/events/pinch-gesture-processor.cpp
@@ -184,21 +184,22 @@ void PinchGestureProcessor::Process( const Integration::PinchGestureEvent& pinch
       // Only send subsequent pinch gesture signals if we processed the pinch gesture when it started.
       // Check if actor is still touchable.
 
-      if ( mCurrentGesturedActor )
+      Actor* currentGesturedActor = GetCurrentGesturedActor();
+      if ( currentGesturedActor )
       {
-        if ( mCurrentGesturedActor->IsHittable() && !mCurrentPinchEmitters.empty() && mCurrentRenderTask )
+        if ( currentGesturedActor->IsHittable() && !mCurrentPinchEmitters.empty() && mCurrentRenderTask )
         {
           // Ensure actor is still attached to the emitters, if it is not then remove the emitter.
-          PinchGestureDetectorContainer::iterator endIter = std::remove_if( mCurrentPinchEmitters.begin(), mCurrentPinchEmitters.end(), IsNotAttachedFunctor(mCurrentGesturedActor) );
+          PinchGestureDetectorContainer::iterator endIter = std::remove_if( mCurrentPinchEmitters.begin(), mCurrentPinchEmitters.end(), IsNotAttachedFunctor(currentGesturedActor) );
           mCurrentPinchEmitters.erase( endIter, mCurrentPinchEmitters.end() );
 
           if ( !mCurrentPinchEmitters.empty() )
           {
             Vector2 actorCoords;
             RenderTask& renderTaskImpl( GetImplementation(mCurrentRenderTask) );
-            mCurrentGesturedActor->ScreenToLocal( renderTaskImpl, actorCoords.x, actorCoords.y, pinchEvent.centerPoint.x, pinchEvent.centerPoint.y );
+            currentGesturedActor->ScreenToLocal( renderTaskImpl, actorCoords.x, actorCoords.y, pinchEvent.centerPoint.x, pinchEvent.centerPoint.y );
 
-            EmitPinchSignal( Dali::Actor(mCurrentGesturedActor), mCurrentPinchEmitters, pinchEvent, actorCoords );
+            EmitPinchSignal( Dali::Actor(currentGesturedActor), mCurrentPinchEmitters, pinchEvent, actorCoords );
           }
           else
           {

diff --git a/dali/internal/event/events/tap-gesture-processor.cpp b/dali/internal/event/events/tap-gesture-processor.cpp
index 97f9e11..77b83be 100644 (file)
--- a/dali/internal/event/events/tap-gesture-processor.cpp
+++ b/dali/internal/event/events/tap-gesture-processor.cpp
@@ -138,12 +138,12 @@ void TapGestureProcessor::Process( const Integration::TapGestureEvent& tapEvent
 
     case Gesture::Started:
     {
-      if ( mCurrentGesturedActor )
+      if ( GetCurrentGesturedActor() )
       {
         HitTestAlgorithm::Results hitTestResults;
         HitTestAlgorithm::HitTest( mStage, tapEvent.point, hitTestResults );
 
-        if ( hitTestResults.actor && ( mCurrentGesturedActor == &GetImplementation( hitTestResults.actor ) ) )
+        if ( hitTestResults.actor && ( GetCurrentGesturedActor() == &GetImplementation( hitTestResults.actor ) ) )
         {
           TapEventFunctor functor( tapEvent );
           GestureDetectorContainer gestureDetectors;

diff --git a/dali/internal/event/events/touch-event-processor.cpp b/dali/internal/event/events/touch-event-processor.cpp
index eddc888..f54668e 100644 (file)
--- a/dali/internal/event/events/touch-event-processor.cpp
+++ b/dali/internal/event/events/touch-event-processor.cpp
@@ -450,7 +450,8 @@ void TouchEventProcessor::ProcessTouchEvent( const Integration::TouchEvent& even
 }
 
 TouchEventProcessor::ActorObserver::ActorObserver()
-: mActor ( NULL )
+: mActor ( NULL ),
+  mActorDisconnected(false)
 {
   DALI_LOG_TRACE_METHOD( gLogFilter );
 }
@@ -463,7 +464,7 @@ TouchEventProcessor::ActorObserver::~ActorObserver()
 
 Actor* TouchEventProcessor::ActorObserver::GetActor()
 {
-  return mActor;
+  return mActorDisconnected ? NULL : mActor;
 }
 
 void TouchEventProcessor::ActorObserver::SetActor( Actor* actor )
@@ -472,10 +473,7 @@ void TouchEventProcessor::ActorObserver::SetActor( Actor* actor )
 
   if ( mActor != actor )
   {
-    if ( mActor )
-    {
-      SceneObjectRemoved( *mActor );
-    }
+    ResetActor();
 
     mActor = actor;
 
@@ -487,15 +485,25 @@ void TouchEventProcessor::ActorObserver::SetActor( Actor* actor )
   }
 }
 
+void TouchEventProcessor::ActorObserver::ResetActor()
+{
+  if ( mActor )
+  {
+    DALI_LOG_INFO(gLogFilter, Debug::Verbose, "Stop Observing:             %p\n", mActor);
+    mActor->RemoveObserver( *this );
+    mActor = NULL;
+    mActorDisconnected = false;
+  }
+}
+
 void TouchEventProcessor::ActorObserver::SceneObjectRemoved( ProxyObject& proxy )
 {
   DALI_LOG_TRACE_METHOD( gLogFilter );
 
   if ( mActor == &proxy )
   {
-    proxy.RemoveObserver( *this );
-    DALI_LOG_INFO(gLogFilter, Debug::Verbose, "Stop Observing:             %p\n", mActor);
-    mActor = NULL;
+    // do not call proxy.RemoveObserver here, proxy is currently iterating through observers... you wouldnt want to upset proxy now would you?
+    mActorDisconnected = true;
   }
 }
 

diff --git a/dali/internal/event/events/touch-event-processor.h b/dali/internal/event/events/touch-event-processor.h
index a6d9df1..2b73ce1 100644 (file)
--- a/dali/internal/event/events/touch-event-processor.h
+++ b/dali/internal/event/events/touch-event-processor.h
@@ -112,6 +112,11 @@ private:
      */
     void SetActor( Actor* actor );
 
+    /**
+     * Resets the set actor and disconnects any connected signals.
+     */
+    void ResetActor();
+
   private:
 
     // Undefined
@@ -143,7 +148,8 @@ private:
     virtual void ProxyDestroyed(ProxyObject& proxy);
 
   private:
-    Actor* mActor; ///< Raw pointer to an Actor.
+    Actor* mActor;              ///< Raw pointer to an Actor.
+    bool   mActorDisconnected;  ///< Indicates whether the actor has been disconnected from the scene
   };
 
   Stage& mStage; ///< Used to deliver touch events