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