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

4 Sep 2015

      On 9/4/2015 3:46 PM, Nat Goodspeed wrote:
...
On Fri, Sep 4, 2015 at 2:24 PM, Agustín K-ballo Bergé
 wrote:
...
I just had a quick look at the future implementation, to see if proper
chrono support (one of the reasons for my previous rejection) was
implemented. While `wait_for` depicts the standard signature, it does not
seem `wait_until` does.
I want to make it as easy as possible for Oliver to act on people's
feedback. Please suggest a specific change?
That would be:

"- Every API involving time point or duration should accept arbitrary
clock types, immediately converting to a canonical duration type for
internal use."

For further reference: 
http://talesofcpp.fusionfenix.com/post-15/rant-on-the-templated-nature-of-st...
...
...
The shared state has a few functions that *must* be called while holding a
lock, like `mark_ready_and_notify_`. It'd be better to make that explicitly,
and preferably have the type system enforce it, as currently proving the
correctness of those functions require tracing all the callers.
Is your suggestion that Fiber's use of its shared_state track more
closely the way Boost.Thread uses its own shared_state?
I am not familiar with the way Boost.Thread uses its own shared_state. 
My concern is that functions like `mark_ready_and_notify_` have certain 
lock requirements that currently are implicit:

     void mark_ready_and_notify_() { // is there a race here??
         ready_ = true;
         waiters_.notify_all();
     }

To prove the correctness of those functions, one has to look up all 
calls to `mark_ready_and_notify_`, all calls to those calls, and so on.
To make the code readable, I would expect lock requirements to be stated 
explicitly, for instance:

     void mark_ready_and_notify_(std::unique_lock< mutex >& lk) {
         ready_ = true;
         waiters_.notify_all();
     }

Although holding a mutex while firing on the condition variable is 
considered bad practice, so this would be even better:

     void mark_ready_and_notify_(std::unique_lock< mutex >&& lk) {
         ready_ = true;
         lk.unlock();
         waiters_.notify_all();
     }

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