Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add possibility to truly cancel/dispose Fututre #42855

Open
anowakiteo opened this issue Jul 27, 2020 · 21 comments
Open

Add possibility to truly cancel/dispose Fututre #42855

anowakiteo opened this issue Jul 27, 2020 · 21 comments
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async type-enhancement A request for a change that isn't a bug

Comments

@anowakiteo
Copy link

Hi,
In current version of dart there is possibility to cancel Future, using CancelableOperation. But this solutions do only one thing, cancels listening for result.

There is missing possibility to truly cancel working Future.

Eg. This small piece of code will run, even all futures are canceled:

 for (int j = 0; j < 10; j++) {
    Future<void> longOne = () async {
      await Future.delayed(Duration(seconds: 1));
      int i = 0;
      int b = 0;
      Random c = Random(j);
      while (++i < 42949672) {
        b = c.nextInt(i);
      }
      print("Debug: index: $j");
      return;
    }().timeout(Duration(milliseconds: 100)).catchError((e) {
      print(e);
    });
    CancelableOperation cancelableOperation = CancelableOperation.fromFuture(longOne, onCancel: () {
      print("cancel called");
    });
    cancelableOperation.cancel();
  }

output:

cancel called
cancel called
cancel called
cancel called
cancel called
cancel called
cancel called
cancel called
cancel called
cancel called
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
TimeoutException after 0:00:00.100000: Future not completed
Debug: index: 0
Debug: index: 1
Debug: index: 2
Debug: index: 3
Debug: index: 4
Debug: index: 5
Debug: index: 6
Debug: index: 7
Debug: index: 8
Debug: index: 9
@lrhn lrhn transferred this issue from dart-lang/language Jul 27, 2020
@lrhn lrhn added area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async type-enhancement A request for a change that isn't a bug labels Jul 27, 2020
@lrhn
Copy link
Member

lrhn commented Jul 27, 2020

There is no cancel on Future because the Future value represents the result of an asynchronous computation, not the computation itself. It's a value object, is immutable (from the outside) and it can be safely shared.

The code above is not cancelling any futures. It's cancelling a cancelable operation, but you forgot to link the onCancel callback to the code that you wanted to cancel. That would be something like:

for (int j = 0; j < 10; j++) {
    bool isCancelled = false;
    Future<void> longOne = () async {
      await Future.delayed(Duration(seconds: 1));
      if (isCancelled) return;
      int i = 0;
      int b = 0;
      Random c = Random(j);
      while (++i < 42949672) {
        b = c.nextInt(i);
      }
      print("Debug: index: $j");
      return;
    }().timeout(Duration(milliseconds: 100)).catchError((e) {
      print(e);
    });
    CancelableOperation cancelableOperation = CancelableOperation.fromFuture(longOne, onCancel: () {
      print("cancel called");
      isCancelled = true;
    });
    cancelableOperation.cancel();
  }

That's how a CancelableOperation is intended to be used.

To change futures so that they can be cancelled directly, and make the future represent the entire computation would be a significant change.
Sharing the future would no longer be safe if everybody with access can cancel the operation for everybody else. We'd probably want to enforce "single ownership", perhaps by making the future only accept a single listener for its result.
Alternatively, there will be "uncancellable" wrapped futures which ignore their cancel, or "reference counted" futures where cancels are ignored until all "owners" have cancelled. A much more complex concept.

There would need to be some kind of onCancel callback on Completers. That's fairly easily doable. Code would then be expected to check for that. A then/catchError would propagate cancels. They would probably need a callback too, so future.then(something(value), onCancel: () { ...}), since there is no Completer here. An async function would also need a way to know whether it's cancelled or not, maybe just a single boolean isCancelled made available somehow.

All in all, definitely more complicated, and not something that an be compiled directly to JavaScript promises.

@anowakiteo
Copy link
Author

The code above is not cancelling any futures. It's cancelling a cancelable operation, but you forgot to link the onCancel callback to the code that you wanted to cancel. That would be something like:

This was intented as a showcase :)

So in that moment the only way to somehow stop "Future operations" is move computation into new isolate, am I right?

@lrhn
Copy link
Member

lrhn commented Jul 28, 2020

Yes, an isolate is the only kind of computational unit that you can actually cancel from the outside in Dart.
There is no way to cancel a single "computation" in an isolate unless it is cooperating, because it is not a well-defined unit from the language's perspective.

@MatrixDev
Copy link

