J
James Kanze
For some meanings of thread safe.
For the only meaning that makes sense. Do you know of another
meaning? I don't. Thread safe code is code that specifies a
set of guarantees for use in a multithreaded environment, and
conforms to those guarantees. G++ definitly does that (although
there is some question concerning what the actual guarantees are
for std::string).
None of the stl classes as far as I know support simultaneous
modification from multiple threads.
So? Is there any reason for them to?
Formally, of course, if by STL, you mean the standard library,
doesn't support threading at all. But all of the actual
implementations I know of today do, as an extension.
See attached: It finds the "supposed" bug in 24 milliseconds on my machine.
global.begin() is pulling a modifyable pointer - all bets are off.
That's not what Posix says. If the std::string class is
supposed to give the Posix guarantees (which is not clear---but
all of the other STL classes explicitly do), then the above code
is legal.
It's a non-const call so the implementation is free to do
whatever it wants to global.
I don't modify anything, so the implementation is not free to do
whatever it wants.
In my initial implementation of the test I had taken a const reference
to the string (as a default thing I do without thinking) before I called
begin. Then I proceeded to tweak the parameters to trigger the problem,
I has one test run for ten minutes and alas, NO failures. So, I looked
again and I made it non-const and viola immediate failure.
I don't think I would call this one a bug.
It depends on whether G++ wants to support Posix guarantees or
not. As I said, it's not clear. (The next version of the C++
will probably statute one way or the other. I suspect in favor
of requiring this to work, but that suspicion is based on my
knowing the people involved, and not on any specific discussions
or vote.) At any rate, either std::string is in error, or the
code above is in error.
(An interesting point if the code above is in error: you
definitely cannot write a test to detect the error using VC++ or
Sun CC, because the code works with those compilers. Obviously,
testing can't reveal dependencies on the compilers you're
currently using.)
Ya ... I thought we were talking about UNIT TESTING ! Do you like to
digress all the time ?
The claim is that writing the tests before writing the code
improves quality. I'm just showing that this is a specious
claim.
Having the computer do the hard work is far better than for me doing the
hard work. My B.S. meter just pegged again.
The problem is that the computer doesn't do the hard work for
you. It does the easy part, saying that there is an error
somewhere. Afterwards, you have the hard part: finding where.
In a code review, this is automatic.
I don't consider this one a bug. It's expected to fail IMHO.
As I said, it depends on the guarantee. Posix says it should
work. It works with char[]; if the goal of std::string is to
replace char[], it has to work. I suspect that the next version
of the standard will require it to work.
I couldn't get this to compile on my machine; I'm missing some
of the necessary headers. I'll have to download them first. If
I've understood it correctly, the basic idea does seem
interesting. I think it still depends somewhat on the
scheduling algorithm being used by the system, but I'll try to
find time to evaluate it thoroughly. (It obviously depends
somewhat on the scheduling algorithm, because the code is thread
safe if non-preemptive threading is used. But that's not the
default on most modern platforms.)
[...]
// Should this be const or not - I think it should
// James Kanze possibly believes otherwise.
Just a nit: in real code, if I were accessing through a
reference, I would use a const reference unless I planned on
modification (in which case, a lock is required). My problem is
when the code accesses the object directly, without an
intervening reference. (I presume, here, however, that the
reference is an artifact of the test suite, and is used to
simulate accessing the object directly.)
Of course, the question here isn't how the user should write
code, but whether the specific construction should be guaranteed
to work or not. Posix bases its guarantees on whether
modification takes place, not on const-ness, which is the basis
of my argument.
[...]
enum { s_count = 43 }; // prime number
Just curious: why is it important that s_count be prime?
static const unsigned s_length = 3; // tweak for maximum effect
std::string m_strings[ s_count ];
virtual void test2( int l_val, std::string & o_tval )
{
t_local_type & l_str = m_strings[ l_val ];
t_iter_type beg = l_str.begin();
// at::OSTraitsBase::SchedulerYield();
o_tval.assign( beg, l_str.end() );
Just curious: is there some reason behind using o_tval.assign,
and not simply:
std::string localCopy( l_str.begin(), l_str.end() ) ;
followed by the AT_TCAssert?
[...]
class HardTask
: public String_TaskBase< 2 >
{
public:
static at::AtomicCount s_task_counter;
static int s_xcount;
// each test thread calls this with a unique thread number
virtual void TestWork( int l_thrnum )
{
unsigned pnum = g_primes[ l_thrnum % at::CountElements( g_primes ) ];
unsigned count = l_thrnum;
unsigned done = 0;
std::string l_teststr;
for ( int i = 0; i < m_iters; ++i )
{
int choice = ( i * pnum ) % g_test->s_count;
switch ( l_thrnum & 1 ? i % 8 : (7 - (i % 8) ) )
{
case 0 : case 2 :
g_test->test1( choice, l_teststr );
break;
case 1 : case 3 :
g_test->test2( choice, l_teststr );
break;
case 4 :
g_test->test3( choice, l_teststr );
break;
case 5 :
g_test->test4( choice, l_teststr );
break;
}
// after every 1<<5 iterations - rebuild the test object
// with brand new strings
if ( true && !( i % (1<<5) ) )
{
// stuff is done here
at::Lock<at::ConditionalMutex> l_lock( m_mutex );
int l_num = ++ s_xcount;
if ( m_thr_count == l_num )
{
s_xcount = 0;
g_test = new STDStringTest();
l_lock.PostAll();
}
else
{
l_lock.Wait();
}
}
I'm not sure I understand this. How is it synchronized with the
previous loop? If some of the threads are still in the previous
loop when a thread gets here, then it is undefined behavior.
Even using const_iterators.
I've saved a copy of your code locally, and will try to find
time to download your test suite in order to evaluate it on my
machines. If you really can trigger threading errors with any
degree of reliability, I'm very interested.