A simple unit test framework

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.
 
J

James Kanze

........
I don't consider this one a bug. It's expected to fail IMHO.
[/QUOTE]
All the time I want to point out that code with undefined
behavior is expected to do anything. This is first what I have
learned on this newsgroup.

That's actually an important point. Gianni's first example:

void thread1()
{
globalString += 'A' ;
}

void thread2()
{
globalString += 'B' ;
}

entails undefined behavior with STLport, g++ and (I think) VC++,
but is defined using Sun CC (with the Rogue Wave library).
Logically, I would expect it to be undefined behavior, and
consider the behavior of Sun CC an extension (defining undefined
behavior)---I'm fairly sure that this will be the situation in
the next version of the standard.

Now, suppose that you are developping on Sun CC, but the
requirements are that your code be portable. How do you test
this?
Seems that your test code also employs undefined behavior:

I'm not sure myself. I don't know enough about his framework to
say for sure. But there are supicious points.
[tst_string.cpp]/**
* Test std::string.
*
*/
.........
template <int N, int ThreadCount = 3 >
class String_TaskBase
: public at::Task
{
..........
void Work();
};
...........
class HardTask
: public String_TaskBase< 2 >
{
}
..........

These two are sure sign of possible undefined behavior.
I can bet that Task's start function, passes context
of object into other thread while construction still works
in first one.

I think that HardTask is the most derived class. While
formally, it is true that you cannot use a pointer to the class
until having finished construction, in practice, it's safe to
say that once all subobjects have been constructed and
initialized, as the last thing in the constructor of the most
derived class, it should be safe. (Provided, of course, that
Start() ensures proper synchronization everywhere.)
Since call to member function is made,
no wonder that you can't do this in constructor/destructor
of base class.

Did I miss something? I didn't see any derivation from HardTask.
 
J

James Kanze

Branimir Maksimovic wrote:
Yeah - I know - technically speaking you're right - practically speaking
it's not an issue. It's platform specific for any platform that works.
So far that UB is working as expected on every platform I have tested
so I have not bothered to fix it.

Supposing, of course, that HardTask is the most derived class.
(which I'm pretty sure is the case).
For which definition of reliability ? That code is reliable at least on
the platforms it compiles on - at the moment. Also, I am not alone with
this one and so it is becoming a de-fact standard anyway.

There is an impedence mismatch here with regards to what can be
specified clearly in the standard, and what actually works.
Practically speaking, provided you have ensured proper
synchronization in Start, etc., an implementation where your
specific code won't work is impossible. Try writing a
specification, however, that defines its behavior, but doesn't
allow cases where you don't actually initialize something until
after the call to Start.
The *right* way to use task is to call Start in the most derived
constructor ... and Wait() in the most derived destructor. Anything
else is UB - i.e. not supported.

And of course, Start() must be *after* all essential
initializations in the constructor, and Wait() before you start
tearing things down in the destructor.
 
J

James Kanze

Say Fred and Jim work on a story for a couple of hours, then Fred moves
off and Bill joins Fred to carry on, doesn't Bill have a fresh view of
Fred and Jim's code?

Certainly, for that part which is already written. Does Bill
carefully reread all of the code that was already written, to
ensure that it is correct and conform to the local coding
standards? Is there a mechanism which ensures that yet someone
else intervenes once Bill and Fred have written something?

In practice, well run code reviews are necessary, and once they
are taking place, adding pair programming adds very, very little
value, while effectively doubling the cost. In general---I can
imagine that in some specific circumstances, it is worth while.
 
J

James Kanze

Not a bug in my opinion. See my other post for a unit test case that
finds the supposed bug in less than 25 milliseconds.

I'll try it. (I'm not 100% sure that there isn't a bug in your
code, however.)
They are surprisingly easy to find too.

Really. Consider the following implementation of DCL:

pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER ;
pthread_mutex_t mutex2 = PTHREAD_MUTEX_INITIALIZER ;
Singleton* theOneAndOnly = NULL ;

