(wangle) Use an atomic_flag to make FutureObject threadsafe
authorHans Fugal <fugalh@fb.com>
Thu, 3 Apr 2014 15:48:29 +0000 (08:48 -0700)
committerSara Golemon <sgolemon@fb.com>
Fri, 4 Apr 2014 02:54:40 +0000 (19:54 -0700)
commit8fefa085f20436d8c268e8c05f9ddd2d34b6fc55
treeb778c2ae698501957c778273cf3d1bc01c4c5ea9
parent7de41cd34151873c87d86353e596fbae2ebc74a8
(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
folly/wangle/Future-inl.h
folly/wangle/Future.h
folly/wangle/Later-inl.h
folly/wangle/Later.h
folly/wangle/README.md
folly/wangle/detail.h