[Insight-developers] FMR in RemoveObserver()

[William A. Hoffman]

I think it is just the order that is wrong:

  if((*i)->m_Tag == tag)
       {
       delete (*i);  // this should be done after the remove
       m_Observers.remove(*i);
       return;  // this return will stop all iteration on the list
}

So, the only thing that needs to be done, is to move the delete one line down.
Remove only removes the first element in the list.


At 01:49 PM 7/23/2001 -0700, Lydia Ng wrote:
>Hi,
>
>I am getting free memory read (FMR) errors in my app
>due to RemoveObserver():
>
>void
>SubjectImplementation::
>RemoveObserver(unsigned long tag)
>{
>   for(std::list<Observer* >::iterator i = m_Observers.begin();
>       i != m_Observers.end(); ++i)
>     {
>     if((*i)->m_Tag == tag)
>       {
>       delete (*i);
>       m_Observers.remove(*i);
>       return;
>       }
>     }
>}
>
>The problem seems to be with using an iterator as the argument to
>list::remove( const T & ).
>
>remove() tries to remove *all* occurences of the element in the list.
>After it removes the element associated with the iterator, the iterator
>becomes invalid. But the function still needs to do comparisions on the
>rest of the list and it tries to dereference the invalid iterator.
>This results in FMR errors (at least in the VC++ 6.0 sp 5 implementation
>anyway) and the possibility that all occurences are not not
>been removed.
>
>See also MSDN note on the subject:
>http://support.microsoft.com/support/kb/articles/Q250/8/48.ASP
>
>
>There are two solutions
>
>(1) make a copy of the element and pass that to remove:
>
>Observer * observerPtr = *i;
>m_Observers.remove( observerPtr );
>
>(2) Since all the observers should be uniquely
>identified by m_Tag we could use erase( iterator )
>which only removes the one occurence specified
>by an iterator:
>
>m_Observers.erase( i );
>
>-----------------------------------
>I prefer (2) since erase is O(1) and it also doesn't
>require the extra copying.
>
>Any objections to the change?
>
>Cheers,
>Lydia
>
>_______________________________________________
>Insight-developers mailing list
>Insight-developers@public.kitware.com
>http://public.kitware.com/mailman/listinfo/insight-developers