static Singleton*
Singleton::instance()
{
if ( theOneAndOnly == NULL ) {
Locker lock( mutex1 ) ;
if ( theOneAndOnly == NULL ) {
Singleton* tmp ;
{
Locker lock2( mutex2 ) ;
tmp = new Singleton ;
}
theOneAndOnly = tmp ;
}
}
return theOneAndOnly ;
}

It doesn't work, but can you write a test which will reveal the
error? (For starters, you certainly cannot on most Intel 32
bits processors, because they do guarantee that it works, even
if it violates Posix, and fails in practice on some other
architectures.)
 
B

Branimir Maksimovic

[tst_string.cpp]/**
* Test std::string.
*
*/ .........
template <int N, int ThreadCount = 3 >
class String_TaskBase
: public at::Task
{ ..........
void Work();
};
...........
class HardTask
: public String_TaskBase< 2 >
{
} ..........
........
~HardTask()
{
Wait();
}
};
These two are sure sign of possible undefined behavior.
I can bet that Task's start function, passes context
of object into other thread while construction still works
in first one.

I think that HardTask is the most derived class. While
formally, it is true that you cannot use a pointer to the class
until having finished construction, in practice, it's safe to
say that once all subobjects have been constructed and
initialized, as the last thing in the constructor of the most
derived class, it should be safe. (Provided, of course, that
Start() ensures proper synchronization everywhere.)

Yes. But I have to comment that proper practice is not
to share objects between threads that are not yet constructed
and not to destruct them while other threads are still using them.
This code violates both rules.
Did I miss something? I didn't see any derivation from HardTask.

Obviously, I can't express myself in English good enough.
I meant if calls to Start and Wait are moved to base class Task
constructor/destructor,then, not necessarily, but probably,
code will fail.

Greetings, Branimir.
 
J

James Kanze

James Kanze wrote:
This overlooks another powerful feature of TDD - algorithm discovery.
You may not know how to solve the problem when you write the first test.
As each successive test adds functionality to the unit under test, the
algorithm will find its self. Only then do you know what the
*required* branches are.

But suppose you never add a test which triggers a required
branch? You'll admit that it isn't reasonable to test every
possible double value. So how do you choose the test values in
such a way as to ensure that they trigger all of the required
branches? And that they trigger them at the right place?

Take a simple example: a function that requires special handling
for very small values. Obviously, you'll end up testing some
very small values, find that the results are wrong, and modify
the code to handle them. But having done that, how does
"testing" help to determine the cut-off point? (Once you know
the cut-off point, of course, you will add the two bounding
values to the test suite. But you should use numeric analysis
to find the optimal cut-off point.)

And what about threading problems? Gianni has claimed that his
test find the error I found in g++'s std::string. Maybe...
I've not yet been able to evaluate it. But would it have
occured to anyone to write such a test unless the error case
were known in advance? There is an almost infinite number of
combinations of doing A in one thread, and B in another; I doubt
you can cover them all. And of course, the actual problem only
occurs very rarely once you've got the right operations.

Also: how do you know that the tests are correct. At first
glance, I think there was a problem in Gianni's test. I could
be wrong, because I'm not familiar with the surrounding
environment, but basically, how do you know that the test tests
what you want it to test, and not something else? How do you
know you've covered all of the special cases?
 
P

Phlip

James said:
Certainly, for that part which is already written. Does Bill carefully
reread all of the code that was already written, to ensure that it is
correct and conform to the local coding standards?

In my situation, yes; Bill asks to be shown all the changes, and he reads
them to himself with limited explanation. Any comments are expected to be
transferable to self-documenting code as the project progresses.

In Ian's situation, no. XP does not "require" code review, so a new pair
might just dive in and start adding the next feature. Ian's point was that
code review will still happen, serendipitously. Suppose Bill formally
reviewed but Alice is the next pair. She gets her say, too.

