(wangle) Use an atomic_flag to make FutureObject threadsafe
Summary:
There are two operations on `FutureObject` (the backing object that both `Future` and `Promise` front), which are not threadsafe currently. Those are `setContinuation` and `fulfil`. They're not threadsafe because they have a race condition between checking to see whether they should run the continuation and the other side making its modification. This diff introduces a `std::atomic_flag` variable to avoid the race and make these threadsafe.
NB Making `Future`/`Promise` threadsafe does not make this pattern safe:
f1.via(...).then(...).then(...)
(In a hypothetical world where `Future::via` is another name for `executeWithSameThread`)
There is a race between the future from `via` and the call of `then` (and between the first and second `then`s), so the `then`s could execute in the far thread as intended, or they could execute in the current thread (probably not what was intended). This diff does not solve that semantic problem of which thread this runs in. But it does make it safer, in that all those continuations will always execute, just maybe not in the thread you had intended.
You may ask what is the benefit if that semantic problem still exists? That's up for debate. I think the safety is worth it: at least everything a n00b throws at it will *work*. The question of thread semantics can iterate. If we end up with thread semantics that really don't need locks (e.g. maybe because we move to an Rx/Later style "launch" at the end of a chain) we can always toss this atomic.
How does this affect performance? I'm doing some experiments and going to write up my findings. Early results indicate that outlier performance is impacted but not by a lot, but don't count those chickens until I hatch them. Part of the reason for sending this diff out is to run windtunnel experiments.
Test Plan: tests, benchmark, close inspection
Reviewed By: davejwatson@fb.com
FB internal diff:
D1241822