Re: A simple unit test framework
On May 7, 10:42 pm, Gianni Mariani <gi3nos...@mariani.ws> wrote:
James Kanze wrote:
On May 7, 10:55 am, Gianni Mariani <gi3nos...@mariani.ws> wrote:
...
std::string is not thread safe
Everything in the standard library in g++, from 3.0 on, is
supposed to be thread safe.
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.
... There is some uncertainty, for some
classes, as to how this is defined, and I hesitated to post the
bug, because I wasn't sure that std::string was supposed to meet
the Posix requirements (although all of the other g++ containers
meet them). That is, however, a bit irrelevant to the
discussion here. Posix compliant thread safety is a reasonable
choice, the current implementation of std::string in g++ doesn't
meet it, and I cannot imagine any possible test which would
display this defect in a reliable fashion.
See attached: It finds the "supposed" bug in 24 milliseconds on my machi=
ne.
but this should work:
std::string global1( "A" );
std::string global2( global1 );
void thread1()
{
global1 += "1";
}
void thread2()
{
global2 += "A";
}
That definitly should (and as far as I know, does) work. The
problem is more along the lines of:
std::string global( "a" ) ;
void thread1()
{
std::string s1( global ) ;
}
void thread2()
{
std::string s2( global.begin(), global.end() ) ;
}
Since I'm not modifying global ...
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.)
...
Yes. Better tested code is easier to develop with.
The issue hasn't been raised to date, but...
Good code is easy to understand. Code that isn't easy to
understand fails code review. How does testing verify this?
(In some ways, easy to understand is more important than an
absence of errors.)
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.
What are you smoking ? Sure you can find some obvious bugs in
a code review, but I would have already run the unit tests
before I do the code review.
That's doing things the hard way, and is hardly cost effective.
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.
In code review, you are told that you forgot to initialize
variable i in line 1234 of abc.cc. The unit test tells you that
tests 25 through 30 all fail. Which information makes it easier
to find and fix the error?
And a well run code review doesn't find just the obvious bugs.
It also finds those which no test will find. (I found the bug
in the g++ implementation of std::string by reviewing the code.)
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_prime=
s ) ];
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.
--
James Kanze (Gabi Software) email: james.kanze@gmail.com
Conseils en informatique orient=E9e objet/
Beratung in objektorientierter Datenverarbeitung
9 place S=E9mard, 78210 St.-Cyr-l'=C9cole, France, +33 (0)1 30 23 00 34