Re: boost.threads: on_thread_exit

From: williamkempf_at_[hidden]
Date: 2001-09-07 08:08:13

--- In boost_at_y..., "Peter Dimov" <pdimov_at_m...> wrote:
> From: "William Kempf" <williamkempf_at_h...>
> > From: "Peter Dimov" <pdimov_at_m...>
> > >The on_thread_exit service exposed by threadmon.dll is very 
useful on its
> > >own (for win32 developers.) Perhaps it deserves to be documented?
> >
> > It's an implementation detail and should not be used directly.  
What I
> have
> > considered was adding a boost::thread::atexit() method.  I'm 
reluctant to
> > add to the interface during this review, but if it's considered 
important
> > enough I'll do this.
> 
> Yes, I know that it is an implementation detail, however it's very 
useful to
> me (as an implementation detail of my prototype thread::ref design.)
> 
> Of course I can build my own threadmon2.dll but I wanted to reuse 
as much of
> Boost.Threads as possible; I hope that you don't mind. ;-)

Well, you're free to use it even if it's not documented ;).  However, 
like I said, I'd prefer the addition of a thread::atexit().

> > >I suggest the following improvement: change
> > >
> > >void on_thread_exit(void (*) ());
> > >
> > >to
> > >
> > >void on_thread_exit(void (*) (void *), void *);
> > >
> > >(rationale: when on_thread_exit is called to register a function 
for the
> > >main thread, by the time the function is invoked, all thread-
specific
> > >variables are already gone.)
> >
> > I don't follow this.  The thread_specific_ptr<> already handles 
cleanup of
> > thread-specific data and so boost::thread::atexit() would not be 
needed
> for
> > cleanup of this type.  Passing a pointer to thread-specific data 
to
> atexit()
> > is going to result in an error since the data will have already 
been
> > reclaimed.  I personally think that boost::thread::atexit() 
should use the
> > same syntax as std::atexit().  That's why I coded on_thread_exit
() that
> way.
> 
> I ran into some very weird problems with thread_specific_ptr (which 
fits my
> needs perfectly in theory) so I had to replace it with a __declspec
(thread)
> raw pointer to thread::state. This is fine since I don't actually 
need to
> 'delete' this pointer _unless_ the thread has been adopted, in 
which case I
> allocate a new thread::state and wanted to use
> on_thread_exit(delete_thread_ref) to avoid the memory leak. Alas, I 
can't
> pass it the pointer as a parameter. ;-)

This isn't fine.  Read the docu for __declspec(thread) carefully.  It 
won't work correctly with dynamically loaded libraries, and even if 
your implementation doesn't do this one of your users will.  
Otherwise I'd have gone this route for thread_specific_ptr<> to begin 
with.  If there's "weird problems with thread_specific_ptr<>" then we 
should figure out the cause and fix it.  Can you elaborate?

> In any event, I think that the ( void (*)(void*), void* ) pair is a 
good
> thing to have in general since you can build a version that takes an
> arbitrary function object on top of it, whereas the current version 
can't
> handle this.

The same argument applies to std::atexit(), and there are work 
arounds for this, so I'd just feel better sticking with the well 
known interface even if we consider it inferior.

Bill Kempf