And "community agreements" like clear and enforcable coding standards,
covering both esthetic and technical details, is a primary XP practice. And,
no, we don't claim to have invented it. You should not be able to tell who
wrote what code.
Is there a mechanism which ensures that yet someone else intervenes once
Bill and Fred have written something?

Again, in my situation the mechanism is overt and in Ian's it is covert. It
still happens. Either the modules are an active site of progress, and they
get more review, via pairing, or they are relatively "finished", and we
don't look at them again.
In practice, well run code reviews are necessary, and once they are taking
place, adding pair programming adds very, very little value, while
effectively doubling the cost. In general---I can imagine that in some
specific circumstances, it is worth while.

You just said, "because it costs more and adds no value, it costs more and
adds no value".

I look forward to pairing with you some day.
 
P

Phlip

James said:
But suppose you never add a test which triggers a required branch?

Then the branch never exists in the code.
You'll admit that it isn't reasonable to test every possible double value.

Those aren't branches. They are (typically) continuous relations with
(typically known) discontinuities. So you test-first the edge cases because
you know there will be branches there.
So how do you choose the test values in such a way as to ensure that they
trigger all of the required branches? And that they trigger them at the
right place?
Take a simple example: a function that requires special handling for very
small values. Obviously, you'll end up testing some very small values,
find that the results are wrong, and modify the code to handle them. But
having done that, how does "testing" help to determine the cut-off point?
(Once you know the cut-off point, of course, you will add the two bounding
values to the test suite. But you should use numeric analysis to find the
optimal cut-off point.)

You are applying your understanding of soak testing to TDD. Imagine if you
knew nothing about soak testing, and start again.

TDD is a good way to emerge algorithms, but I'd rather use it for command
and control code. If I need to TDD a subtle math thing, I look up its
algorithm, and this tells me where the edge cases are. If there's no
algorithm to look up, I can't use TDD to _start_ the feature. I can
certainly use it to _finish_, after developing the algorithm.

Game programmers do this all the time, for the physics and geometry stuff.
And what about threading problems?

Once again, from the top, read my lips: TDD is not soak testing, and is not
expected to find every possible problem. TDD is a system to match each line
of code to a line of test, which will fail if you ERASE the line. NOT if you
put a tiny bug in it.

That's why TDD helps you go fast and avoid punitive debugging.
Also: how do you know that the tests are correct.

Asked and answered; by reviewing them. That's typically easy, because they
are typically 3~5 lines long. Even in C++.
 
G

Gianni Mariani

James said:
For the only meaning that makes sense.

Are stl objects thread safe ? i.e. can I call string::assign from
multiple threads simultaneously ? I know of no STL implementation that
allows that. All recent implementations that I have used though do
allow different stl objects to be called by multiple threads
simultanteously.

... Do you know of another
meaning? I don't.

Chris Thomasson on comp.pragramming.threads is proposing calling it
"strong thread safe" and "normal thread safe". Ya, don't start me.
... 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).


So? Is there any reason for them to?

Yes and no, that's the point. Sometimes it would be nice, but most of
the time no.
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.


I don't modify anything, so the implementation is not free to do
whatever it wants.

Well, you call a non const method (begin()) and it does some
modification for you, whether you like it or not in this implementation.
It's kind of a given for COW when you pull a pointer. Otherwise,
every iterator gets really complicated. IIRC, the standard wanted to
specifically allow COW implementations of basic_string, maybe this is
where the rubber meets the road on this requirement.

