[Insight-developers] FMR in RemoveObserver()

[Bill Hoffman]

I take it back again, erase should be used....


At 11:04 PM 7/23/2001 -0400, William A. Hoffman wrote:
>I guess it should
>be :
>Observer* o = *i;
>m_Observers.remove(*i);
>delete o;
>
>
>At 10:26 PM 7/23/2001 -0400, William A. Hoffman wrote:
>>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
>>
>>
>>_______________________________________________
>>Insight-developers mailing list
>>Insight-developers@public.kitware.com
>>http://public.kitware.com/mailman/listinfo/insight-developers
>
>
>_______________________________________________
>Insight-developers mailing list
>Insight-developers@public.kitware.com
>http://public.kitware.com/mailman/listinfo/insight-developers