You cannot use CancelableOperation everywhere. This removes any benefits of async/await syntax (not even taking to account nested awaits). It also very hard to wrap everything when multiple await calls are happening as it will lead to spaghetti callbacks code.

Frankly, I'm a little surprised that if was not foreseen when designing async/await. This cancellation really limits where await can be used.

It would be really nice to implement something like cancelable coroutines in Kotlin (or Tasks in C#). They just throw CancellationException on suspension points (aka await calls). This greatly helps with resource cleanup when no-one needs it anymore.

@lrhn
Copy link
Member

lrhn commented Oct 28, 2020

That's basically what async* streams and await for are doing.
An async* function runs until a yield-point, then emit a value that other code can use. If that other code is an await for loop, the original code is paused at the yield point until the other code is ready for a new event. If the other code decides it doesn't want more events, it can cancel, and the original code will break out at the yield point (it'll return rather than throw, but it still bails out).

The future async/await combination is not intended for that kind of control. There is no yield points to break at, and allowing a cancellation at any asynchronous break is highly fragile.

@MatrixDev
Copy link

await for is a completely different thing as it is mostly used for async iteration. Moreover it will not cancel nested awaits when Stream is unsubscribed.

I'm talking about complex logic with multiple nested awaits that can't be moved to a await for loop, like awaiting results from different objects at different point of time:

Future<void> doSomething() async {
   // some logic here
   await cloudApi.getFromSomewhere() // additional awaits might happen inside
   // do something else
   await databaseApi.someEvent() // additional awaits might happen inside
   // do more stuff
   await ... // for multiple futures here
   // etc.
}

How is coroutines cancellation in Kotlin or tasks in C# fragile?
await itself can be "yield" point (or whatever you name it) at which exceptions can be thrown (or special Dart-internal value returned and handled appropriately).

@lrhn
Copy link
Member

lrhn commented Oct 28, 2020

