Re: Subtle MT problem with __gnu_cxx::hash_map

[Jonathan Wakely <cow at compsoc dot man dot ac dot uk>]

On Mon, Nov 22, 2004 at 05:43:03PM -0500, Paul Dubuc wrote:

> >>>>In our case we were using operator[] to access a hash_map that had
> >>>>been fully populated.  Each index we were using did have an entry in
> >>>>the hash_map.  So we were accessing elements in the map with
> >>>>operator[] using multiple threads thinking that this would be a 
> >>>>thread
> >>>>safe, const operation.  This is implied by the statement 
> >>>>"simultaneous
> >>>>read accesses to to shared containers are safe" in the SGI STL user's
> >>>>guide (http://www.sgi.com/tech/stl/thread_safety.html).
> >>>
> >>>But operator[] isn't read access.  It's defined to be equivalent to a
> >>>certain form of insert().
> >>
> >>If one can be sure that the elements that are accessed using 
> >>operator[] do
> >>exist, I would also tend to assume it should behave like a read only 
> >>access.
> >
> >But it isn't.  Read-only accesses are things you can perform on a const
> >object.
> 
> Not ONLY on a const object.  You can also perform them on non-const 
> objects. Isn't any access that doesn't actually do a write a read-only 
> access?

Would you say ::write(STDOUT_FILENO,"",0) was a read-only file operation ?

> >You can't use operator[] on a const hash_map.  (Or on a const
> >map, for that matter.
> 
> True, but at least the compiler catches it when you do.  operator[] can't 
> be a const method because it MAY do an insert.  That's playing it safe 
> where the interface is concerned.  So?

There's no "playing it safe" - operator[] is not a const operation,
therefore is not a read-only operation, therefore requires explicit
locking. Even if the library was changed as you suggest, other standard
library implementations may not be, so you would be relying on
non-portable, unsafe assumptions.

> >You'll be unhappy if you try to think of 
> >operator[] on maps as a read-only operation in a multithreaded 
> >environment.)
> 
> I'll agree with you here.  You're right.  I guess this is an unfortunate 
> consequence of operator[] being BOTH a find() and an insert().  It's easy 

No, it's an insert, which might not actually insert anything.

> to see the insert() as a convenient side-effect that you can avoid if you 
> know what you're doing.  (Especially since it bears so much resemblance in 
> its use to array subscripting in C which doesn't have this side-effect).  
> After all, the programmer KNOWS he's not doing an insert in this case.  And 

Then he should use find().

> it's true that NOTHING was actually inserted.  It's just that the hash_map 
> implementation decided to reallocate anyway, unnecessarily, based on an 
> arbitrary container size.
> 
> What is wrong with removing the unnecessary and wasteful reallocation and 
> taking the view that, if the programmer knows what he's (not) doing, let 
> him (not) do it without some arbitrary penalty?  Can you think of any a 
> good reason why a container should reallocate itself (or change itself in 
> anyway) if it's not going to insert a new element?  Wouldn't we normally 
> call that a bug?

You've called a mutating member function, so the container is allowed to
modify it's state. If you don't want that, use find().


As an aside, am I right in thinking that the call to resize() that is
under discussion might change the ordering of elements in the container?
c.f. http://www.sgi.com/tech/stl/HashedAssociativeContainer.html#3

That would seem to be another argument against using operator[] when you
want a read-only find operation.

jon


-- 
"Anyone who cannot cope with mathematics is not fully human.
 At best, he is a tolerable subhuman who has learned to dress himself,
 bathe, and not make messes in the house."
	- Robert Heinlein