failsafe-lib / failsafe

Fault tolerance and resilience patterns for the JVM
https://failsafe.dev
Apache License 2.0
4.2k stars 297 forks source link

Timeout may never trigger when execution starts after the timeout has already expired #260

Closed timothybasanov closed 3 years ago

timothybasanov commented 4 years ago

Reproduction:

  1. Create a thread pool with one thread and infinite queue
  2. Build FailsafeExecutor that has RetryPolicy and Timeout
  3. Schedule three tasks (getAsync) where each is slightly faster than the timeout

Execution order (with timeout of 10s and task length 9s):

As far as I can see if the future returned from net.jodah.failsafe.Functions#getPromiseAsync cancelled before its execution starts then supplier is never called and promise is never completed.

This is somewhat similar to #257, but does not require thread pool and its queue to overflow. This may require a special scheduler (e.g. recommended in that issue that executes timeoutFuture directly on the ScheduledExecutorPool.) With default scheduler timeouts do not start until the actual task execution starts, so there are no timeouts for all three tasks.

Reproducible on 2.4.0.

This is not a contribution

jhalterman commented 3 years ago

It's been a while on this issue, apologies. I tried reproducing your scenario against 2.4.0 and I couldn't exactly working as you described, though clearly with a single thread Timeouts don't fire as expected. Here's the test case I added based on your description: https://github.com/failsafe-lib/failsafe/blob/master/src/test/java/net/jodah/failsafe/issues/Issue260Test.java

This issue is now resolved in master. Let me know if the test case looks off or if I missed something.