Re: [boost] Boost.Fiber mini-review September 4-13

Subject: Re: [boost] Boost.Fiber mini-review September 4-13
From: Agustín K-ballo Bergé (kaballo86_at_[hidden])
Date: 2015-09-07 11:52:08

On 9/4/2015 12:14 PM, Nat Goodspeed wrote:
> Hi all,
>
> The mini-review of Boost.Fiber by Oliver Kowalke begins today, Friday
> September 4th, and closes Sunday September 13th. It was reviewed in
> January 2014; the verdict at that time was "not in its present form."
> Since then Oliver has substantially improved documentation,
> performance, library customization and the underlying implementation,
> and is bringing the library back for mini-review.
>
> The substance of the library API remains the same, which is why a
> mini-review is appropriate.

I had a look at `condition_variable[_any]` documentation, and was 
surprised to see that those two classes are guaranteed to be just 
aliases to a single implementation. The destructor documentation states:

"All fibers waiting on *this have been notified by a call to notify_one 
or notify_all (though the respective calls to wait, wait_for or 
wait_until need not have returned)."

However, the implementation tries to grab a lock on a mutex member after 
the fiber has been resumed: 
https://github.com/olk/boost-fiber/blob/master/include/boost/fiber/condition.hpp#L129

If the destructor starts after waiters have been notified but before the 
respective calls to `wait[_xxx]` have returned, then this access would 
potentially be undefined behavior. Is there anything specific about the 
way fibers are implemented that guarantee that a call to `notify[_all]` 
can't return until all calls to `wait[_xxx]` have returned? Or any other 
implementation detail that guarantees this to be well defined?

Have you had a look at N2406 
[http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2406.html]? It 
has a lot of valuable information on this subject.

Also, I would like to suggest you make the name `condition` an 
implementation detail, even if a single implementation meets the 
requirements of both cvs. As the documentation states, this name has 
been long deprecated in Boost.Thread.

Finally, I believe someone already mentioned that the use of a `queue` 
while waiting is inefficient, so I won't go into detail.

Regards,
-- 
Agustín K-ballo Bergé.-
http://talesofcpp.fusionfenix.com