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

Should await using Await for forward compatibility? #189

Open
mhofman opened this issue Jul 11, 2023 · 8 comments
Open

Should await using Await for forward compatibility? #189

mhofman opened this issue Jul 11, 2023 · 8 comments
Labels
discussion Discussion topic, no immediate action required. normative Indicates a normative change to the specification

Comments

@mhofman
Copy link
Member

mhofman commented Jul 11, 2023

Given the talks about potentially adding a Symbol.asyncEnter in follow-up proposals, I would like to discuss the implications.

I assume that a [Symbol.asyncEnter]() method would return a promise (or it wouldn't have to be differentiated from [Symbol.enter]()), in which case the await using declaration would have to introduce an Await step for the result.

Given that we at Agoric are still opposed to non statically analyzable conditional await points (aka depending on the value), and that we consider a later change to the static await using semantics to be a non backward compatible change for the same reason, if there is a fair possibility of such future language extensions, we ask the champions to consider introducing an Await step today for any await using declaration.

@rbuckton
Copy link
Collaborator

rbuckton commented Jul 13, 2023

I am not convinced we should consider implementing full Python-like context managers. If we are not borrowing that complexity, then the only purpose that adding Symbol.create would serve is to act as an indicator that using or await using is preferred. If that's the only thing we want to express, then I believe the addition of Symbol.asyncEnter would be an unnecessary complication as the combination of Symbol.create + Symbol.asyncDispose would be sufficient information to indicate that an await using is desired.

@mhofman
Copy link
Member Author

mhofman commented Jul 13, 2023

My main worry is that by not awaiting unconditionally at the await using declaration, we preclude any future changes that would require awaiting. If we don't think we'll ever need something like Symbol.asyncEnter, then I agree we don't need to add an Await step today, but we are closing that possibility.

@rbuckton rbuckton added discussion Discussion topic, no immediate action required. normative Indicates a normative change to the specification labels Jul 19, 2023
@rixtox
Copy link

rixtox commented Jul 25, 2023

If it's an AsyncDisposable, the create function should be async in case the creation fails and needs to perform async roll back.

@mhofman
Copy link
Member Author

mhofman commented Jul 25, 2023

I think @rbuckton 's argument is that the create can always return an AsyncDisposable. In the case of a create failing, it can technically initiate the rollback right away and return a useless AsyncDisposable that will block that rollback's completion when disposed, and maybe return the creation error there? I do agree it's not optimal, hence why I had raised this issue.

@rbuckton
Copy link
Collaborator

This is part of the reason why I'm not in favor of calling it Symbol.create. If you were going to do work asynchronously to create the resource, you do it on acquisition/initialization, i.e.:

await using x = await openResourceAsync(); // explicit `await` as part of opening the resource

If the intent of the symbol is purely to guide users towards using/await using over const, then it should not be a way to introduce other complex effects. If you want complex effects like this, then you want something more like full Python-like context managers, which are being discussed in #49.

@rixtox
Copy link

rixtox commented Jul 25, 2023

@mhofman
I think @rbuckton 's argument is that the create can always return an AsyncDisposable. In the case of a create failing, it can technically initiate the rollback right away and return a useless AsyncDisposable that will block that rollback's completion when disposed, and maybe return the creation error there? I do agree it's not optimal, hence why I had raised this issue.

If you do that, you can't bail out the program after you return the Disposable, the closest point you can bail out the caller is when the caller uses any method on the disposable, or until the object is disposed.

function createRemoteHandle(addr: string, file: string): Creator<AsyncDisposable<Handle>> {
    return { [Symbol.create]() {
        const stack = new AsyncDisposableStack();
        const { open, closeAsync: closeRemoteAsync } = remoteHandle(addr);
        stack.defer(async () => await closeRemoteAsync());
        try {
            const { writeAsync, closeAsync: closeFileAsync } = open(file);
            stack.defer(async () => await closeFileAsync());
            return {
                writeAsync,
                async [Symbol.asyncDispose]() {
                    await using _ = stack.move();
                }
            };
        } catch (error) {
            return {
                async writeAsync() {
                    throw error;
                },
                async [Symbol.asyncDispose]() {
                    await using _ = stack.move();
                    throw error;
                }
            };
        }
    } };
}

{
    await using handle = createRemoteHandle(addr, file); // if the Symbol.create function fails,
    await someIntensiveOperations(); // you can't bail out here;
    await handle.writeAsync(data); // only here you have a chance to bail
}

And if remoteHandle() and open() above are async, it would be more complicated.

@rbuckton
Copy link
Collaborator

@mhofman
I think @rbuckton 's argument is that the create can always return an AsyncDisposable. In the case of a create failing, it can technically initiate the rollback right away and return a useless AsyncDisposable that will block that rollback's completion when disposed, and maybe return the creation error there? I do agree it's not optimal, hence why I had raised this issue.

If you do that, you can't bail out the program after you return the Disposable, the closest point you can bail out the caller is when the caller uses any method on the disposable, or until the object is disposed.

As I said, I think if you're going to perform any kind of async acquisition of a resource, you should do so explicitly in the initializer, i.e. await using handle = await createRemoteHandle(addr, file). I'm not really a fan of adding additional implicit await behavior. We chose the await using keyword order to more clearly indicate that what is being "awaited" is the side-effect of the using declaration (i.e., disposal).

@rixtox
Copy link

rixtox commented Jul 25, 2023

But the Deferrer contract I mentioned in #159 can help:

type AsyncDeferrer = AsyncDisposableStack["defer"];

function AsyncDeferrer(): Creator<Disposable<AsyncDeferrer>> {
    return { [Symbol.create]() {
        const stack = new AsyncDisposableStack();
        return Object.assign(stack.defer.bind(stack), {
            [Symbol.asyncDispose]: stack.dispose.bind(stack)
        });
    } };
}

async function createRemoteHandle(defer: AsyncDeferrer, addr: string, file: string): Handle {
    const { openAsync, closeAsync: closeRemoteAsync } = await remoteHandle(addr);
    defer(async () => await closeRemoteAsync());
    const { writeAsync, closeAsync: closeFileAsync } = await openAsync(file);
    defer(async () => await closeFileAsync());
    return { writeAsync };
}

{
    await using defer = AsyncDeferrer();
    const handle = await createRemoteHandle(defer, addr, file);
    await someIntensiveOperations();
    await handle.writeAsync(data);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion Discussion topic, no immediate action required. normative Indicates a normative change to the specification
Projects
None yet
Development

No branches or pull requests

3 participants