Re: [boost] [afio] Formal review of Boost.AFIO

Subject: Re: [boost] [afio] Formal review of Boost.AFIO
From: Niall Douglas (s_sourceforge_at_[hidden])
Date: 2015-08-28 20:05:10

On 28 Aug 2015 at 10:16, Thomas Heller wrote:

> I took the effort to also reimplement your hello world example:
> https://gist.github.com/sithhell/f521ea0d818d168d6b35

Firstly thank you very much for your gist. This makes it much easier 
to understand what you are talking about.

My very first thought was that you are using reference capturing 
lambdas in a concurrent use case which may be fine in internal 
implementation, but is a serious no-no to ever place upon a library's 
end users. People regularly get waits and wakeups wrong, and I have 
seen code bases wrecked by memory corruption and race conditions 
induced by reference capturing lambdas used in concurrent situations.

For me this is an absolute red line which cannot be crossed. I will 
never, *ever* agree to a library API design which makes end users 
even feel tempted to use reference capturing semantics in a 
concurrency context. Period. They are banned as they are nuclear 
bombs in the hands of the typical C++ programmer.

So let's replace all your reference captures with shared_ptr or value 
captures and thereby making them safe. I have forked your gist with 
these changes to:

https://gist.github.com/ned14/3581d10eacb6a6dd34bf

As I mentioned earlier, any of the AFIO async_* free functions just 
expand into a future.then(detail::async_*) continuation which is 
exactly what you've written. So let me collapse those for you:

https://gist.github.com/ned14/392461b85e73add13e30

This is almost identical to my second preference API for AFIO, and 
the one Gavin Lambert suggested in another thread.

As I mentioned in that thread with Gavin, I have no problem at all 
with this API design apart from the fact you need to type a lot of 
depends() and I suspect it will be a source of unenforced programmer 
error. I feel the AFIO API as submitted has a nice default behaviour 
which makes it easy to follow the logic being implemented. Any 
depends() which is a formal announcement that we are departing from 
the default stand out like a sore thumb which draws the eye to giving 
it special attention as there is a fork in the default logic.

Under your API instead, sure you get unique futures and shared 
futures and that's all very nice and everything. But what does the 
end user gain from it? They see a lot more verbiage on the screen. 
They find it harder to follow the flow of the logic of the operations 
and therefore spot bugs or maintain the logic. I am seeing lots of 
losses and few gains here apart from meeting some supposed design 
principle about single responsibility which in my opinion is a useful 
rule of thumb for inexperienced programmers, but past that isn't 
particularly important.

I however am probably being a little unfair here. I reduced your 
original to something AFIO like and something I previously gave deep 
consideration to adopting before rejecting it, and you were probably 
actually advocating that people manually write out all the .then() 
logic instead of using free functions which expand into exactly the 
same thing.

If you really strongly feel that writing out the logic using .then() 
is very important, I can add that no problem. Right now the 
continuation thunk types live inside a detail namespace, but those 
can be moved to public with very little work.

Niall

-- 
ned Productions Limited Consulting
http://www.nedproductions.biz/ 
http://ie.linkedin.com/in/nialldouglas/