04:40
<Steve Hicks>
In terms of events that we wanted to set to dispatch-context from the start, there was mention of same-window postMessage (I assume it's nonsense to talk about preserving context when messaging a different window?). Would MessageChannel also be covered here? (the reason I ask is that I'm working on a userland aroundEach for Jasmine - it mostly works, but only if I polyfill context propagation for MessagePort.prototype.onmessage)
11:58
<Chengzhong Wu>
agree, these tasks are pretty similar and I think we should apply the same policy on them. Contexts should not be preserved across agents. But if the async context variable is explicitly passed to a different window, I don't see it to be a problem to preserve the context.
11:58
<nicolo-ribaudo>
In terms of events that we wanted to set to dispatch-context from the start, there was mention of same-window postMessage (I assume it's nonsense to talk about preserving context when messaging a different window?). Would MessageChannel also be covered here? (the reason I ask is that I'm working on a userland aroundEach for Jasmine - it mostly works, but only if I polyfill context propagation for MessagePort.prototype.onmessage)
Given that there is a use case we can surely include it in the list
11:59
<nicolo-ribaudo>
agree, these tasks are pretty similar and I think we should apply the same policy on them. Contexts should not be preserved across agents. But if the async context variable is explicitly passed to a different window, I don't see it to be a problem to preserve the context.
Yeah I think context preservation should work across realms in the same agent: if they can pass (some) objects to each other, they should be able to preserve the context
12:00
<Andreu Botella>
Part of the reason why we focused on same-window postMessage is because that is sometimes used as a scheduler
12:01
<Andreu Botella>
but yeah, it makes sense to extend that to other windows in the same agent, or to MessageChannel
16:46
<snek>
i've been playing around with async context and disposables, and i realized they don't really work well together because we don't restore the context around suspend points (except in async generators?). curious if anyone else has thought about this at all.
16:48
<Chengzhong Wu>
async context variable disposable needs https://tc39.es/proposal-async-context/#sec-generatorresume to play well with generator/yield
16:54
<snek>
more specifically i was hoping these two examples would behave the same, but the second one doesn't work because you'd have to restore the context "inside" await, after it sets the adds the promise reaction and before it returns to the caller. https://gist.github.com/devsnek/2eee5001144f7e39513e3694ca6b3e8d
18:40
<Steve Hicks>

I'm not sure I quite understand your example. You've written [[AsyncContext]] = 'FOO' but (syntax aside) the current proposal doesn't allow just setting the context. Are you assuming something like

{
  using _ = asyncVar.setWithDisposable('FOO');
  foo();
}

or are you thinking of something else entirely?

18:49
<Steve Hicks>

That said, we're currently thinking about the impact of whether yield preserves context (i.e. the initialization context from the initial generator call), or whether it brings in the context from the surrounding next() caller (i.e. the dispatch context). If it uses initialization context then context is sensibly block-scoped, and using makes a lot more sense. If context might change across a yield then it's a lot less clear that using is at all viable, since the state to clean up at the end of the block scope may have changed out from under it. So if yield exposes the dispatch context, then we're back to reconsidering what an enterWith or set semantics might look like. In particular is the question of whether set in an inner/outer function body should change the value out from under an outer/inner function. I.e.

async function f() {
  g();  // n.b. not awaited
  x.set(2);
  await 1;
  console.log(y.get()); // 3 ?
}
async function g() {
  await 1;
  console.log(x.get()); // 2 ?
  y.set(3);
}
18:51
<Steve Hicks>
and how exactly one sets the boundaries on where mutations do affect
19:01
<nicolo-ribaudo>

snek I'm also not fully understanding your question, but it seems similar to a discussion we had in the past:

runWithContext(1, async () => {
  console.log(getContext()) // 1
  await new Promise(resolve => runWithContext(2, resolve));
  console.log(getContext()) // ?
});

Is answering what the second getContext() call logs the same as answering your question?

19:03
<snek>

I'm not sure I quite understand your example. You've written [[AsyncContext]] = 'FOO' but (syntax aside) the current proposal doesn't allow just setting the context. Are you assuming something like

{
  using _ = asyncVar.setWithDisposable('FOO');
  foo();
}

or are you thinking of something else entirely?

yes i am thinking about a disposable like that
19:04
<snek>
except this doesn't work with async functions because they don't restore the scope on awaits, you're forced to wrap the function.
19:13
<nicolo-ribaudo>

Async functions capture the scope right before pausing and restore them when resuming:

