Skip to content
This repository has been archived by the owner on Feb 2, 2023. It is now read-only.

Forbid await in finally blocks #51

Closed
benjamingr opened this issue Jul 1, 2015 · 14 comments
Closed

Forbid await in finally blocks #51

benjamingr opened this issue Jul 1, 2015 · 14 comments

Comments

@benjamingr
Copy link

If finally blocks are going to be used for clean up when async functions will be cancellable and await is allowed in finally then it looks like there is no way to perform actual cancellation in async functions.

See tc39/proposal-observable#33 (comment) for a convincing and well thought out argument by @jhusain for generators and cleanup.

@getify
Copy link

getify commented Jul 1, 2015

how is that different than yield being allowed in finally blocks in generators?

@benjamingr
Copy link
Author

@getify it's not. However we can't fix yield being allowed in finally blocks because the ES2015 spec has already been released.

@domenic
Copy link
Member

domenic commented Jul 1, 2015

I don't really understand this. There's many use cases where I want to do async things inside a finally block. So many in fact that C# is introducing a whole new type devoted to this. dotnet/roslyn#114

@zenparsing
Copy link
Member

I'm not sure that allowing await within finally blocks necessarily means that the operation is not cancelable. The cancelation routine might even require calling out other async functions.

@domenic
Copy link
Member

domenic commented Jul 1, 2015

+1 @zenparsing, pretty much what I was going to write.

It does imply that any cancelable promise type might want to change its onCanceled from returning void to returning a promise.

@benjamingr
Copy link
Author

@domenic C# has a proposal for a new type for this. For what it's worth his proposal isn't the most correct. I recall talking to Eric Lippert about this let me see if I can dig this. The mega-thread you posted on the es-observable issues shows that there is a lot to learn though. In C#, cancellation works differently anyway. I'll investigate further. Disallowing finally sounded like a future compatible way to say "we can figure this out later and ship most of the funtionality today".

I talked to @petkaantonov about what bluebird does in coroutines and to @bwoebi about what PHP does in task aborts. Also bringing @rdlowrey for having implemented coroutines with these semantics.

@zenparsing - what bluebird does is allow yield in finally blocks only if the generator has not been cancelled. If a finally block is run on a cancelled generator - the coroutine throws. This is similar to what PHP does with generators.

I think that as future compatibility with cancellation semantics is a goal - we need to figure out some solution to this problem. When we had these discussions implementing using semantics in bluebird a lot of interesting issues arose.

@spion
Copy link

spion commented Jul 1, 2015

I can only see the downsides of forbidding this, and no upsides.

await in finally should be fully allowed even with cancellation. There are some resources that cannot be cleaned up without waiting.

The promise is still cancelled, but the underlying finally block continues to run. What is the issue with that?

@spion
Copy link

spion commented Jul 1, 2015

There is a difference with generators in terms of the return result. The return result of an async function is a promise. Canceling the promise changes its behavior directly. But awaiting inside the finally block does not directly affect the result of the async function. The promise can remain cancelled. Therefore it should be allowed.

Generators are different. yielding inside the finally block in a generator directly affects the end result (the iterator). Therefore, if yield is allowed within finally, the generator isn't cancelled, its generating more values. Which leads us to the conclusion that yielding inside finally should not be allowed.

@benjamingr
Copy link
Author

@spion so the promise is marked as cancelled but the async function can continue running for as long as it wants (even forever) and does not have to actually attempt cleanup and finish?

@spion
Copy link

spion commented Jul 1, 2015

@benjamingr if you want to support async cleanup, yes. I don't see an issue with its continued work in the background - as long as it doesn't observably affect the resulting promise.

@benjamingr
Copy link
Author

Hmm, then that guarantee (or lack of) will need to be documented in cancellable promises. That sounds reasonable to me.

@zenparsing
Copy link
Member

@domenic Should a hypothetical cancel method on a promise itself return a promise, then? That's the only way that async errors arising from cancellation would be catchable.

@benjamingr
Copy link
Author

@petkaantonov ^ this makes sense to me. (Where cancel returns a promise for cancellation being done, this makes cancel asynchronous again but for a different reason I believe)

@benjamingr
Copy link
Author

I think we can allow await in finally - after reevaluation it's OK to not make as strong of a guarantee on cancellation.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants