Re: basic_string::release and thread safety: what about

[Joe Buck <jbuck at Synopsys dot COM>]

> >>>>> "Joe" == Joe Buck <jbuck@Synopsys.COM> writes:
> 
>     Joe> So, should basic_string::Rep::grab also use an atomic
>     Joe> instruction to increment grab?  Or is the low-probability
>     Joe> memory leak OK?

Mark Mitchell writes:

> The memory leak is definitely *not* acceptable, provided that we're
> supporting the idea of multiple threads accessing the same Rep.  An
> alternative would be to explicitly require threads to lock objects
> before accessing them.  That is simpler for us, but harder for users.

Actually, as a user I would prefer the status quo with the low-probability
leak than no support for thread safety at all, because without some
support, the string class can't be safely used in multithreaded programs,
unless only one thread ever accesses string objects.

If the leak can't be tolerated.  It seems that the only solution then is
to make the same change for grab as was made for release.  This means code
to do ++rep; in an atomic manner.

The user cannot lock the basic_string::Rep before accessing it, because
this access is hidden (the user believes s/he is not sharing objects
across threads).  All default-initialized strings will share the same
Rep, for example.  The user could ensure that a string does not share
a representation by calling some method on it that invalidates iterators
(e.g. begin()).

The difficulty for the user of the class in doing locks was the argument
that led the SGI people into advocating use of vector<char> for the string
class.

In any case, the atomic instructions are far, far cheaper than general
locks.  For the x86, it seems that the assembly language instruction
required is exactly the same for grab as for release, changing the -1 into
a 1.

For the Sparc code, I'm not sure.

It might be cleaner to write, for all platforms, something like

    charT* grab () { if (selfish) return clone (); 
		     __atomic_increment(ref);
		     return data (); }
    void release () { if (__atomic_decrement(ref) == 0) delete this; }

and then define atomic_increment and atomic_decrement as inline
functions on a processor-specific basis.  These functions could then
be used elsewhere in the library.