19:14
<snek>
that's not exactly what i'm talking about
19:16
<snek>
its about code that effectively uses enterWith
19:18
<snek>
i'm referring to specifically when the async function is suspended, it does not restore the context. additionally, as you noted, the promise reactions capture and restore their context. i'm basically suggesting that async function body evaluation should do the same.
19:18
<nicolo-ribaudo>
Oh you mean that before pausing they restore the context that was active before that the code in the async function run?
19:19
<snek>
yes
19:19
<snek>
this is only relevant if you can "mutate" async variables though, which the proposal currently does not allow
19:19
<nicolo-ribaudo>
Ok -- just within the current AsyncContext itself this seems to be only editorial right? It's not observable, and it only becomes observable if somebody builds a new API that behaves like async context and uses the same propagation mechanism
19:20
<snek>
idk if i'd call it editorial but yes the functionality would be something the host could build on top of, not something the js api currently exposes
19:22
<Justin Ridgewell>
There are various changes to the spec we’ll need to make if we ever add using support. Right now they’re not necessary to make.
20:54
<Steve Hicks>
But if we're contemplating either (1) making yield bring in the dispatch-context, or (2) exposing enterWith, then it's possible that we're already painting ourselves into a corner where using is a non-starter.
20:54
<Steve Hicks>
To be clear, this I'm not convinced that this is a problem, but I do think it's worth at least some thought
20:56
<Steve Hicks>
In particular, if you have enterWith then presumably you can implement using semi-reasonably in userland.
20:56
<snek>
using can be done in terms of enterWith, but you will get behavior that is probably not what you want unless the context is restored properly at suspend points
20:56
<nicolo-ribaudo>
I'd really prefer that, if we do one of those two in the language, we do `using`. It's much harder to make mistakes with it
20:57
<snek>
i am generally a fan of what using can enable with this, for example using span = tracer.createSpan(). all i'd ask for at this point is that we don't prevent that from happening in the future.
21:00
<Steve Hicks>
IIRC, The concern with using today was that it wasn't sufficiently hermetic.
21:00
<Steve Hicks>
i.e. what happens if you write tracer.createSpan()[Symbol.enter]() and then never dispose it?
21:01
<Steve Hicks>
Of course, that's less concerning if enterWith is already a possibility
21:01
<Steve Hicks>
(effectively the above is enterWith IIUC)
21:02
<snek>
yes i would say this is equiv to providing enterWith
21:02
<snek>
though if we start to seal up some of the things like context leaking out of suspends, the "never dispose" problem becomes less and less of a danger to callers
21:02
<nicolo-ribaudo>
I did have concerns with `using` because the proposal makes it trivial to just forget `using`. However, `using` with some check that you actually get Symbol.dispose would be much better than enterWith, since it makes it hard to *accidentally* forget to close the context
21:03
<Steve Hicks>

but it still leaves the question of what happens if you write

const d1 = v.enter(1);
d1[Symbol.enter]();
const d2 = v.enter(2);
d2[Symbol.enter]();
d1[Symbol.dispose]();
d2[Symbol.dispose]();

?

21:04
<Steve Hicks>
though if we start to seal up some of the things like context leaking out of suspends, the "never dispose" problem becomes less and less of a danger to callers
Not sure what you mean about context leaking out of suspends - IIUC this is already sealed up, though enterWith could (depending on spec) cause a problem
21:05
<snek>
yes i mean if this functionality exists
21:05
<ljharb>
as a separate topic, i don't really grok the details of the proposal personally, but it seems like a number of node core collaborators are unhappy with the design of the proposal (in ways i also don't understand). i don't think it would be appropriate to advance an implementation that isn't also useful in node, so before seeking advancement, can yall please ensure their concerns are explained and hopefully addressed?
21:06
<nicolo-ribaudo>
As far as I understand, the "problem with Node.js" is that they have two APIs for setting the context, and this proposal is only defining one leaving the second one as a follow-up proposal
21:06
<Steve Hicks>
I did have concerns with `using` because the proposal makes it trivial to just forget `using`. However, `using` with some check that you actually get Symbol.dispose would be much better than enterWith, since it makes it hard to *accidentally* forget to close the context
At the time we were talking about forcing the use of using so that it was syntactically guaranteed to be correct - but Ron Buckton had an issue with composability. snek's example of tracer.createSpan() exactly demonstrates the composability problem, since createSpan would need to call [Symbol.enter] and [Symbol.dispose] directly rather than syntactically.
21:07
<nicolo-ribaudo>

but it still leaves the question of what happens if you write

const d1 = v.enter(1);
d1[Symbol.enter]();
const d2 = v.enter(2);
d2[Symbol.enter]();
d1[Symbol.dispose]();
d2[Symbol.dispose]();

?

In this example, I'm hoping we can make it throw at the second dispose call somehow
21:08
<nicolo-ribaudo>
I haven't thought about it enough to say how
21:13
<Steve Hicks>

Independently, I think snek's original question was how do the following two snippets work?

async function f() {
  await 1;
  console.log(v.get());
}
{
  using _ = v.enter(2);
  f();
}

and

