-
Type: Bug
-
Resolution: Unresolved
-
Priority: Major - P3
-
None
-
Affects Version/s: None
-
Component/s: None
-
None
-
Service Arch
-
ALL
-
SP Prioritized List
As this test case demonstrates, callbacks chained via ExecutureFuture::getAsync do not run when the executor rejects work / is shutdown. While this matches the semantics for ExecutorFuture::then / ExecutorFuture::onError, it obviously does not parallel the semantics for the other method of terminating a continuation chain, ExecutorFuture::get. The two cases aren't exactly parallel, because get requires another thread to block and receive the result of the continuation chain. However, terminating a continuation chain with getAsync and then allowing that getAsync to never run means that errors from earlier in the chain may never be handled/addressed and are silently ignored. Logically, I don't think it makes sense to allow chains to terminate with unhandled results in the common case.
This has also bit us practically several times: SERVER-54735, BF-20215 , BF-20298 , SERVER-71662 , SERVER-79070 all arise from this issue.
I propose that we either:
- Ban ExecutorFuture::getAsync and remove existing use-cases, rewriting them to use onCompletion and somewhere requiring a blocking get
- Modify the semantics so that the code in getAsync always runs, even in the case of shutdown, but will possibly run inline in the case the executor rejects work. This is how we end up fixing the issue practically outside of the future-library today, via unsafeToInlineFuture, but the implications of doing so inside the library are greater.