BTW - I added this code below to your GCC bug.
(http://gcc.gnu.org/bugzilla/show_bug.cgi?id=21334) It turns out you
don't need to have multiple threads to expose the bug. See this:

#include <string>
#include <cassert>

std::string str( 1<<20, 'a' );

void foo()
{

std::string str2( str );

const std::string & rstr2 = str2;

// str2.begin(); // uncomment line and all works well

std::string::const_iterator bc = rstr2.begin();
std::string::iterator bnc = str2.begin();

std::string::const_iterator ec = rstr2.end();
std::string::iterator enc = str2.end();

std::string r1( bc, ec );
std::string r2( bnc, enc );

assert( r1 == r2 );

assert( r1 == str );
}

int main()
{
foo();
}

I'm not a C++ lawyer so I'm not sure if the above code is malformed but
I would advocate that it does not for purely selfish reasons. Since no
threads are involved here, I'd say that it's a standard
interpretation/clarification thing.
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.
Yup.


(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.)

? I'm not sure what the point is here ? You can't detect a problem
that's not there ? An astonishing revelation ?
....

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.

Oh, that's the easy part, give me an evidence trail and I'll find the
culprit. Give me a 100 million lines of code and I'll fall asleep in 20
minutes.
In a code review, this is automatic.

The human is too fallible. Yes, a humans can pick the obvious things
pretty quickly. Things like "are there loops in your resource
acquisition" can at times be hard to tell by reading the code and some
times resource acquisition might be non-obvious, i.e. not just mutexes.
These things can be easily found in a MC stress test.
....
I couldn't get this to compile on my machine; I'm missing some
of the necessary headers.

You will need the latest Austria C++ alpha -
http://netcabletv.org/public_releases/ - warning large download - needs
cygwin on win32 to compile. Some of the code in this alpha is not pretty.
... 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.)


Most OS's on machines with multiple processors will do the same kind of
thing and yes, it does depend on pre-emptive multitasking but all modern
OS's (NT,W2K,XP,OSX,Solaris etc) are.

Machines with multiple cores or hyperthreads are pretty much the norm.
[...]
// 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.)

Well yes, in theory it is an artifact, however my first cut at the code
had me taking a const reference as a default thing.
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?

Because of this:

( i * pnum ) % g_test->s_count;

I want every thread to be able to access every element. If it's not
prime (relatively prime) then some threads won't access all the strings.
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() ) ;

A hunch - mabe a false one. I'm trying to keep the loop from doing too
many other things that allocate memory and produce contention - avoid
the threads from synchronizing if possible. Keeping a pre-constructed
string in the thread's stack might avoid (maybe not) a memory
allocation. In this case, it probably will.
followed by the AT_TCAssert?

TCAssert is "Test Case" Assert. It's an assert for the test case, it
normally throws on failure (aborting the rest of the test) but it can
also abort and dump core (which the Makefiles do when running automated
tests).
[...]
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.

It's a standard barrier, all the threads except one drop into the
condition variable Wait. The last thread resets a few things and pulls
all the threads out of the wait. Bad code - I really need to implement
a standard barrier class and fix it a little bit. Threads can drop out
of lock.Wait() spuriously according to pthread_cond_wait but I have
never witnessed it in a unit test.
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.

Well, writing test cases that really push the interface still needs to
be done, and no it's not a design test (As in TDD). It has however
uncovered some really contorted bugs that I would never have imagined
way before code release. For example, in the Austria C++ timer code, a
very contorted shutdown sequence could happen that needed a "take the
lock but give up if X happens".

This is the comment:

* On shutdown, the client thread locks *m_mutex, sets the
* m_shutdown flag, and waits for this thread to die (while
* still holding *m_mutex). If we try to lock *m_mutex
* normally here, it will be a deadlock. So we spin until
* either we get the lock, or m_shutdown is set.

TryLock< Mutex > l_trylock( *m_mutex, m_shutdown );

This one was found using the timer MC test. This is the kind of bug
that usually gets through the human only process. I'm pretty confident
that the Austria C++ timer code will handle almost any situation because
it's pushed very hard on every test run with random new requests every
time it is run. Sure, there is a more regular test as well and it found
bugs that the MC test didn't.
 
G

Gianni Mariani

Branimir Maksimovic wrote:
....
Yes. But I have to comment that proper practice is not
to share objects between threads that are not yet constructed
and not to destruct them while other threads are still using them.
This code violates both rules.