When we designed async and await, we found that having aborts occur at arbitrary locations (and await in an async function is rather arbitrary - it's driven by whether the code you call is async, not your own choices) makes it very hard to reason about the correctness of the code.
Originally, an async* function was specified to be able to abort at both yield and await if the stream was cancelled.
The example that prompted that to be changed was:

Future resourceUser() async* {
  var resource = await allocateResource();
  try {
    ...do something with resource ...
    ... yield something ...
  } finally { 
    resource.release();
  }
}

This code is currently safe. If the allocation fails, the code throws. If the allocation succeeds, the code enters the try/finally block, and the resource is guaranteed to be released again.
If the await itself could choose to throw or exit the function, then the resource could be allocated and never released.
Being able to "abort" at arbitrary points in user code means that the user code will lose control of the computation at that point.

Even if cancelling at the await would also recursively cancel the allocation of the resource, there could still very easily be race conditions where the resource has been allocated, but the await has not gotten around to deliver the value yet. This is asynchronous code after all. Ensuring that that could not happen would mean that calling cancel on the future would need to be able to respond whether that's possible. A completed future is not cancellable any more, so you could get into situations where an await inside a cancelled operation must still not bail out, because it can't recursively cancel the awaited future any more. This gets complicated fast.

The current design for futures is not intended to solve cancelling. It never was, even before adding async/await syntax ("when designing async/await" was not when futures were designed, they predate the syntax by some time). If you need to cancel an operation, you need to cooperate with it. It's simple, but that also makes it very easy to understand: You get a future, it will eventually complete, and that is all.

Could we have done differently from the start? Probably. We could have added bool cancel() to futures, and an set onCancel(bool cancel()) callback on Completer where the callback returns true if cancellation would succeed.
That would essentially have made futures non-sharable (don't want to give people the chance to cancel futures for each other). We'd probably only have allowed you to listen to a future once, then (no sharing means no need to support sharing).
Then with async/await, cancelling the future of the async function would recursively cancel any future currently being await'ed (if it can still be cancelled, otherwise the outer cancel fails too). if successful, the await would throw a CancelError. It would also similarly cancel-and-throw if the function is blocked at an await for or yield (this is always possible, it doesn't have to recursively cancel any futures), and finally complete the original future with the result of the function.
(Which means that if you catch the CancelError or use a finally, the code might keep running. We'd have to say that any await after a cancel would immediately try to cancel its future - which can fail - and await for/yield would immediately throw too, to avoid starting new computations). Complicated, but possibly workable.

That's just not what the goal of Future was when it was designed. Maybe it should have been, but I don't see us changing that now.

@mraleph
Copy link
Member

mraleph commented Oct 28, 2020

The problem is not propagating cancellation upwards (await already would throw if the Future it waits for completes with error), it is propagating it downwards - for which there is no mechanisms right now, e.g.

Future<void> foo() async {
  await bar();
}

Future<void> bar() async {
  await baz();
}

Future<void> baz() async {
  // ...
}

void main() {
  foo().cancel();
}

Here cancellation should propagate from downwards all the way to Future produced by baz which should then complete with Error.

It is not clear to me if there is anything prevents us from defining the following semantics:

  1. async functions produce CancellableFuture<T> which extends normal Future<T> interface with cancel method.
  2. When cancel is called if underlying async method is paused on await point and value that is being awaited is CancellableFuture<T> propagate cancel to that future. If awaited value is not CancellableFuture then simply treat this await as if underlying future completed with CancellationError. If method is paused on await for - cancel subscription instead (this point might need tweaking).

I would assume this is what Kotlin does.

UPDATE: an example with allocateResource above illustrates potential source of breakage when introducing cancellation.

@MatrixDev
Copy link

@mraleph, at very least minimal api to check whether current Future async chain is cancelled would be really nice. That way developers can design "interruption" points themself. For ex.

Future<void> doSomething() async {
  // do some stuff
  if (isAsyncCancelled) return
  // do some other stuff
}

isAsyncCancelled is a really lightweight alternative to the coroutineContext.isActive from Kotlin that can be accessed only from async functions. All old code will still work, nothing will break. But it will be possible to manually check for future cancellation at "safe" points.

@lrhn
Copy link
Member

lrhn commented Oct 28, 2020

We could add magic and let doSomething.isCancelled be special-cased inside the invocation of the async function doSomething itself. That way, if you have more than one nested async function, you can check the correct one, and we won't have to insert extra names into the scope (which would be both breaking and confusing).

The idea of introducing CancellableFuture as a new interface, and make all existing platform Futures implement that, might work. I fear that it just means that we'll just end up adding extension CancelFuture<T> on Future<T> { bool cancel() => (this as CancellableFuture<T>).cancel(); }, and nobody will actually check whether their future is a CancelableFuture becasue "they all are" (except when they're not).
It would be nicer if we could add cancel on Future itself, but that will likely require interface default methods.

@MatrixDev
Copy link

MatrixDev commented Oct 28, 2020

@lrhn yeap, it would be great!

But there is still a question regarding nested cancellation mentioned by @mraleph.
baz.isCancelled should return true when foo is cancelled. Otherwise it will not be possible to properly cancel a tree of futures.

@lrhn
Copy link
Member

lrhn commented Oct 28, 2020

Cancelling a chain is fine - try recursing down and if the final future returns true, then the rest are also cancelled.
Cancelling a tree is extremely hazardous. If one side of the tree successfully cancels and the other doesn't, then you have to figure out whether that counts as a success or not, or whether you've just gotten yourself stuck in an inconsistent state.

That's a problem we have already had with Future.wait, which throws if one of the futures throw. We then have to go back and clean up after any other successful future.

@lrhn
Copy link
Member

lrhn commented Oct 28, 2020

Here's a quick first-draft of a possible proposal for cancelable futures: It will contain typos. (And isn't something I expect us to be able to pull off any time soon).

Dart Cancelable Futures

There have been many requests for cancelable futures in Dart, where you can directly write future.cancel() and it would somehow stop the underlying computation.

This is a proposal for a change to Dart which implements this and integrates it into the async/await syntax. Adding cancel to the Future API is a breaking change, so this change will likely the require interface default methods feature to be viable.

Proposal

Future API

We add a bool cancel() to the Future interface:

/// Attempts to cancel the operation behind this future.
/// 
/// Returns `true` if the operation is cancelled and `false` 
/// if cancelling the operation is not possible.
/// If the cancel is successful, further calls to this method 
/// will return `true` again and do nothing further
///
/// Some asynchronous code may not be in a state where it is possible
/// to abort its operation.
/// Perhaps it has already started an unstoppable external operation,
/// or it has allocated a resource which the caller is intended to release,
/// or maybe the computation has already completed the future,
///
/// If cancellation is successful, the underlying operation is interrupted
/// in some way, and will *likely* complete this future with a [CancelError]
/// error. It may choose to do any number of other things, including 
/// not completing the future at all, but the operation creating such
/// a future should document its behavior.
bool cancel();

