Re: [boost] Re: alternative thread_specific_ptr idea

From: Martin Wille (mw8329_at_[hidden])
Date: 2003-11-19 11:04:29

David Abrahams wrote:
> "Peter Dimov" <pdimov_at_[hidden]> writes:
> 
> 
>>Martin Wille wrote:
>>
>>>[...] grammar objects
>>>would simply use an alternative_thread_specific_ptr instead
>>>of all the object_with_id stuff, [...]

NB: this was related to Spirit's code; that's why I wrote
that in parentheses.

>>The right thing to do here is to just fix thread_specific_ptr (starting with
>>its specification, if it precludes the alternative implementation).

FWIW, I didn't mean the alternative implementation to fix anything,
since I didn't consider boost::thread_specific_ptr broken when I
started the alternative.

The only thing that was supposed to be "fixed" was the limitation
on the number of thread specific data items imposed by operating
systems.

However, after finding out that boost::thread_specific_ptr has to
throw an exception where the alternative implementation also has to
(.reset(p!=0); see my separate message for that) the interface
differences between the thread_specific_ptr and the alternative start
to vanish.

>>                                                                   There is
>>no need to call the fixed version alternative_thread_specific_ptr, unless
>>the alternative version fails some of the tests where the original
>>implementation succeeds.

Currently, there is a difference between thread_specific_ptr
and the alternative implementation: the alternative may throw
bad_alloc in the .get() function. I'm now inclined to disallow
that exception in favour of simply returning 0.

With the new insight, it is reasonable to discuss wether the
existing tsp and the alternative should be merged (fixing the
specification during the process)

>>Incidentally. This case is a good illustration why we should have a formal
>>process at Boost that allows contributors to submit papers (and issues) that
>>meet certain criteria and guarantees that these papers _will_ be reviewed by
>>lists members and the respective library maintainers and that a formal
>>resolution _will_ be reached and documented.
> 
> 
> I rather agree.  Boost.Threads is languishing and its design has
> frankly become too important to be allowed to do so.

Yes, I found the specification for the tsp incomplete, e.g. .reset(0)
should offer the nothrow guarantee (it is likely used in a dtor).
.release() should offer the nothrow guarantee, too, IMHO.

I propose these changes to the tsp interface definition (in the
hope of meeting the "certain criteria" ;-) ):

1. thread_specific_ptr():

Action: add std::bad_alloc to the list of exceptions being thrown.

Rationale: implementations could be required to allocate data.

Note: perhaps, an implementation defined excption should be
allowed here.

2. thread_specific_ptr():

Action: add to "Requires": "T::~T()" does not throw.

Rationale: 1. Required to guarantee no-throw for some tsp
member functions. 2. ~T() can be called when the thread
terminates.

3. ~thread_specific_ptr():

Action: replace the entire note by "Destructing a
thread_specific_pointer<T> instance while it holds
a non-NULL pointer for one or more threads will
invoke undefined behaviour."

Rationale: the existing wording is to weak, IMHO.

4. T *release();

Action: add "Throws: nothing"

Rationale: 1. This function could be used in cleanup code (dtors).
2. If there wasn't stored anything then it is pointless to throw
an exception to indicate that storing something wouldn't work. If
there was stored something, then release() must not fail, anyway.

5. void reset(T* p=0);

Action: add "Throws: nothing, if p==0 or a previous call to
reset succeeded within the same thread for an non-NULL
argument. an implementation defined exception, otherwise, when
not enough resources could be allocated for storing a copy of p"

Rationale: 1. .reset(0) is used in cleanup code, esp. in dtors.
2. an implementation could be required to allocate storage for
a copy of p. 3. once that storage exists there shouldn't be
any reason for a failure. 4. allows to implement the nothrow
guarantee for code sections that use reset(p!=0) by calling
reset(aligned_storage<T>()); reset(0); before that code section.

Note: perhaps a new member function should be added which
   implements the effects of the sequence
   reset(aligned_storage<T>()); reset(0);

6. void reset(T* p=0);

Action: replace "Effects: ... get()." by "Effects:
if an exception is thrown by reset(p) then
delete p. Otherwise: if p!=this->get() then
delete this->get()."

Rationale: 1. .reset() clearly takes onwership of p
2. enables the idiom mytsp->reset(new T).

7. void reset(T* p=0);

Action: replace "Postconditions: .... thread." by
"Postconditions: if an exception is thrown then
the state of *this does not change. *this holds
the pointer p for the current thread, otherwise."

Rationale:strong ES guarantee.

8. T* get() const;

Action: add "Throws: nothing"

Rationale: 1. tsp<> always holds a well defined value.
2. .get() might be used in cleanup code

9. T* operator->() const;

Action: replace "-<get()" by "->get()"
Rationale: typo

10. T* operator->() const;

Action: add "Throws: nothing"
Rationale: ->() is just a wrapper for .get()

11. T& operator*() const;

Action: replace "-<get()" by "->get()" (two times)
Rationale: typo

12. T& operator*() const;

Action: add "Throws: nothing"
Rationale: operator *() basically is a wrapper for .get().
A NULL value returned by .get() is a programming error
like other cases of dereferencing the null-pointer.
An implementation may add "assert(get())";

Regards,
m

PS: Thanks to Jeff Mirwaisi for discussing .get() with me.
     It really helped me to think straight.