search

google / guava

Google core libraries for Java
Apache License 2.0
50.05k stars 10.86k forks source link

Feature Request: Futures.getOrCancel #1068

Open gissuebot opened 9 years ago

gissuebot commented 9 years ago

Original issue created by michael.deardeuff on 2012-07-16 at 06:39 AM

I sometimes require an atomic get or cancel after timeout on a future. This is important when cancelling the future updates data-structures, such as in a request-id-to-response-handler mapping.

Care has to be taken, though, to return the correct result. A typical Java developer may miss the race condition.

I propose an utility method named Futures.getOrCancel with an implementation similar to what follows.

public static <T> T getOrCancel(final Future<T> future, long time, final TimeUnit unit)
throws InterruptedException, ExecutionException {
    try {
        return future.get(time, unit);
    } catch (final TimeoutException e) {
        if (!future.cancel(true))
            return future.get();
        throw new CancellationException("Canceled due to timeout");
    }
}
gissuebot commented 9 years ago

Original comment posted by michael.deardeuff on 2012-07-16 at 05:21 PM

Here's a simpler formulation:

public static <T> T getOrCancel(Future<T> future, long time, TimeUnit unit) throws InterruptedException, ExecutionException {     try {         return future.get(time, unit);     } catch (TimeoutException e) {         future.cancel(true);         return future.get();     } }

gissuebot commented 9 years ago

Original comment posted by kurt.kluever on 2012-07-24 at 05:34 PM

Can you provide a bit more information on when this atomic get or cancel is necessary?

Status: Research Labels: Type-Addition, Package-Concurrent

gissuebot commented 9 years ago

Original comment posted by michael.deardeuff on 2012-08-22 at 08:03 PM

The caller of getOrCancel can be assured that upon the method's return, the future is redeemed AND that it has a consistent view of the future with all other callbacks and clients.

For an example, say the callback represents the result of a web-service call. Multiple callbacks may be waiting for the result to update data-structures in different layers of the system. These callbacks do different things depending on success or failure of the future.

The driving thread determines that after N seconds the request times out and should be canceled. All the system layers should act on the same result of the future.

But without an atomic getOrCancel there is a race condition where the driving thread may assume the result is canceled but the other listeners/callbacks a redeemed with success.

I use this idiom frequently in designing asynchronous web-service APIs, and have my own getOrCancel method, which I've listed above, to prevent code duplication and bugs.

gissuebot commented 9 years ago

Original comment posted by cpovirk@google.com on 2012-08-31 at 07:58 PM

cancel() is allowed to fail for basically any reason, so in theory, this getOrCancel() implementation could wait too long. Or it could be changed to neither get nor cancel in that case, presumably throwing a TimeoutException, though that's inconvenient....

http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/Future.html#cancel%28boolean%29

gissuebot commented 9 years ago

Original comment posted by wasserman.louis on 2013-04-30 at 03:19 PM

Issue #1391 has been merged into this issue.

gissuebot commented 9 years ago

Original comment posted by dopsun on 2014-02-11 at 05:15 AM

One of the case I found getOrCancel could be really useful, is while doing Request/ Reply over JMS.

In my case, my Listener of JMS consumer on a temp queue, put result into an instance of SettableFuture. The reply is useful only if it's returned before timeout, otherwise, it should be ignored. So, reply returned successfully, or not, my listener should be removed from consumer, and in certain cases, JMS consumer itself is not required and should be cosed as well.

Currently, I'm using Futures.addCallback, to close underlying JMS Consumer in onSuccess and onFailure. But, by design (and it's right), TimeoutException of get function is not reaching Callback.onFailure.

Assume my request/ reply API like this (returned future already atached with callback to handle resource closure) ListenableFuture<Reply> request(Message request);

Without getOrCancel, the code will like: ListenableFuture<Reply> futureReply = request(msgRequest); Reply reply; try {   reply = futureReply.get(3, TimeUnit.SECOND); } catch(TimeoutException ex) {   futureReply.cancel(true);   throw ex; }

But with getOrCancel in place: ListenableFuture<Reply> futureReply = request(msgRequest); Reply reply = Futures.getOrCancel(3, TimeUnit.SECOND);

Here is my version of getOrCancel:

public static <V> V getOrCancel(ListenableFuture<V> future, long timeout, TimeUnit unit)         throws InterruptedException, ExecutionException, TimeoutException, CancellationFailedException {   Preconditions.checkNotNull(future);

  try {       return future.get(timeout, unit);   } catch (TimeoutException e) {     if (future.cancel(true)) {       throw e;     } else {       if (future.isDone()) {         return future.get();       } else {         throw new CancellationFailedException(e);       }     }   } }

CancellationFailedException (or any existing exception type?) is used to highlight the cancellation failure, as once this happens, it may (or not in most of the cases) be requied to check and close the underlying resource from user code.

Futures.addCallback (usually done in a app utilies/ wrapper, like my case), will be used together with getOrCacnel, here is the exception handling summary:

Does this sound right?