Catch illegal hash table modifications when they happen.

Catch illegal hash table modifications when they happen.

Not for landing, this approach won't work out -- iterator
stack lifetimes aren't accurate wrt the period we want
hash tables not to be modifiable.

Iterator lifetime is accurate for something like

  for (auto it : m_observers) {
      ..cannot modify m_observers here..
  }

but not for

  auto it = m_observers.find(o);
  if (it != end())
      return;
  m_observers.add(o);

[sof, 2016-04-15 21:40:22]

Description was changed from

==========
Catch illegal hash table modifications when they happen.

R=
BUG=
==========

to

==========
Catch illegal hash table modifications when they happen.

Not for landing, this approach won't work out -- iterator
stack lifetimes aren't accurate wrt the period we want
hash tables not to be modifiable.

Iterator lifetime is accurate for something like

  for (auto it : m_observers) {
      ..cannot modify m_observers here..
  }

but not for

  auto it = m_observers.find(o);
  if (it != end())
      return;
  m_observers.add(o);
==========

[haraken, 2016-04-18 00:05:29]

Thanks for digging into this.

>  auto it = m_observers.find(o);
>  if (it != end())
>      return;
>  m_observers.add(o);

Another verification idea would be to invalidate the iterator when there is any
modification on the hash table, so that a subsequent access on the iterator will
just crash?

[sof, 2016-04-18 07:29:08]

On 2016/04/18 00:05:29, haraken wrote:
> Thanks for digging into this.
> 
> >  auto it = m_observers.find(o);
> >  if (it != end())
> >      return;
> >  m_observers.add(o);
> 
> Another verification idea would be to invalidate the iterator when there is
any
> modification on the hash table, so that a subsequent access on the iterator
will
> just crash?

checkModifications() is called on advancing the iterator and access (get()), so
the mechanism is in place.

Using release asserts for these will require growing the size of HashTable (and
iterators) to hold a modification counter; that is the trade-off here.