Re: Subtle MT problem with __gnu_cxx::hash_map

[Paul Dubuc <pdubuc at cas dot org>]

Matt Austern wrote:
>
> On Nov 19, 2004, at 12:38 PM, Lothar Werzinger wrote:
> > On Friday 19 November 2004 12:30, Matt Austern wrote:
> > > On Nov 19, 2004, at 12:21 PM, Paul Dubuc wrote:
> > > > There's a subtle thread safety problem with hash_map that was found 
> > > > in
> > > > our testing recently.  It's understood that operator[] is a non-const
> > > > method since it can insert an element into a container if one is not
> > > > found with the given index.
> > > >
> > > > 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?

> 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?

> 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 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 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?

Instead we've got somone else suggesting that the way to fix the problem is the 
much more complicated solution of making sure the use of operator[] reliably 
fails in an MT environment.  Please don't go through all that trouble!  Let's 
just stick with the letter of the law that says that since operator[] MIGHT do 
an insert, it therefore IS and insert() whether it actually inserts or not. But, 
whether it inserts or not, it always does a find()!

I can live with that.  ;-)   (I would rather live with that.)
--
Paul M. Dubuc