[Insight-developers] FMR in RemoveObserver()

[Lydia Ng]

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