Find Futures.combine (or whenAllSucceed/whenAllComplete.call/callAsync) tasks that read from a non-input Future
- Dominant language
- Java
- Stars
- 7.2k
- Forks
- 820
- Avg merge
- 5h 9m
- Merged PRs (30d)
- 50
Description
The summary is probably unclear, so here's an example:
```
Futures.whenAllComplete(a, b, c).call(
new Callable() {
@Override
public Foo call() throws Exception {
return doSomething(a.get(), b.get(), c.get(), d.get()); // uh-oh
}
});
```
Here, we provide a task to be executed once `a`, `b`, and `c` are complete. But our task accidentally depends on `d`, as well. The result is code that accidentally blocks, perhaps even on a network thread. I'm told that this was [common in <redacted Google product>](http://go/ibuoo) before they switched to a higher-level API.
I'm hoping that such a check could be an error, but there may be false positives:
- Maybe `d` is known to be an input to `a`, so if `a` is done, so too is `d` (assuming that `a` wasn't somehow cancelled without affecting `d`). But I'm hoping that those cases are rare and that their authors see them as fragile. They can always add `d` to the `whenAllComplete` call without a change in behavior.
- Maybe the code actually says something like `d.isDone() ? d.get() : someDefaultValue`. If this comes up often enough, we could add an `isDone` heuristic.
We probably want to recognize calls to other variants of `get`:
- maybe `get(timeout, unit)`, though perhaps not if the timeout is 0 (since this would be similar to the `isDone` check)
- `Futures.getUninterruptibly`
- `Futures.getChecked` and `getUnchecked`
- possibly `Futures.getNow` and/or `getDone` (if we add such things to Guava)
- possibly `CompletableFuture.getNow`
I'm unsure about `getNow` methods. On the one hand, the use of `getNow` prevents the code from hanging. On the other hand, if users use `getNow` for exactly that reason, they would lose the checking that Error Prone would provide.
I might try to write this someday, but it probably won't be anytime soon. If I do, I might want to steal a few pieces from my Google-internal `AsyncFunctionReturnsImmediate`.
Contributor guide
Assessment
This issue has not been assessed yet.