The constructor may not have returned, but when Start is called, it
should be, for all intents and purposes, fully constructed. If it's
not, it's broken, no question.
Obviously, I can't express myself in English good enough.
I meant if calls to Start and Wait are moved to base class Task
constructor/destructor,then, not necessarily, but probably,
code will fail.

It will fail very quickly on a call to a pure virtual function since the
thread will not call Work (the most derived destructor's work) but work
in the base class which is undefined (i.e. pure virtual).
 
G

Gianni Mariani

James said:
I'll try it. (I'm not 100% sure that there isn't a bug in your
code, however.)



Really. Consider the following implementation of DCL:

pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER ;
pthread_mutex_t mutex2 = PTHREAD_MUTEX_INITIALIZER ;
Singleton* theOneAndOnly = NULL ;

static Singleton*
Singleton::instance()
{
if ( theOneAndOnly == NULL ) {
Locker lock( mutex1 ) ;
if ( theOneAndOnly == NULL ) {
Singleton* tmp ;
{
Locker lock2( mutex2 ) ;
tmp = new Singleton ;
}
theOneAndOnly = tmp ;
}
}
return theOneAndOnly ;
}

It doesn't work, but can you write a test which will reveal the
error? (For starters, you certainly cannot on most Intel 32
bits processors, because they do guarantee that it works, even
if it violates Posix, and fails in practice on some other
architectures.)

What is that mutex2 doinf other than an attempt to make mods visible ?

GCC supports it like:


static Singleton* Singleton::instance()
{
static Singleton* s_x = new Singleton;
return s_x;
}


or even:


static Singleton* Singleton::instance()
{
static Singleton s_x;
return & s_x;
}

OK - DCL on singletons, I'd implement it using thread local data. Yes,
a hard one for memory systems with visibility issues. This is one of
the cases where you have to re-start the whole executable and introduce
randomization on start up and you need to determine how the errors are
found. i.e. two threads calling Singleton::instance() returning
different values.

I must admit, I have not had much experience on recent archs that need
acquire and release semantics.
 
P

Phlip

James said:
The claim is that writing the tests before writing the code
improves quality. I'm just showing that this is a specious
claim.

The claim is that "developer tests" written first improve velocity and
productivity. A "unit test" is a QA concept. Applying all the known
guidelines for QA tests to developer tests will naturally slow you
down. So, yes, such a claim would be specious. Nobody is making it
(except you, responding to a Straw Person argument).
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.

If you use TDD, and if any test fails unexpectedly, you have the
option to revert to the last state where all code worked. That means
you have the option to /not/ find where something broke. Most of the
time it will be obvious, but you could even revert then anyway.

So the point of TDD is not to somehow make every conceivable soak test
before writing the code. The point is to increase your options while
coding.
 
M

msebor

entails undefined behavior with STLport, g++ and (I think) VC++,
but is defined using Sun CC (with theRogue Wavelibrary).
Logically, I would expect it to be undefined behavior, and
consider the behavior of Sun CC an extension (defining undefined
behavior)---I'm fairly sure that this will be the situation in
the next version of the standard.

FYI, Sun ships a modified version of the Rogue Wave
implementation. The Sun string is thread safe only when
the _RWSTD_STRING_MT_SAFETY_LEVEL_2 macro
is defined. Otherwise it's up to the user to synchronize
accesses to the same string object from multiple threads.
That is also the behavior of the Rogue Wave C++ Standard
library (including the derived Apache stdcxx), and I suspect
most other implementations out there. Here's some more
detail on thread safety in the Sun implementation:
http://developers.sun.com/solaris/articles/stl-new.html
 
J

James Kanze