async function f() {
  using _ = v.enter(2);
  await 1;
}
f();
console.log(v.get());
21:14
<snek>
well i know how they work given the current spec text
21:15
<Steve Hicks>
they don't work at all under the current spec text
21:15
<Steve Hicks>
because enter isn't a thing
21:15
<snek>
yes i mean assuming that such a thing existed
21:16
<Steve Hicks>
gotcha - I think introducing enter would absolutely require substantial changes to account for it
21:17
<Steve Hicks>
and there's a few different options for what those changes look like
21:19
<snek>
i think its pretty simple in terms of the spec, store context on the asyncfn/generator object when started and swap back to it whenever a suspend happens.
21:20
<snek>
but we don't need to deal with it right now, i was just more curious about previous thoughts in this space and ensuring that we don't prevent this from happening in the future
21:22
<Steve Hicks>
I don't quite understand what you mean by "whenever a suspend happens" - I would have expected to swap back to it when it resumes? But the conversation yesterday was motivated by iterator helpers - if we decide that they should behave like synchronous events then it would be more consistent for generators to not swap back to the initiating context on resume - for one thing, that would make it impossible to write Iterator.prototype.map as a generator, which seems surprising.
21:23
<Steve Hicks>
There was a side benefit that removing the context swap on yield makes the spec simpler and is also probably easier and more performant for implementations
21:24
<snek>
when resuming, the context is already handled by promise reaction jobs.
21:24
<Steve Hicks>
not for generators
21:24
<nicolo-ribaudo>
It would probably be clearer @devsnek:matrix.org if you remove the top-level await from your example
21:24
<nicolo-ribaudo>
That's what confused me
21:25
<nicolo-ribaudo>
Just delete the await keyword
21:25
<Steve Hicks>
but then I'm also that much more confused because you're concerned with what the context will be outside the asyncfn body?
21:25
<snek>
well its interesting both with and without the await keyword, i guess. but those are different things.
21:27
<Steve Hicks>
(my understanding is that it gets restored to whatever it was immediately before the body was entered - which is either the same thing if it's the first entry, or else it's the empty/top context from the microtask queue)
21:27
<snek>
the current semantics are this https://gc.gy/f5b45fb1-6bd7-4357-ba4f-95ee1e20c464.png
21:27
<Steve Hicks>
but maybe that's what you're getting at - the first entry wouldn't restore anything?
21:28
<snek>
get/setAsyncContext being exactly equiv to agent.[[AsyncContext]]=whatever
21:29
<Steve Hicks>
and you're saying you want it to be undefined?
21:30
<snek>
i think that's what i would expect it to be, yes
21:31
<snek>
(updated above to a slightly more rigorous example)
21:32
<Justin Ridgewell>
Updated what?
21:32
<Steve Hicks>
I agree, I think that makes the most sense, but I wonder if that would require an unacceptable amount of function call overhead?
21:32
<snek>
i can't speak for all engines but in v8 at least its a single cpu instruction
21:32
<snek>
Updated what?
sorry, i meant the above screenshot
21:33
<Justin Ridgewell>
There is a change to await that needs to happen to prevent the using scope from leaking out in:
async function f() {
  using _ = v.enter(2);
  await 1;
}
f();
console.log(v.get());
f() synchronous execution ends at the await, and we need to cleanup any using mutations before resuming past the f() call.
21:33
<Justin Ridgewell>
sorry, i meant the above screenshot
There’s a screenshot?
21:34
<snek>
~13 messages up. i also posted a more abstract gist much further above
21:35
<Chengzhong Wu>
For disposable in an async function, only mutations before the first await need to be restored when exiting the function frame. Not all await tho.
21:36
<snek>
oh yeah i was thinking about that before. all subsequent mutations happen within microtasks so they're already restored right?
21:36
<Chengzhong Wu>
they will be overridden in the next task anyway
21:37
<Justin Ridgewell>
Wow, Cinny is missing a huge part of the conversation
21:37
<snek>
i was also playing with this, not that i intend to merge it but if anyone is curious https://chromium-review.googlesource.com/c/v8/v8/+/6020635
21:37
<snek>
sent an image.
yikes
21:38
<shu>
haha love2matrix
21:40
<snek>
at the very least, it seems we're all on the same page that changes would be needed for this. and that there are some concerns around those changes.
21:42
<Chengzhong Wu>
yes, more changes additional to the current spec are needed
21:42
<Chengzhong Wu>
it should be a follow-up proposal
22:51
<Steve Hicks>
My concern with a follow-up proposal is that it would send polyfills back to square 1 - i.e. you couldn't leverage the working earlier-spec'd AsyncContext and get the new behavior. And worse, the polyfill would be impossible without instrumenting every function in the entire program, rather than just async/generators. I know we don't want to block on polyfill feasibility, but that's potentially a pretty big problem for me at least.
22:51
<Steve Hicks>
(As it is, I'm already at a bit of a loss w.r.t. how I'm going to polyfill the context-propagation in MessageChannel...)