To receive the cancellation, we extend Completer with:

/// Sets the cancel callback.
///
/// The cancel callback is called if the [future] of this completer
/// is cancelled using [Future.cancel].
///
/// This callback is not called again after it has returned `true` once.
/// At this point, the future is considered cancelled, and nothing can
/// change that again. Further calls to [Future.cancel] on [future]
/// will then return `true` and do nothing further.
///
/// If no cancel callback is set, the default is a function returning `false`.
set onCancel(bool cancel());

/// Whether the future of this completer has been successfully cancelled.
///
/// Whether the [future] has been *successfully* cancelled using [Future.cancel],
/// meaning that the [onCancel] callback was called and returned `true`.
bool get isCancelled;

It's still the responsibility of the creator of the Completer to actually act on the onCancel callback. If they can't, the default is to return false, meaning that cancellation is not currently possible.

Even if future.cancel() returns false once, it doesn't mean that it can't be cancelled at a later time, but if it has returned true once, all future calls should also return true (and won't invoke the onCancel callback again).

We also add:

abstract class CancelError implements Error {}

to dart:async so that we have a recognizable error to throw on cancellation.

Propagating cancels

A future doesn't need to be created using a Completer, it can also come from another asynchronous operation like future.then((x) => …) or stream.last.

Each such operation is updated to propagate the cancel operation as well as possible.

Example: var f2 = f1.then((x) => ...). If f2 is cancelled then it tries calling f1.cancel(). If that succeeds, then f2.cancel() also succeeds, and f2 is completed with a CancelError. If f1 later completes with a value or error (which is most likely a CancelError), that result is ignored. If f1 has already completed, it cannot be cancelled, and then neither can f2.

Example: var f = stream.last;. If f is cancelled before it has been completed, then the stream subscription is cancelled, f.cancel() returns true, and f is completed with a CancelError. The result of calling cancel on the stream subscription is ignored. (The alternative would be to make Future.cancel return a future, so we could wait for StreamSubscription.cancel, but that is likely too expensive and annoying).

Example: var f0 = Future.wait([f1, f2, f3]);. If f0 is cancelled, then we attempt to cancel each of f1f2, and f3. If any of them are successfully cancelled, f0.cancel() returns true and f0 completes with a CancelError. If any of the futures fail to be cancelled, they will be cleaned up the same way as if one of them had completed with an error.

Async/await behavior

An async function returns a Future which can be cancelled.

This call will (most likely) happen while the function body is blocked on an await orawait for.

If blocked on an await, then the awaited future will be attempted cancelled, and if that succeeds, the future of the function invocation is also considered cancelled. If the awaited future cannot be cancelled, then neither can this function. If cancelled, the await completes by throwing a CancelError which will (most likely) end the function and complete the future with that error.

If blocked on an await for (waiting for an event), then the subscription is cancelled, and when that has completed (whether with an error or not), the await for completes by throwing a CancelError, which will again (most likely) end the function and complete the future with that error.

If the CancelError is caught, or intercepted by a finally, then the function invocation's future is still considered cancelled. Any new attempt to do an await will immediately try to cancel the awaited future, and if successful throw a new CancelError (it might not be successful if the future cannot be cancelled, so nothing guarantees that the future will complete immediately). Any new attempt to do an await for will throw a CancelError and not listen to the stream at all (or should it listen and immediately cancel?). Coming back to an already active await for makes it cancel its subscription and throw a new CancelError rather than wait. When the function invocation completes, whether by returning a value or throwing, the future is completed with that result. For most well-behaved code, that will be a CancelError error, but we won't prevent the async function from returning a value if it really wants to.

Inside an async function, like Future<int> asyncFunction(int x) async { … }, you can check whether the current invocation's future has been cancelled by writing asyncFunction.isCancelled. If the async function is implemented using a Completer, that can be implemented using the Completer.isCancelled getter.

If someone calls future.cancel() on an async function invocation's future while the function is running (that is, it synchronously calls some code which then turns back and calls future.cancel()), then we can either:

  1. Fail to cancel. That's always a safe default, but may not be what the user wants.
  2. Succeed to cancel. That should also be safe, we'll start throwing the moment we reach an await or await for.

We should probably choose the latter, and only refuse to cancel if another awaited future prevents it.

Cost

All this obviously adds overhead. Every await will need to check whether the invocation has been cancelled (in order to immediately try to cancel the awaited future), and ditto for await for.

The completer needs to retain more state (at least one bit to remember whether it has successfully been cancelled, and one word for the onCancel callback, although those can probably be combined into one nullable field).

The state machines used by our async function implementation will need to account for the extra way to change state.

There will be a number of extra throw CancelError() expressions in the compiled code.

@MatrixDev
Copy link

Good proposal but I didn't get one thing about cancel() - "Even if future.cancel() returns false once, it doesn't mean that t can't be cancelled at a later time"

  1. isCancelled in each nested future still should return true even if cancel returned false. cancel should guaranty that all nested future's isCancelled returns true.

  2. I don't see where this return flag will be useful. In most cases cancellation happens when Future's scope dies (for example widget state was disposed). No-one will try to cancel future multiple times with the hope that if might become cancelled.

@lrhn
Copy link
Member

lrhn commented Oct 29, 2020

This is API design, it has to cover all possible invocation sequences. We can't assume that no-on will try to cancel a future multiple times. The cancel method is there, you can call it multiple times if you want to, so we have to specify what happens if you try.

If you are only using async/await and directly awaiting your futures as soon as they are created, then most likely everything will just work, and the first call to cancel will succeed, and each future will be cancelled only once. Direct uses of Future can do other things too. It has to work for that as well.

The issue that I'm trying to handle with returning false from cancel is that the future contains, or will contain, some value which must be handled (maybe an allocated resource), or that the operation cannot be cancelled for some other very important reason that I cannot even begin to guess. If the cancel fails, then you must still be ready to receive the result.
A completed future cannot tell what it contains, and it can't ask the originating code any longer, so it is always considered uncancelable.

If you do var x = await someOperation(); then you can cancel if you can cancel someOperation, but if not, you must not lose the value before it's safely stored in x. Before the await completes, it's someOperation's job to handle things. After the await, it's the caller's job to ensure that the value in x is properly disposed or otherwise handled, using finally or something similar if necessary. During the await, nobody is in charge, no running code has the value, which is why the platform can't decide to just throw away the value. The await must complete if someOperation cannot be cancelled.

(I didn't actually add a way to check whether a Future has been cancelled. The Completer can ask whether its future has been cancelled, the same way it can ask whether it has been completed, but the user of a future can only wait for results or try to cancel, not query the state first).

@MatrixDev
Copy link

MatrixDev commented Oct 29, 2020

Main concern here is that all nested isCancelled should return true event when cancel() returns false.

Maybe it is even better for users to throw CancelError manually when cancellation is handled and then callbacks will always be triggered if no CancelError was thrown? It will be easier (and more linear) than handling onCancel callbacks and also will be easy to debug whether resource was actually returned or cancellation handled.

Future<Resource> allocate() async {
  var resource = await allocation
  if (isCancelled) {
    resource.release()
    throw CancelError()
  }
  return resource
}

@MatrixDev
Copy link

Almost 2 month have passed. Is there any updates on this issue?

As an additional example for this feature you can look no further that into 'http' package published by 'dart.dev'. You can send requests there only via futures:

final response = await http.read('http://some_url')

The only way to cancel request is to call Client.close. It requires creation of Client objects per-request.

Current Future implementation limits even you own libraries...

@lrhn
Copy link
Member

lrhn commented Jan 11, 2021

A feature like this is not currently on our short-list of priorities. Also, as I stated above, it likely needs the interface default methods language feature (or something similar) for it to be viable for us to add members to Future, so it can't happen any time soon.

@AndryCU
Copy link

AndryCU commented Dec 17, 2022

Any update on this issue?

@lrhn
Copy link
Member

lrhn commented Dec 18, 2022

No updates, still not something we are working actively towards. I'm not expecting that to change in the near future.

Allowing arbitrary futures to be cancelled is a deep change, with not easily-predicted consequences.

@mraleph
Copy link
Member

mraleph commented Jul 5, 2023

Based on our discussions with @lrhn and consistent interest in this feature I have written in a proposal for how we could add some support for cancellation into the language:

https://gist.github.com/mraleph/6daf658c95be249c2f3cbf186a4205b9

@lrhn could you take a look and let me know what you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core-library SDK core library issues (core, async, ...); use area-vm or area-web for platform specific libraries. library-async type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

5 participants