James said:
James Kanze wrote: [...]
Most of the problems in DCL.
They are surprisingly easy to find too.
Really. Consider the following implementation of DCL:
pthread_mutex_t mutex1 = PTHREAD_MUTEX_INITIALIZER ;
pthread_mutex_t mutex2 = PTHREAD_MUTEX_INITIALIZER ;
Singleton* theOneAndOnly = NULL ;
static Singleton*
Singleton::instance()
{
if ( theOneAndOnly == NULL ) {
Locker lock( mutex1 ) ;
if ( theOneAndOnly == NULL ) {
Singleton* tmp ;
{
Locker lock2( mutex2 ) ;
tmp = new Singleton ;
}
theOneAndOnly = tmp ;
}
}
return theOneAndOnly ;
}
It doesn't work, but can you write a test which will reveal the
error? (For starters, you certainly cannot on most Intel 32
bits processors, because they do guarantee that it works, even
if it violates Posix, and fails in practice on some other
architectures.)
What is that mutex2 doinf other than an attempt to make mods visible ?

It's making the modifications in the constructor of Singleton
visible before allowing the modification of theOneAndOnly.
Without it, the compler or the write scheduler in the CPU can
reorder the writes, so that the write of the non-null value to
Singleton precedes the writes in the constructor of the
Singleton.

Obviously, you don't need a mutex. Introducing some platform
specific membar or fence instructions can achieve the same ends.
But I felt that using Posix was going far enough; I didn't want
to throw in inline ASM.
GCC supports it like:
static Singleton* Singleton::instance()
{
static Singleton* s_x = new Singleton;
return s_x;
}
or even:

static Singleton* Singleton::instance()
{
static Singleton s_x;
return & s_x;
}

On some platforms. The generated code doesn't work on a Sparc,
or at least, it doesn't meet the guarantees it is claimed to.
In 32 bit mode, on a Sparc, it can even result in an endless
loop; I consistently use the option to turn it off. (In 64 bit
mode, it's missing a critical membar. Which means that on most
machines, it will work anyway, because most Sparc processors
don't use the fully relaxed memory model. Which is a real trap,
because obviously, until you do use the relaxed memory model,
the code works. A hardware upgrade, and it starts crashing.
Only rarely, and of course, nothing that you can reproduce.)
OK - DCL on singletons, I'd implement it using thread local data. Yes,
a hard one for memory systems with visibility issues. This is one of
the cases where you have to re-start the whole executable and introduce
randomization on start up and you need to determine how the errors are
found. i.e. two threads calling Singleton::instance() returning
different values.
I must admit, I have not had much experience on recent archs that need
acquire and release semantics.

They do make life rather difficult. Not just for testing.
 
I

Ian Collins

James said:
In practice, well run code reviews are necessary, and once they
are taking place, adding pair programming adds very, very little
value, while effectively doubling the cost. In general---I can
imagine that in some specific circumstances, it is worth while.
In practice, pair programming and continuous integration are necessary,
and once they are taking place, adding code reviews adds little value,
while increasing the cost.

Remember, pair programming adds more than just continuous review.
 
G

Gianni Mariani

James said:
It's making the modifications in the constructor of Singleton
visible before allowing the modification of theOneAndOnly.
Without it, the compler or the write scheduler in the CPU can
reorder the writes, so that the write of the non-null value to
Singleton precedes the writes in the constructor of the
Singleton.

Obviously, you don't need a mutex. Introducing some platform
specific membar or fence instructions can achieve the same ends.
But I felt that using Posix was going far enough; I didn't want
to throw in inline ASM.

OK, I would declare this code too hard to test. It should use a
standard mechanism (like a pthread once or something like that) and that
can be tested in an MC test.
On some platforms. The generated code doesn't work on a Sparc,
or at least, it doesn't meet the guarantees it is claimed to.
In 32 bit mode, on a Sparc, it can even result in an endless
loop; I consistently use the option to turn it off.

Do you have a GCC bug open on this one ?
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,743
Messages
2,569,478
Members
44,899
Latest member
RodneyMcAu

Latest Threads

Top