Boost :

terekhov_at_[hidden]

----
UUUU.. tired. hope I did not miss something
important among the points you have identified.
Anyway, thank a lot Bill, for your review and
taking the time! Personally, I would never
even look at such poorly prepared "document"
which I've thrown on you! ;-)
regards,
alexander.
To:	Alexander Terekhov/Germany/IBM_at_IBMDE, bdawes_at_[hidden], 
pdimov_at_[hidden]
cc:	 
Subject:	Re: Boost.Thread/thread_ptr ?!
>< in some other long and rather frustrating thread Beman wrote: >
>
> > ...back onto more productive ground. Please help!
>
>Yes, sir ;)
>
>Below is the stuff from my "sketch-pad" related to C++
>front-end to the "mythread_t" I've already presented
>to you privately and later, on the boost list as well,
>as a link to my private web site.
>
>Basically, from my year long or so presence on c.p.t,
>I could testify that the most frequent threading
>errors are:
>
>1. failure to detach/join threads;
>2. failure to call pthread_exit() from main or
>    just join relevant threads prior to invoking
>    process termination;
>3. assumption that a pthread_t written by
>    pthread_create is visible to newly created
>    thread.
Helpful summary.  Let me address my own opinion of these 3.
1) This is no different then the failure to delete a pointer.  A real 
issue,
and one for which a solution is warranted, but no solution is 
optimal.  For
example, look at the number of smart pointer variants there are to 
deal with
this issue.  So I don't think it's right for the boost::thread class 
to
address this problem.  I think it's the domain of higher level 
concepts to
deal with this.
2) I'll have to look at how you've addressed this one, but I think 
it's not
really any different than (1).  Most of the solutions I can think of 
have a
cost that I'd consider too high.  In other words they would prevent 
me from
paying for only what I need.
3) I honestly don't understand this one.
>I think that the "design" roughly sketched
>below does address all of these problems
>plus provides typesafe C++ equivalents of
>"exit( result )/result* {try/timed}join()".
>
>Please have a look at it and feel free
>to do with it whatever you think it deserves,
>starting from filling your dev/null and
>up to perhaps asking more questions ;-)
>
>regards,
>alexander.
>
>
>----
>
> > ...but why do joins return result* and not result?
>
>That is basically to:
>
>- provide canceled/no-result indication
>  (returning e.g. static joinable_thread< typename result > result*
>   joinable_thread< result >::canceled() ptr or static template class
>   ptr constant instead of static function, similar to PTHREAD's
>   PTHREAD_CANCELED non-NULL unique ptr)
>
>- provide tryjoin-failed/busy/no-result indication
>  (returning e.g. static joinable_thread< typename result > result*
>   joinable_thread< result >::busy() ptr or static template class ptr
>   constant instead of static function, similar to PTHREAD's
>   PTHREAD_CANCELED non-NULL unique ptr)
>
>- provide timedjoin-failed/join_timedout/no-result indication
>  (returning e.g. static joinable_thread< typename result > result*
>   joinable_thread< result >::join_timedout() ptr or static template
>   class ptr constant instead of static function, similar to 
PTHREAD's
>   PTHREAD_CANCELED non-NULL unique ptr)
I disagree with this decision.  Here's my reasoning:
1) Returning a pointer from a function invariably leads to the very 
bad
mistake of returning a pointer to an object on the stack which won't 
exist
after the function returns, dropping us into undefined behavior land.
What's really bad here is that on many platforms such a mistake can go
uncaught since it appears to work.
2) The vast majority of the time there's no need for a return value 
at all.
Requiring a return value in the function signature then leads to the
annoyance of having to provide a bogus return value in these cases.
3) When you do need to return information from a thread there's 
already a
logical place to put the data... in the function object used to 
create the
thread.
4) To really use the pointer return concept correctly you wind up 
using the
equivalent of (3) any way.
If there truly is a need to determine if a thread was cancelled and
explicitly coding for that using (3) above is considered too much 
work then
maybe we should return a simple bool only, which would be implicitly
returned?
>(note that user's thread function/routine itself may not
>necessarily return PTR-type, it could be type/void)
>
>Below is the newer version from my "sketch-pad".
>Hopefully it will clarify things a bit more (instead
>of confusing you totally ;)
>
>Basically the idea is to use both runtime-polymorphism/
>dynamic_casts for typesafe exit()/{..}join() AND
>complile-time meta-stuff. I've got an impression
>that "thread_new" is pretty much the same (but w/o
>_? stuff) as Peter's "bind"er already is (which drives
>me crazy when I see how complicated its impl is for
>a such meta-impl-newbie as I am ;)
Most "polymorphic" solutions for this are too expensive and generally 
create
some problems for the users.  The Boost.Bind library may be 
complicatd in
it's implementation because it avoids this, but its usage is 
straightforward
and simple.  You shouldn't care about the implementation, only about 
how
easy and safe it is to use.  More importantly, I think it's a bad 
idea to
"reinvent the wheel" for a concept such as this in a threading 
library.
>// This one is basically encapsulating "my_thread_t"
>// but does NOT allow join operations
>class thread;
Then does it not "leak"?  I don't see how/why this is used in your
descriptions.
>// Holds a ptr to thread object and calls addref()/unref()
>// COULD hold null pointer
>class thread_ptr;
I thought you wanted an implementation that was closer to POSIX.  
POSIX
doesn't ref-count, and I think that was the right decision.  I don't 
have
anything against there being a ref-counted variant, but the 
description
presented here shows that this is actually *THE* variant actually 
used, with
no non-ref-counted variant that can be used when creating threads 
(the only
non-ref-counted variant is not thread shareable and can only be 
created for
the current thread of execution).
>// Result of thread::current - does not provide ref.counting
>// (for better performance) and is convertable to thread_ptr
>// and 
joinable_thread_ptr<result>/joinable_result_holding_thread<result>
>// *using dynamic_cast*
This description points out what I like the least about all of this.  
There
are too many variants for a single concept (thread), all of which 
behave
very differently but can be used to represent the same thread of 
execution
(at the same time!).  This is just going to lead to confusion, and a 
lot of
"newbie" mistakes.
>// -------
>// Provides template< class result > void exit( result );
>// which does dynamic_cast under the cover
>// -------
>// thread::current() *returns NULL-PTR for non-adopted/"non-C++" 
threads*
Then how do you "adopt" the thread?
>// -------
>// valid in a calling thread ONLY/non-thread-shareable w/o
>// construction of some other type of ref.counted thread pointer
>// -------
>class current_thread_ptr;
You didn't like the semantics of the original boost::thread in this 
regard.
The only differences here that I see are that you use a seperate type 
and
you allow promotion to a shareable type.  Using different types at 
first
seems to make the differing semantics obvious and so would be 
considered a
good thing, but the existing of different types (especially when they 
are
convertable to each other) will only lead to confusion, IMHO.  If we 
can
remove the restriction on not being able to share the object entirely 
then
you've got a much better solution.  I think the latest design I 
posted does
this, and gives us a design that's more faithful to POSIX.
>// ---
>// I was just thinking that to just store/compare different thread 
id/ptrs
>// it would be too prohibitive to require them to be of the same type
>// (according to type of thread_routine return value). For example,
>// it would significantly complicate/invalidate code which currently 
stores
>// and compares pthread_t objects. Basically thread_ptr is a C++
>"equivalent"
>// of pthread_t (but with extra NULL state, lack of witch is a real
>problem/
>// inconvenience in PTHREADS). Making thread::current() return 
thread_ptr
>// instead of current_thread_ptr would mean extra mutex locking... 
might be
>// OK, but given that I've used to FAST pthread_self(), personally,
>// I have difficulties with this idea. On the other hand, I do not 
see it
>// as a critical issue.
>// ---
>// (The idea of current_thread_ptr is based on my experience that
>programmers
>// thread-share pthread_t objects obtained from pthread_self() rather
>rarely. And
>// since "self" ref is already counted in the "thread state", it is 
ok NOT
>to ref.count
>// copies of it AS LONG as they are NOT shared with respect to other
>threads.
Correct as far as safety is concerned, but I still don't like the 
idea.
>//
>// Also, I do not like the idea of "automatic" adoption under the 
cover of
>thread::current(),
>// I would rather prefer that thread::current() would return an 
indication
>(NULL-ptr) that
>// running/current thread was not created as C++ thread/already 
adopted as
>C++ thread,
>// and would let ME decide what to do in such situation. I think 
that it is
>more flexible.
I'd agree, but you don't discuss *HOW* to adopt such a thread 
explicitly.
Also, it complicates things if "current()" can return "null".  The 
latest
design I posted doesn't return "null", but instead returns a non-
adopted
thread, for which most operations can still be applied.  More 
importantly,
since this design is very close to POSIX, if adopted as a standard it 
should
be possible for implementers to do all of this with out the need for
adoption (and so I'd expect that the standard would define this 
concept with
out adoption).
>// Exception would also work, and actually, I am not sure what is 
better -
>exception or
>// NULL-ptr...)
I'd say "null" is, but either would work.  I just think the solution I
posted addresses the issue in a cleaner way.
>// Provides typesafe result* {try/timed}join() methods.
>// Used for functions returning a pointer type only
>template< class result >
>class joinable_thread : public thread {};
>
>// joinable_thread smart ptr
>template< class function >
>class joinable_thread_ptr : public thread_ptr {};
>
>// Provides typesafe result* {try/timed}join() methods.
>// Used for NON-ptr returning functions only (and void funcs),
>// holds function result for joiners (or NULL-ptr for void
>// funcs);
>template< class result >
>class joinable_result_holding_thread : public joinable_thread_ptr {};
>
>// Used to create joinable_thread (for "type* f()") and
>// joinable_joinable_result_holding_thread (for "type f()")
>// objects
>template< class function >
>struct thread_traits;
>
>// Helper class
>template< class function >
>class new_thread_ptr : public joinable_thread_ptr< thread_traits< 
function
> >::result > {};
And now we seem to have about 6(?) different "thread" types with no 
clear
distinction of when to use which.
>// Thread factory for functions with no arguments
>template< class function >
>new_thread_ptr< function > new_thread( function );
>
>// Thread factory for functions with ONE argument
>template< class function,class arg >
>new_thread_ptr< function > new_thread( function,arg );
>
>// etc..
I think it's much cleaner to use a concept such as Boost.Bind here.
>// Example
>
>int* f1()    { /***/thread::current()->exit( some_int_ptr ); }
>int* f2(int) { /***/thread::current()->exit( some_int_ptr ); }
>int  f3()    { /***/thread::current()->exit( blah+blah    ); }
>int  f4(int) { /***/thread::current()->exit( blah+blah    ); }
>...
>// No cancel expected
>return *new_thread( f1    )->join() +
>        *new_thread( f2,10 )->join() +
>        *new_thread( f3    )->join() +
>        *new_thread( f4,10 )->join();
>
>----
>string fs() { /***/thread::current()->exit( string("hello word") ); }
>
>// Cancel checking
>joinable_thread_ptr<string> tptr = new_thread( fs ); //
>joinable_result_holding_thread
>// joinable_thread_ptr<thread_traits( fs )::join_result> tptr = 
new_thread
>( fs );
>
>//...
>tptr->cancel();
>string* presult = tptr->join();
>
>cout << ( tptr->canceled() == presult ) ? "CANCELED" : *presult;
>
>----
>string fs() { /***/thread::current()->exit( string("hello word") ); }
>
>// try join/no cancel expected
>joinable_thread_ptr<string> tptr = new_thread( fs ); //
>joinable_result_holding_thread
>// joinable_thread_ptr<thread_traits( fs )::join_result> tptr = 
new_thread
>( fs );
>//...
>string* presult = tptr->tryjoin();
>
>cout << ( tptr->busy() == presult ) ? "BUSY" : *presult;
>
>----
>string fs() { /***/thread::current()->exit( string("hello word") ); }
>
>// timed join/no cancel expected
>joinable_thread_ptr<string> tptr = new_thread( fs ); //
>joinable_result_holding_thread
>// joinable_thread_ptr<thread_traits( fs )::join_result> tptr = 
new_thread
>( fs );
>//...
>string* presult = tptr->timedjoin( timeout );
>
>cout << ( tptr->join_timedout() == presult ) ? "JOIN TIMEDOUT" : 
*presult;
>
>-----
>// user's main
>void thread::main( blabla )
>{
>
>   // Exit main/initial thread and wait for last thread term.
>   // The same is done automatically on return from this func.
>   thread::current()->exit(); // checks the type of currect thread
>   // object and essentially just calls pthread_exit( NULL );
>
>   // Process exit should be done explicitly:
>   // exit( 0 );
>
>}
>
>// lib's main
>int main( blabla )
>{
>
>   // Adopt main thread/create joinable thread object for *void* func
>   //....
>
>   // Resume "forced-suspended" global threads
>   //....
>
>   // Call user's main()
>   thread::main( blabla );
>
>   // Exit main thread
>   pthread_exit( NULL );
>
>}
Unless main() is supplied by the library the above is simply too 
complex.
If main() is supplied it makes it more difficult to add some threads 
to an
existing application (and may make it impossible to do in shared 
libraries)
and results in rather unusual coding practices.  I'd rather leave 
things as
they are in POSIX when it comes to the "pthread_exit()" problem (i.e. 
the
user must explicitly call it if there's any chance there are other 
threads
still running) and when it comes time to standardize this we can ask 
for a
core language change here to remove the need entirely.
Bill Kempf
williamkempf_at_[hidden]
_________________________________________________________________
Get your FREE download of MSN Explorer at 
http://explorer.msn.com/intl.asp.