2023-01-05 [13:14:20.0345] MarkM started a talk-through of his investigation into whether to embrace AsyncContext at the SES Strategy Meeting yesterday. We didn’t get close to finishing the review, so this is Part 1, and we expect these to get gradually more refined https://youtu.be/vECr5IDJzpg [13:15:51.0706] See also the PR at https://github.com/endojs/endo/pull/1424 2023-01-12 [08:00:05.0999] > <@kriskowal:matrix.org> MarkM started a talk-through of his investigation into whether to embrace AsyncContext at the SES Strategy Meeting yesterday. We didn’t get close to finishing the review, so this is Part 1, and we expect these to get gradually more refined https://youtu.be/vECr5IDJzpg How has this discussion gone? Any more progress on the subsequent meeting? (Sorry for missing these.) [08:30:09.0379] Second video is https://www.youtube.com/watch?v=28wfHOWCROo [08:31:29.0369] We didn't get much feedback _during_ the meeting, but Mark came up with an interesting attack on membranes: - [Attack](https://github.com/endojs/endo/blob/markm-fluid-scopes/packages/eventual-send/test/async-contexts/test-attack.js) - [Fix](https://github.com/endojs/endo/pull/1424#discussion_r1067570517) [08:33:08.0684] > <@jridgewell:matrix.org> We didn't get much feedback _during_ the meeting, but Mark came up with an interesting attack on membranes: > - [Attack](https://github.com/endojs/endo/blob/markm-fluid-scopes/packages/eventual-send/test/async-contexts/test-attack.js) > - [Fix](https://github.com/endojs/endo/pull/1424#discussion_r1067570517) huh, could you explain what this attack and fix mean for AsyncContext? [08:34:18.0164] Briefly, it means all membranes will need to update to use an `AsyncContext` themselves if they care about preventing this type of communication. If not, it still doesn't allow direct access to to the other's graph, so it's not so terrible. [08:34:35.0144] oh, which kind of communication? [08:35:12.0748] You can communicate 1 bit of information per-call that is not directly seen by the membrane [08:35:27.0162] See my first few paragraphs of the fix [08:35:48.0700] > The crux of this attack is that Alice has received 2 callbacks from Bob (they're actually the same === callback, but run in different fluid var contexts), which she stores. After receiving these callbacks, she can then invoke either. Carol cannot tell from the callback's identity which is invoked. Once invoked, Bob can tell from the fluid var state whether the first callback or second callback is invoked. [08:36:09.0503] (Carol is the membrane) [15:08:17.0500] Justin Ridgewell: Can we talk more about whether `wrap` is Realm-specific? [15:08:23.0469] I think this would significantly complicate any implementation [15:08:49.0818] the use cases for membranes and the types of mitigations don't depend on Realms at all, so I'm not convinced we should do it this way [15:11:02.0786] On second thought, maybe implementation wouldn't be so complex, but this depends a lot on what semantics we adopt when different realms call each other (I honestly don't know what you have in mind) [15:27:23.0316] Having it be per-realm or per-agent isn't important to me, I chose realm because it's easiest to implement in JS, I figured it be easy in C++, and it seems the least objectionable (per the dedent caching semantics) [15:38:52.0472] Given that membranes will need to update to use `AsyncContext` in same-realm anyways, I don't think it matters if we choose to do per-agent (and they'll need to update for both same-realm and cross-realm) 2023-01-13 [07:33:28.0139] Justin Ridgewell: Could you describe the semantics you're thinking of in the cross-realm case? [07:33:39.0903] (or if there's some JS implementation, point me to it) [08:03:48.0893] It would require the polyfill to have access to the other realm [08:09:12.0630] Sure, this is a general thing about polyfills that makes them tend to not be spec-complaint [08:09:32.0103] What are the semantics that you want Realm-separated `wrap` to have at runtime? I just don't know what you're proposing. [08:10:28.0767] like, would Promise.prototype.then do a Realm-separated `wrap` or a cross-Realm one? [08:12:01.0090] without Realm separation, each microtask queue item can have a single word added to it, which is "what is the whole group of AsyncContexts to restore during this microtask queue item". If we're being Realm-specific... I don't actually know what this should contain. [08:13:34.0503] I don’t understand how they would be different. It would still be a single pointer to a map. [08:13:50.0658] OK, but the map would contain only the async contexts that are in the Realm that called `then`? [08:14:43.0611] so, synchronously, you could call `otherRealmContext.get()` and get that value, but it'd be lost when you restore the microtask queue item [08:14:49.0783] that's the observable difference of being realm-separated [08:14:51.0490] right? [08:15:05.0603] Isn’t that intrinsic to then? Like which globals and environment is it going to resume with? [08:16:05.0955] sure but I imagined this being different from that (since synchronously you can `get` multiple Realms' AsyncContext things) [08:16:16.0176] > <@littledan:matrix.org> so, synchronously, you could call `otherRealmContext.get()` and get that value, but it'd be lost when you restore the microtask queue item Is this what you are picturing? [08:17:13.0982] Yes, if we choose per realm. [08:18:42.0503] OK, now I understand. And so what is the advantage of these semantics, of partitioning the AsyncContexts in this way? [08:20:18.0269] > Having it be per-realm or per-agent isn't important to me, I chose realm because it's easiest to implement in JS, I figured it be easy in C++, and it seems the least objectionable (per the dedent caching semantics) [08:20:35.0907] There’s no real benefit to it this way, it’s just easy [08:24:53.0569] One thing it consider is that, in some implementations, this data structure gets a bit complex since you might want to scale up to a large number of `.run` calls nested in each other without that being quadratic (so you don't want to have to clone the whole map for each `.run` call). But I'm not sure whether this argues for Realm-partitioned or global (maybe it's the same?) [08:26:15.0146] oh, also, how should we handle AsyncContexts that aren't in any Realm at all because they're implicitly set by other built-in libraries (e.g., web standards)? [08:26:29.0890] I think it’s much more likely that you’ll have many snapshots happening than many nested runs happening, so I prioritized zero-copy snapshots [08:27:02.0760] yes, snapshots must be extremely cheap, that's for sure [08:27:55.0157] I don't think one thing excludes another. A classic purely functional map (like Clojure has) would work for what I described. [08:28:03.0855] (snapshots are also free there) [08:28:10.0733] Maybe we could add a multi-run behavior where many contexts are set, to avoid N clones of the map? [08:28:29.0865] what do you mean? [08:29:04.0969] oh, a JS-visible API for that? [08:29:14.0925] `AsyncContext.runMultiple([a, val, b, otherVal])` [08:29:46.0345] So `a` and `b` are set without 2 clones [08:29:48.0432] I don't think this would remove the potential demand for multiple calls of run being more efficient. Also I'm speculating here; I can't think of when it'd make sense to do that very deeply [08:30:09.0811] Generally it should be separate pieces of code which ask for different variables (otherwise they should consolidate into one) [08:31:01.0513] > <@littledan:matrix.org> I don't think one thing excludes another. A classic purely functional map (like Clojure has) would work for what I described. The issue with pure linked list approaches that a snapshot prevents GC of prior values on the chain [08:31:31.0666] You’ve had to flatten the chain, but maybe that can happen in a GC step [08:31:32.0122] Yeah, this is why you wouldn't use a linked list but instead a functional map. Will find a link... [08:33:23.0329] https://clojure.org/reference/data_structures#Maps [08:34:15.0361] anyway probably just a JS Map is good enough in practice; I just wouldn't want to rule out using this theoretically more efficient mechanism in case things come up. But I think this would be possible and doesn't relate to the use of Realms. [08:35:03.0174] see Chris Okazaki's classic thesis for an overview of this topic, summary at https://en.wikipedia.org/wiki/Purely_functional_data_structure [08:38:23.0411] > <@littledan:matrix.org> oh, also, how should we handle AsyncContexts that aren't in any Realm at all because they're implicitly set by other built-in libraries (e.g., web standards)? OK, so I want to dig into this part. This is core to my agenda of linking AsyncContext with Yoav Weiss 's work, which is around built-in variables (which I believe should be propagated in the identical way to AsyncContext) [08:53:05.0614] > <@littledan:matrix.org> OK, so I want to dig into this part. This is core to my agenda of linking AsyncContext with Yoav Weiss 's work, which is around built-in variables (which I believe should be propagated in the identical way to AsyncContext) Justin Ridgewell: What do you think? [09:11:09.0229] 🤷‍♂️. Is we specify the storage as per-agent, do we need to do any further work to integrate the browser context? [09:37:15.0776] yeah I'd prefer per-agent; if we do per-Realm, I'm not sure how it should work [09:38:11.0291] I guess you convinced me that it wouldn't be harder to implement per-Realm and that the semantics can be defined; now this is potentially the thing to rest the decision on [12:33:23.0388] Just got out of a meeting with the SES folks, and they're supportive of the proposal now [12:34:03.0943] Well, MM in particular, but of the 2 meetings I've had with SES and OCAP folks, there aren't any objections raised [12:35:45.0547] > <@jridgewell:matrix.org> Just got out of a meeting with the SES folks, and they're supportive of the proposal now Wow, that's great! [12:36:00.0689] Could you elaborate a little more on their thoughts? [12:36:14.0824] Did it rest on the per-Realm vs cross-Realm aspect we were discussing above? [12:37:04.0042] No, not at all, they're looking at it from the OCAP language model [12:37:11.0649] cool [12:37:33.0022] While this does give new powers (and there's an implicit message passing), they're ok with it. [12:38:01.0920] I think we'll need to discuss with the browser folks to decide on realm-ness [12:42:43.0898] 🌶️ Also Crockford wants us to stop adding new features to the language [12:43:03.0252] Crockford wants us to take features away, really. [12:43:30.0951] Oh, he was at friam! Of course he was. [12:44:06.0880] Yup [12:44:21.0831] We'll have a video of it uploaded eventually [13:10:52.0602] > <@kriskowal:matrix.org> Crockford wants us to take features away, really. this is in general, not with respect to AsyncContext? [13:11:10.0409] Yes. [13:11:22.0616] > <@jridgewell:matrix.org> I think we'll need to discuss with the browser folks to decide on realm-ness OK, I think we should start this conversation with Yoav Weiss , let's try to organize a call [13:11:24.0554] Crockford has gone from specifically grumpy to generally grumpy. [13:12:44.0249] > <@kriskowal:matrix.org> Crockford has gone from specifically grumpy to generally grumpy. I am not sure whether/why I should care what Crockford thinks at this point [13:14:10.0121] Same as anyone else and in the same way. :-) [13:16:47.0494] > <@kriskowal:matrix.org> Oh, he was at friam! Of course he was. what is this? [13:18:57.0311] There was an ocap research group meeting at a company called Agorics in the 90s. The company was acquired by Microsoft and the team scattered, but has continued to meet every Friday morning ever since. Folks as noteworthy as the late Carl Hewitt were regulars at Friam. [13:19:46.0519] Might have started at Xeorx PARC. I’m not sure. [13:20:21.0702] oh, this was the audience that Justin brought this proposal to? [13:21:03.0658] Yeah, MarkM wanted to share the same material from the SES meeting with this group for more eyes. [13:21:48.0985] SGTM, are there further eyes in the ocap world we should be seeking here, or is this considered the highest body? [13:25:09.0176] Idk about highest, but it’s certainly the centermost. [13:55:25.0802] Yah, Mark wanted to discuss with OCAP folks because JS is a subset of the OCAP language model [13:56:03.0896] We didn't dive deeply into the JS semantics, it was pretty general discussion on programming 2023-01-14 [19:09:24.0860] > <@littledan:matrix.org> anyway probably just a JS Map is good enough in practice; I just wouldn't want to rule out using this theoretically more efficient mechanism in case things come up. But I think this would be possible and doesn't relate to the use of Realms. After looking at this a bunch, I believe the current `Map` based approach is ideal: - O(1) `wrap()` - O(1) `get()` - O(n) `run()` - Automatic GC [19:10:29.0155] I can't come up with another solution that don't have O(n) `run` without sacrificing `wrap` or `get` [19:11:13.0430] I did come up with a slightly optimized solution which has O(1) `run` **if** the current state has not been `wrap`ped. [19:58:16.0988] https://github.com/legendecas/proposal-async-context/pull/15 2023-01-15 [06:39:22.0439] I agree that Map is a good approach for the polyfill and initial implementations, but a good quality of the current proposal is that it'd also be friendly to an implementation which is O(log n) for all three of those operations, using a persistent map instead of a mutable one. [08:48:41.0814] > <@jridgewell:matrix.org> Well, MM in particular, but of the 2 meetings I've had with SES and OCAP folks, there aren't any objections raised This is great! Thank you Justin for pushing this forward! [08:49:31.0931] Unfortunately, I've always had hard time to join the SES meeting as the meeting time is not comfortable for me at UTC+8. [08:52:27.0123] > <@littledan:matrix.org> OK, I think we should start this conversation with Yoav Weiss , let's try to organize a call Is Yoav active on matrix? Seems like Yoav didn't send messages in the room. 2023-01-17 [23:56:00.0990] FYI: https://twitter.com/jarredsumner/status/1614646182269313025 [12:58:52.0969] native AsyncContext should be more efficient than AsyncLocalStorage because it hooks into the microtask queue in a less flexible way (just storing a pointer, rather than calling a callback [12:59:31.0842] > <@legendecas:matrix.org> Is Yoav active on matrix? Seems like Yoav didn't send messages in the room. It might work better to send a small group message (since this room has too much traffic), or otherwise we could shift to email. I've talked to him on Matrix before. [14:37:25.0500] The implementation of ALS that we have in workers now is quite a bit more efficient than Node.js' but still has a fairly sizable cost due to the way promise hooks are implement [14:39:07.0312] Specifically, not only do we have to handle the promise hook for init, pre and post callback, and resolve, for every promise we have to allocate an object to store as a private field since v8 does not give us any mechanism for storing additional context on the `Promise` object (i.e. we can't dedicate an aligned internal fields to use) [14:43:52.0594] For each Promise, the object we allocate is a opaque wrapper around a ref to the ref-counted "async context frame" that is storing the relevant context [14:44:24.0492] So we do need more than storing a simple pointer but there is tons of room for improvement still [14:54:02.0711] yeah a naive built-in implementation would address these issues [14:55:53.0825] absolutely doesn't need to be much 2023-01-18 [17:05:29.0529] I'm surprised you need to do so much work to integrate it [17:06:06.0522] The spec has a hook designed for this usecase, and it seems to exist in V8: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/api/api.cc;l=6837-6853;drc=dd38db94df728f36d61d5d6c943156dbe37144a4 [17:07:27.0216] I spotted that previously but couldn't find any detail whatsoever on what/how it's used [17:09:13.0400] specifically, on the promise hook callback, for the init event, we create an opaque JS object that wraps the reference to the current async context frame. We then set that as a private field on the Promise object. What's not clear in any way (I couldn't find any documentation or examples) if Set/GetContinuationPreservedEmbedderData could be used for that [17:10:08.0459] specifically, it's not clear *when* we would call `SetContinuationPreservedEmbedderData` and what effect that would have [17:14:47.0001] If I'm reading this right, I would call it in `Set…` inside `AsyncContext.run()`, and call `Get…` inside `AsyncContext.get()` [17:15:19.0904] - https://source.chromium.org/search?q=continuation_preserved_embedder_data%20-filepath:%5Eout%2F&ss=chromium%2Fchromium%2Fsrc - Preserved on jobs: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/promise-misc.tq;l=75-90;drc=39c3a97e848a7ecd1fa95e738771cc61d6d72552 - Restored on run: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/builtins-microtask-queue-gen.cc;l=238-245?q=continuation_preserved_embedder_data%20-filepath:%5Eout%2F&ss=chromium%2Fchromium%2Fsrc [17:16:14.0535] Looks like Yoav might be using it in the new Chrome TaskAttribution work: https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/scheduler/task_attribution_tracker_impl.cc;l=233-289;drc=71630a0e336a703e21de9ebeb98a5abf84e8c96c [17:18:24.0463] Still not super clear but it's worth exploring to see if it does make it easier [17:19:06.0578] one key issue is that we need the context tracking to also work with things like timers [17:19:46.0175] What other times do you provide? [17:20:03.0317] so long as we can preserve the context with non-v8 things and it all just works, then hopefully this could make things easier. I'll play with it [17:20:40.0429] Just setTimeout and setInterval but v8's only involvement there is that we call the function when the timer expires [17:20:49.0237] I think if you port https://docs.google.com/presentation/d/1yw4d0ca6v2Z2Vmrnac9E9XJFlC872LDQ4GFR17QdRzk/edit#slide=id.g18e6eaa50e1_0_192 into C++, and everywhere you access `__storage__` you call `Get…`, and everywhere you set you call `Set…`, then it'd work correctly [17:20:59.0171] we capture a reference to the current frame and enter it when we invoke the callback [17:22:23.0503] ok, I'll play with this a bit. If we can eliminate the promise hook I'll be really quite happy [17:25:32.0952] It doesn't look like anything else uses the embedder continuation data, so I'm not sure if `setTimeout` would propagate correctly. [17:25:42.0536] It might be easy to do manually, though [05:52:39.0030] > <@jridgewell:matrix.org> The spec has a hook designed for this usecase, and it seems to exist in V8: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/api/api.cc;l=6837-6853;drc=dd38db94df728f36d61d5d6c943156dbe37144a4 Oh wow I didn’t know about this API [05:57:20.0170] Would be cool to see if someone can build AsyncContext as a Node native module just based on that, or if there are limitations [12:30:54.0553] `SCOPING.md` has the following: [12:31:02.0246] > Only code with direct access to `AsyncContext` instances can modify the value, and only for code execution nested inside a new `context.run()`. [12:32:46.0777] Doesn't `AsyncContext.wrap` allow any code to modify the value of all `AsyncContext` instances in the callback's context? [12:33:41.0514] Yes and no [12:34:15.0880] The `AsyncContext` instance can't contain a value unless that value is placed there by someone with access to the instance [12:34:47.0672] `undefined` (the default state of an `AsyncContext`) is just a "value placed there by someone with access" [12:35:24.0126] So restoring the state to a prior one just places a value into the `AsyncContext` that was already placed there by someone with access [12:36:04.0664] Mark Miller's equates this more to a closed over variable [12:37:14.0325] If I have a opaque `Map` (that I can't directly acccess, but which can be access by `AsyncContext` instances) which holds the data, then all I've done is propagate my closed-over map for you to read from. [12:37:59.0766] - https://github.com/endojs/endo/blob/markm-fluid-scopes/packages/eventual-send/src/async-contexts/7-fluid-passing-style-transform.md - https://github.com/endojs/endo/blob/markm-fluid-scopes/packages/eventual-send/src/async-contexts/7-fluid-passing-style.js [12:40:36.0900] I guess to connect these things, the modification is well-behaved [12:40:49.0937] yes, you definitely can restore things via `wrap`, that's the whole point, that is a well-behaved modification [12:41:03.0996] so maybe this could be clarified in the wording of SCOPING.md [12:41:50.0736] yeah, in that sense `wrap` doesn't introduce any dynamic scoping, so it's just a matter of wording that [12:41:56.0033] Just a quick update here. I've implemented use of this api to store context for workers (https://github.com/cloudflare/workerd/pull/282).... it works well for promises in general but does have a limitation. [12:42:25.0124] Specifically, I'm currently not able to completely eliminate the use of the promise hook because the v8 api does not make the context propagation available for the unhandledrejection event handlers [12:42:50.0948] I'm not familiar with programming language theory, and there's a lot of stuff I'm missing here [12:42:57.0857] I end up having to still associate the current context frame with the promise in the promise hook and entering the frame manually when emitting those events [12:43:32.0509] > <@jasnell:matrix.org> Specifically, I'm currently not able to completely eliminate the use of the promise hook because the v8 api does not make the context propagation available for the unhandledrejection event handlers yep, that doesn't surprise me... this is what you need the init hook for, right? [12:43:51.0517] but it's great to hear if everything but the init hook works!!! [12:44:09.0461] Second, I am capturing the async context when `queueMicrotask()` is called so that the context at the time queueMicrotask is called to schedule the callback is entered. v8 does not appear to propagate the stored context when invoking those scheduled tasks [12:44:36.0365] > <@abotella:igalia.com> I'm not familiar with programming language theory, and there's a lot of stuff I'm missing here Andreu, I recommend you make PRs to SCOPING.md to make things clearer for you, and we can iterate on it in code review. You shouldn't have to be an expert in PL theory to read our docs. [12:44:39.0206] but otherwise, the v8 api is quite helpful. Thank you for pointing it out Justin Ridgewell [12:45:03.0366] > <@jasnell:matrix.org> Second, I am capturing the async context when `queueMicrotask()` is called so that the context at the time queueMicrotask is called to schedule the callback is entered. v8 does not appear to propagate the stored context when invoking those scheduled tasks Sorry, could you explain this a little more? [12:45:09.0394] well, that was a bit more about Justin Ridgewell's response [12:45:46.0917] ``` als.run(123, () => queueMicrotask(() => console.log(als.getStore()))); ``` [12:45:53.0257] the thing is, Mark Miller's school of PL theory is really not widely understood... it is just his small community [12:46:26.0922] I've implemented it such that when the scheduled microtask is run, `als.getStore()` returns the appropriate value [12:46:32.0493] > <@jasnell:matrix.org> ``` > als.run(123, () => queueMicrotask(() => console.log(als.getStore()))); > ``` huh, I thought that would be the whole point of V8's API, to propagate over exactly that pattern... if it doesn't handle this, what does it do? [12:47:32.0579] I don't know. I could be doing something wrong here but in local testing it wasn't picking up the value [12:47:41.0583] I can play with it more [12:48:40.0397] did you get it to work at all, in any case? for example, maybe it works for promises but not calls to queueMicrotask? [12:49:26.0130] > <@littledan:matrix.org> the thing is, Mark Miller's school of PL theory is really not widely understood... it is just his small community Mark asked to update the proposal docs to reference his explanations, which I'm a little worried about. 😬 [12:50:08.0816] > <@jridgewell:matrix.org> Mark asked to update the proposal docs to reference his explanations, which I'm a little worried about. 😬 reference like link to, or put everything in his terms? [12:50:24.0880] They may not be integrating with the job queue correctly [12:50:45.0000] The intention of the hooks is for that to work without modifications, but they're only calling it for promises currently [12:50:59.0641] I'm not sure how Web APIs implement their calls [12:51:11.0977] Redo the explanation in his terms [12:53:30.0773] yeah, definitely doesn't appear to be working [12:53:38.0052] > <@jridgewell:matrix.org> Redo the explanation in his terms Ah, yeah, I think we should decline this request. The main explanation should be intuitive; we could have a doc off to the side in their terms [12:53:49.0671] > <@jasnell:matrix.org> Specifically, I'm currently not able to completely eliminate the use of the promise hook because the v8 api does not make the context propagation available for the unhandledrejection event handlers Looks like that's actually possible with `SetPromiseRejectCallback`: - https://source.chromium.org/chromium/chromium/src/+/main:v8/src/api/api.cc;l=9491-9495;drc=c53c026e6ee5083d71e1cd63607ecbae0c641678 - called by https://source.chromium.org/chromium/chromium/src/+/main:v8/src/runtime/runtime-promise.cc [12:54:27.0763] > <@jridgewell:matrix.org> Looks like that's actually possible with `SetPromiseRejectCallback`: > - https://source.chromium.org/chromium/chromium/src/+/main:v8/src/api/api.cc;l=9491-9495;drc=c53c026e6ee5083d71e1cd63607ecbae0c641678 > - called by https://source.chromium.org/chromium/chromium/src/+/main:v8/src/runtime/runtime-promise.cc Justin, that's one side (where you read the relevant asynccontext) but there's the other side separately (in the promise init hook, when you have to stash exactly that information) [12:55:35.0099] > <@jasnell:matrix.org> yeah, definitely doesn't appear to be working Yoav told me that, for web APIs, he added extra logic to save and restore the right thing when queueing [macro]tasks [12:55:57.0146] so this is why I'm wondering if, in your testing, the save and restore at least happened for promises (it'd be really surprisng to me if it didn't work) [12:56:48.0576] Will the promise reject callback be invoked while the appropriate stored continuation value is current? [12:57:06.0481] if so, I can absolutely capture it then [12:57:40.0243] littledan: You're sometimes replying to a thread on the main channel. Are you using an outdated Matrix client? [12:57:54.0990] I'm using app.element.io... I don't see threads [12:58:13.0684] how do I enable them? [12:58:50.0901] oh, they're not actually enabled by default in recent Element versions, I just had manually enabled them [12:58:58.0271] All Settings / Labs [12:59:23.0383] Yes, it should be [12:59:28.0438] oh! yes, quick test appears that it does work! woo! [13:00:20.0135] > <@littledan:matrix.org> Ah, yeah, I think we should decline this request. The main explanation should be intuitive; we could have a doc off to the side in their terms That's exactly what I said. Lol. [13:01:13.0363] Oh hi now I can see the thread [13:01:33.0272] > Yoav told me that, for web APIs, he added extra logic to save and restore the right thing when queueing [macro]tasks Justin Ridgewell in v8 or somewhere else? [13:02:01.0800] if in v8, it might be in a newer version than we're using maybe? [13:02:17.0716] Hm? [13:02:22.0365] > <@jasnell:matrix.org> Justin Ridgewell in v8 or somewhere else? This is in Chrome, not V8 [13:02:32.0817] so you won't just pick it up [13:02:37.0617] ok yeah [13:03:09.0484] > <@jasnell:matrix.org> Will the promise reject callback be invoked while the appropriate stored continuation value is current? I don't expect this to work at all... I think we'd have to add something specifically to promises code to store exactly this thing [13:03:33.0996] Well, it does appear to be working in local testing [13:03:35.0979] this is why it's important that we document that unhandled rejection asynccontext is important! [13:03:40.0340] huh really? how so? [13:03:47.0602] Could you share your tests? [13:04:38.0562] ``` jsg::AsyncContextFrame::Scope scope(js, jsg::AsyncContextFrame::current(js)); auto ev = jsg::alloc(event, kj::mv(promise), kj::mv(value)); dispatchEventImpl(js, kj::mv(ev)); ``` It's a bit obscure here but... `jsg::AsyncContextFrame::current()` is a wrapper around code that grabs the current stored continuation data from v8::Context [13:06:14.0180] In JavaScript... ``` const { AsyncLocalStorage } = async_hooks; const als = new AsyncLocalStorage(); addEventListener('unhandledrejection', (event) => { // Here we are making sure that the async context properly travels along with // unhandled rejections. if (event.reason === 'boom' && als.getStore() !== 123) { throw new Error("Incorrect async store value"); } if (event.reason === 'boom2' && als.getStore() !== 'ABC') { throw new Error("Incorrect async store value"); } }); addEventListener('rejectionhandled', (event) => { if (als.getStore() !== 'ABC') { throw new Error("Incorrect async store value"); } }); export default { async fetch(request) { // The async context is captured when on(...) is called to register // handlers. als.run(123, () => { Promise.reject("boom"); }); const p = als.run('ABC', () => { return Promise.reject("boom2"); }); await scheduler.wait(1); p.catch(() => {}); return new Response("ok"); } } ``` [13:06:50.0856] Try one that adopts the state of a rejected promise [13:06:56.0442] That'll be the tricky case [13:07:20.0161] `Promise.reject` is a sync rejection signal, so it's very easy to capture the current run state [13:07:21.0379] oh.. no, yeah, shoot [13:08:04.0550] spoke too soon. That case was failing [13:10:07.0592] boo ok [13:16:50.0047] Curious of your test case [13:17:00.0136] The spec flows work, maybe it's just a V8 impl bug [13:17:38.0598] The reject is either called sync (in which case, the context is still there), or it's queued in a job callback, in which case it would have been restored before calling reject [13:18:07.0543] I'll see if I can dig in a bit deeper soon [13:20:19.0134] ```js new Promise((_, rej) => rej(1)); new Promise(res => res(Promise.reject(1))); // technically equivalent to the previous one Promise.resolve().then(() => Promise.reject(1)); ``` [14:20:19.0381] FYI I asked about this API in the #jsc room of WebKit Slack, and Jared Sumner responded explaining that he's implementing AsyncLocalStorage in Bun. Hopefully he takes my offer to come and hang out over here :) [14:23:21.0356] fun fact: Promise.reject is in fact the hard case, since you actually want to wait for the microtask queue to be exhausted before considering it truly an unhandled rejection [14:23:38.0257] the async case where the rejection happens after the responses have already been chained on--that's the easy case [14:24:16.0505] (this was quite tricky for people inside of Bloomberg to rediscover, at least) [14:29:00.0441] For the following case, what would you expect the `als.getStore()` value to be printed.. ``` addEventListener('unhandledrejection', () =>{ console.log(als.getStore()); }) let reject; const p2 = als.run(123, () => new Promise((a,b) => reject = b)); als.run(321, () => reject()); ``` [14:30:29.0403] Ahh, I hadn't considered that `reject()` would be called in a different context [14:32:46.0685] I don't have an answer for this, it's not a usecase that we expose [14:33:06.0378] All user code is guaranteed to run inside the only context we care about [14:34:37.0627] not sure I grok that... in Node.js, `als.getStore()` in the unhandledrejection handler prints `123` [14:34:56.0801] because it grabs the async context reference on promise init and not promise reject [14:36:00.0617] I think that's reasonable, but the other way actually allows more expressiveness [14:36:31.0363] If you wanted to preserve the context for the `reject()`, you would export `return wrap(reject)` [14:37:05.0998] yeah, I'm a bit torn because honestly I do think it should return `321` in this case [14:37:12.0043] I wonder if node had a use case to choose init-time [14:37:40.0985] I'm not entirely convinced this case was considered [14:38:10.0371] I'm going to open an issue in Node.js to discuss [14:38:21.0252] 👍️, please link [14:42:04.0220] > <@jasnell:matrix.org> For the following case, what would you expect the `als.getStore()` value to be printed.. > > ``` > addEventListener('unhandledrejection', () =>{ > console.log(als.getStore()); > }) > > let reject; > const p2 = als.run(123, () => new Promise((a,b) => reject = b)); > als.run(321, () => reject()); > ``` I would say, 123. I guess I'd predict that the V8 API that Justin linked to would log undefined. [14:42:24.0840] This is actually important for Bloomberg's (admittedly weird) use case [14:42:38.0744] https://github.com/nodejs/node/issues/46262 [14:43:28.0628] I think it'd log `321`, no? [14:44:36.0846] `reject` there is https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promise-reject-functions, which immediately invokes https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-rejectpromise, which triggers the hook API [14:45:09.0471] Yeah, it logs `321` [14:46:34.0072] You would need to store the current context until the actual `unhandledrejection` event is fired, but you can do that as part of the hook [14:47:17.0344] > https://github.com/nodejs/node/issues/46262 Nit: "promise is resolved" should say "promise is settled" [14:47:58.0046] yeah, this is exactly the thing that I raised a week or two ago; I'm glad we're coming back to it since apparently we were talking past each other then [14:48:45.0205] I was arguing, we should include something about the kInit storage in the spec (even though it's not visible without the unhandled rejection tracking) [14:48:46.0175] My intuition is that what you really want is the context at the point either `resolve()` or `reject()` is called [14:49:02.0804] I remember. I think we should specify this as part of the proposal, I'm just interested in how we can implement this for workerd given what we already have [14:49:26.0449] it's trivial for me to make it work either way now. [14:49:41.0305] currently I have the Promise hook in place to set the context on kInit [14:50:00.0325] but I have a pending set of changes that eliminates the promise hook and captures it on the rejection handler, which is working great [14:52:39.0912] > <@jridgewell:matrix.org> I remember. I think we should specify this as part of the proposal, I'm just interested in how we can implement this for workerd given what we already have what do the spec sections that you cited have to do with workerd? [14:52:52.0914] > <@jasnell:matrix.org> but I have a pending set of changes that eliminates the promise hook and captures it on the rejection handler, which is working great wait, does this work to implement either set of semantics, or just the 321 one? [14:53:47.0375] it's either or, not both [14:54:02.0754] my pending changes removes the promise hook [14:54:25.0788] workerd uses v8, and v8 exposes these hooks for implementers. [14:55:01.0925] I think we should specify a behavior instead of allowing implementers to implement on their own [14:55:16.0354] For this case, I think we have to [14:55:38.0249] will this be a publicly visible PR? [14:55:45.0980] yes [14:56:14.0272] https://github.com/cloudflare/workerd/pull/282 [14:58:16.0430] Specifically, I think by default, we should capture the context on settle not kInit. So the answer by default should be `321`. If you want `123`, then use `wrap` around `reject` as suggested by Justin. Or, if we are using a mechanism like Node.js' `AsyncResource`, we can do something like: ``` class Foo extends AsyncResource { constructor() { this.deferredPromise = createDeferredPromise(); } resolve() { this.runInAsyncScope(() => this.deferredPromise.resolve()) } reject() { this.runInAsyncScope(() => this.deferredPromise.reject()) } } ``` [14:59:04.0192] This gives us the most flexibility because it allows us either option [15:00:45.0119] Can we have a parallel thread about the same question in legendecas's repo? I'd feel more comfortable sharing my thoughts there (since they're more related to Bloomberg's use case than Node stuff) [15:09:34.0078] https://github.com/legendecas/proposal-async-context/issues/16 [15:42:57.0674] This is also an interesting case because `unhandledrejection` will be the only event API that does not preserve the context at the time of registration... [15:44:20.0474] It would indeed be special, but Yoav found that, in DOM, some events naturally wanted to restore the context when they were registered, whereas others should *not* do so and leave in place the context that triggered them [15:44:34.0746] (I don't know details, but when we get back in touch with him, I'm really excited to hear his thoughts.) [15:44:49.0139] this is sort of a third case, to be sure [15:45:15.0840] but I think we agree it's disagreeing somehow, whether we go with 321 or 123 [15:47:40.0824] Going through this, I'm more and more convinced the answer should be `321` [15:48:35.0236] if we're saying that the context is rightfully captured when the `then()` is called, or when `await` is called.. invoking an unhandledrejection handler falls into the equivalent category of "continuation handler"... it's just a different kind of continuation [15:53:14.0744] I need to pull in an expert in how Bloomberg's system currently works before we make a call for the proposal in general; this whole thing might be unusable to us if the answer is 321 2023-01-19 [16:00:39.0735] I'd be curious as to why. Even if the answer is `321` by default, you should still be able to get `123` as the result by wrapping `resolve` ``` // should print 123 addEventListener('unhandledrejection', () => console.log(als.getStore())); let reject; als.run(123, () => new Promise((a,b) => reject = AsyncResource.bind(b))); als.run(321, () => reject()); ``` [16:01:56.0810] yeah, I guess so, I'd just rather this be based on analyzing how our codebase works today to double-check, rather than just hand-waving [16:02:20.0054] yep, understood [16:02:23.0423] (and I feel like we need an analogous analysis for the Node ecosystem) [16:21:37.0355] > <@littledan:matrix.org> It would indeed be special, but Yoav found that, in DOM, some events naturally wanted to restore the context when they were registered, whereas others should *not* do so and leave in place the context that triggered them Interesting, I think we definitely need to meet with them to figure out what cases these are [16:22:05.0780] Chengzhong Wu I think is setting up a meeting with Yoav already? [16:22:29.0138] Node.js specifically introduced `EventEmitterAsyncResource` for those kinds of use cases [16:26:02.0915] by default, `emitter.emit('...')` will propagate the context where `emit()` is called. With an `EventEmitterAsyncResource`, however, the context is captured when the emitter is created and that's what is propagated when `emit()` is called [16:27:42.0719] but, unhandledRejection is a third behavior, right? [16:28:14.0029] yep, because it's a singular event on an emitter that otherwise acts like the default [16:29:06.0423] ``` process.on('foo', () => console.log(als.getStore()); // context at emit process.on('unhandledRejection' () => console.log(als.getStore()); // context at promise create ``` [16:29:18.0972] same object, two very different behaviors [16:31:38.0280] makes sense. Ultimately, whether we go with `123` or `321`, `unhandledRejection` is still special, right? [16:31:45.0020] yep [19:05:20.0932] This is a bit of a mischaracterization. Mark asked what Justin thought of the idea of using his work as the explainer. Justin (in my opinion rightly) responded that Mark’s reexplanation is better suited for a different audience. I believe Mark found that argument compelling. [19:13:48.0823] Posted https://youtu.be/KKOn5SepxYI [19:24:39.0329] Oh, Mark also said we should probably use per-Agent storage for the contexts [19:24:59.0914] https://youtu.be/KKOn5SepxYI?t=1950 [23:01:15.0384] > <@littledan:matrix.org> Would be cool to see if someone can build AsyncContext as a Node native module just based on that, or if there are limitations Here's a simple package implementing the proposal: https://github.com/jridgewell/async-context-native [00:30:14.0052] Is it special because the mere fact of rejecting causes a macrotask, as opposed to other macrotasks which have to be triggered from a call to a runtime API? [05:21:58.0514] > <@jridgewell:matrix.org> Here's a simple package implementing the proposal: https://github.com/jridgewell/async-context-native What, is that all??? I refuse to believe it! [05:22:59.0204] So the magic V8 API does literally all of the work? [05:23:20.0759] > <@jridgewell:matrix.org> Oh, Mark also said we should probably use per-Agent storage for the contexts Yay also! [05:26:20.0589] I think the specialness here is unrelated to it being a macro task vs micro or sync, and more that we turn out to want this somewhat weird state restored for unique reasons [05:27:55.0858] Anyway, as Daryl from my team wrote on the local issue, it turns out our use case works fine whether the answer is 123 or 321 (though Daryl shares my intuition that 123 feels nicer, maybe related to the fact that he has spent much more time than me on the current 123 implementation) [08:31:32.0335] I think in the overwhelming majority of cases the promise is going to reject in the appropriate context. The only case where I've seen this issue so far is the deferred reject where the reject is called in a different context. If the model were set up to handle returning `321` in that case, the more usual cases would still resolve correctly. [08:31:57.0833] and it would still be possible to implement it so that `123` was returned if that's really what you wanted [10:06:57.0362] > <@littledan:matrix.org> So the magic V8 API does literally all of the work? Yup. The magic V8 API is the parallel for the hooks the spec already implements (though they don't faithfully implement it everywhere it's needed, hence the `queueMicrotask` not working) [10:07:49.0137] It's also not possible to hook into Node's `unhandledRejection` without completely reimplementing the event queue, but that's a Node issue and not a spec issue. [10:08:38.0188] > <@jridgewell:matrix.org> Yup. The magic V8 API is the parallel for the hooks the spec already implements (though they don't faithfully implement it everywhere it's needed, hence the `queueMicrotask` not working) Would be really cool to describe the extent to which this does and doesn't work in its readme [10:09:06.0102] 👍️, I'll add more details later tonight [10:27:22.0063] So the `321` semantics significantly simplify the implementation? [10:27:32.0985] and this is part of the motivation? [10:28:07.0934] it's hard for me to see the part of it which is more intuitive with 321, but if it's easier to implement (avoiding the kInit hook?) then I'm sold [10:28:33.0899] do we need anything more than the small V8 API? e.g., the resolve hook [10:28:44.0098] err, reject hook [10:29:04.0991] or this is handled by the relevant V8 API being synchronous? [10:29:11.0335] V8 could implement efficiently (all promises will require another slot to hold the context pointer). It's inefficient for embedders to do this [10:29:38.0867] yeah, I'm convinced that either option could be implemented efficiently; this is more about curiousity [10:29:48.0689] what is it that V8 would have to implement, if all we want is 321? [10:31:54.0645] V8 wouldn't need to implement anything, it already has the necessary hooks for us. Embedders will need to capture the context when V8 triggers the ProjectReject hook. [10:32:54.0216] If we want 123 behavior, then V8 needs to store the context on each promise allocation, and provide a method for embedders to access that when they V8 triggers the PromiseReject hook. [10:36:28.0326] right, OK [10:40:28.0692] so this works because the V8 API's SetPromiseRejectCallback is invoked synchronously (giving us all the context we need) [10:46:49.0796] Correct [10:53:16.0072] Yeah, the promise kinit hook is unnecessary if v8 will capture the context for us on promise creation and give an API to retrieve it [10:53:54.0941] right but we're thinking that we don't even need that, if we build consensus around 321 [10:53:57.0469] right? [10:54:42.0543] Correct [10:55:06.0695] There is one additional bit of complexity to support multiple AsyncContext instances. That is, for ALS, the context stored is actually a map of ALS instances to the current values associated with each. [10:55:52.0362] Why's that a problem? [10:55:59.0310] That's minor tho. But it would be convenient if the v8 API allowed me to store an embedder value directly rather than a v8::Value [10:56:05.0592] Not a problem, just a detail [10:56:54.0852] yeah, that's exactly what Justin's code does, I think [10:57:00.0953] definitely necessary! [11:44:11.0724] oh yeah I guess basically V8 should remove the existing API and just provide a higher AsyncContext API (or make it an assertion failure to use them both at once) [11:44:51.0556] for Bloomberg's use case, we really want a C++-accessible API for all of this. I think that would be necessary for the browser-internal cases as well. [11:59:44.0213] > <@littledan:matrix.org> yeah, that's exactly what Justin's code does, I think oh sorry I misunderstood this comment; ignore me [15:17:13.0097] Justin Ridgewell Chengzhong Wu : For the TC39 slides, I want to suggest that we talk about OpenTelemetry rather than React Cache. It is a very real client-side use case, and it's also easy to understand based on the context from the previous example (it's just a more sophisticated version). I honestly still don't understand the React Cache case, and I don't think getting into a self-contained explanation of React would play well with theorists in the audience if we had to go down that rabbit hole. OpenTelemetry will sound very convincing to serious people, on the other hand :) and there are intuitive reasons for why you want to split a span on the client side, which Chengzhong Wu 's library shows. [15:18:22.0017] Sure [15:20:23.0531] could also be fun to explain the relationship to Justin's awesome library above and/or James's ongoing work, but this can also just be in the air without slides [15:48:56.0890] OpenTel would speak to me. I used to work with Yuri Shkuro. [15:58:53.0411] Oh god, thenables are broken! [15:59:35.0632] ```js const ctx = new AsyncContext() const queue = []; const thenable = { then(onRes, _onRej) { queue.push("thenable: " + ctx.get()); onRes(); }, }; const out = ctx.run(1, () => { queue.push("new Promise"); const p = new Promise(res => res(thenable)); queue.push("p.then"); const p2 = p.then(() => thenable); queue.push("p2.then"); return p2.then(() => { queue.push("promise: " + ctx.get()); }); }); queue.push("out.then"); out.then(() => { queue.push("done"); //hook.disable(); console.log(queue); }); ``` [15:59:56.0759] ``` [ 'new Promise', 'p.then', 'p2.then', 'out.then', 'thenable: undefined', 'thenable: undefined', 'promise: 1', 'done' ] ``` 2023-01-20 [16:01:55.0865] Because it uses `EnqueueMicrotask`, which doesn't preserve the context: https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/promise-resolve.tq;l=189;drc=4c61bb3131b7951ed2ed896b4df6110b1e5c072f [17:36:39.0731] Let me know if I should write this; I am happy to do so but don’t have Edit access [18:13:52.0209] Send me your email, and I’ll give you edit access [18:14:43.0684] Or send me notes, and I’ll add them to the deck [18:14:50.0144] I don’t really know anything about open telemetry [18:18:17.0877] Sorry I was out yesterday and now I'm back on work. Catching up with the long discussions. [18:29:56.0535] My email for slides is littledan@chromium.org [18:30:09.0463] Will post outline of presentation edits here in the coming hours [18:31:55.0589] So the question then is, is this fixable with V8 changes? [18:35:50.0365] My notes since legendecas was gone: - SES says we are all good!!!!! And the AsyncContext is per agent, not per Realm. - V8 has an API that does most of the work for us, and it is cheaper than PromiseHooks. It is called SetContinuationPreservedEmbedderData. However this only handles some things and not others, where we will need plumbing. - It actually turns out it makes sense to run the promise unhandled rejections callback in the AsyncContext where the promise was rejected, rather than created. This means we don’t need to instrument the init hook. [18:55:30.0902] I'd add that v8's API will definitely need to be expanded to EnqueueMicrotask in order for thenables to be supported [18:55:43.0875] (And for queueMicrotask) [19:02:03.0537] Yes, this is due to V8 not fully implementing the host hooks [19:02:47.0255] All jobs should carry the `[[HostDefined]]` slot (what they're calling `continuation_preserved_embedder_data`), but currently only Promise jobs do it [19:46:47.0018] queueMicrotask is in the same logical bucket as setTimeout though, isn’t it? Like, everything needs plumbing [19:47:02.0936] But thenables would need built in support otoh [19:48:01.0756] Kinda, and no [19:48:24.0502] Note that the job system in the JS spec has historically contained certain… standing disagreements between layers [19:48:42.0313] The V8 API can do whatever it wants; there is no obligation to match the JS spec [19:49:02.0629] Anyway this is to say that I don’t know what “all jobs” means [19:49:11.0088] `queueMicrotask` seems to be the equivalent of a quick job, and they're not properly implementing the hooks for jobs [19:49:30.0021] `setTimeout` might be a job, but I have no idea how it's implemented [19:49:32.0008] Do you mean literally the function queueMicrotask, or the V8 API used to implement it? [19:49:48.0529] Sorry, I meant `queueMicrotask` [19:49:59.0378] No, `EnqueueMicrotask` [19:50:00.0789] Ugh [19:50:10.0642] Oh! ok, the V8 API [19:50:15.0200] Now I understand better [19:50:26.0822] Yeah I can imagine that we need something there [19:51:30.0848] Specifically, steps 13-15 of https://tc39.es/ecma262/multipage/control-abstraction-objects.html#sec-promise-resolve-functions are all jobs code, but they're implemented in V8 as a single `EnqueueMicrotask` [19:52:07.0273] Node.js tracks `queueMicrotask` with the async resource API: https://github.com/nodejs/node/blob/main/lib/internal/process/task_queues.js#L152 [19:52:15.0461] similar to `setTimeout` [19:52:21.0824] Thenables shouldn't need specific support, the spec for them would work if V8 had fully implemented the job hooks [19:54:32.0989] Please, let’s make our arguments on the basis of what is useful, not on what someone is expected to do based on how the spec is written. The latter is a weaker argument and less relevant. [19:55:31.0156] > <@legendecas:matrix.org> Node.js tracks `queueMicrotask` with the async resource API: https://github.com/nodejs/node/blob/main/lib/internal/process/task_queues.js#L152 This makes sense to me. Though if the V8 API did something fancier, it might be more efficient. [19:56:20.0782] This is a place where we should learn from Yoav’s implementation experience [19:57:49.0775] > Please, let’s make our arguments on the basis of what is useful, not on what someone is expected to do based on how the spec is written Because it prevents an embedder from implementing a feature specifically designed in the spec, I do think it's a problem. [20:03:40.0343] I think if you make an argument on this basis, you will get a flat no. It just isn’t the contract that engines sign up for. [20:04:02.0673] If we make an argument based on case by case utility, it will be more effective [20:05:09.0342] Part of this is, there are some things about the interface Ecma-262 provides which are very wacky and bad; engines don’t actually want to sign up for implementing all of that [20:06:01.0581] So you are asking for more than you need if you say that they should do everything corresponding to that interface [20:07:47.0508] Eg it is fine that there is no actual imperative V8 API to get the time zone and that it does things via libc or ICU [20:08:50.0988] I think we're getting off on a tangent. [20:09:07.0986] The spec will have explicit requirements for `AsyncContext`, and when that exists, the hooks won't really matter [20:09:19.0674] They only matter now because we're trying to shoehorn in `AsyncContext` [20:09:27.0201] It's unfortunate it doesn't work properly [20:09:33.0091] But that'll go away with the real impl [20:09:41.0948] Yes, the spec will have requirements, and we will also design a V8 API 2023-01-23 [09:30:03.0767] > <@littledan:matrix.org> Will post outline of presentation edits here in the coming hours OK, coming in at a bit under 100 hours from when I promised this, here's the edits I'd make to the slides: - Don't you want to use the run() function in your wrap() implementation? (Aside: rename run()) - I think mixing in the part about monkey-patching console.log is a distraction which would be better removed. - I would replace the React Cache example with OpenTelemetry (legendecas can do this as he knows that area much better than me). Really, the OpenTelemetry case of holding a span ID instead of an initial timestamp has exactly the same needs--we should say this explicitly. - Slide 18 is really key (maybe reference the main thread scheduling proposal? maybe not?) but the code doesn't then correspond to anything about prioritization, so it's a little confusing - In general, let's say "async context" intead of "async stack" to reinforce the intuition around the terms. Also "stack" implies linear memory usage for the number of async items, which is definitely not the case here, but definitely is the case with general async stack traces (which is a specific non-goal here, and which we should probably avoid comparing this with in slide 19, as it will probably make browsers uncomfortable) - All the stuff about "the dynamic scoping elephant" should be probably rephrased as "Bonus slides (if time allows): ocap analysis" and then just defer to Mark to say he thought about it and decided it's OK. I expect the committee to continue disagreeing internally on whether this deserves to be called "dynamic scoping", and really, that's all irrelevant [09:30:34.0670] Let me know if you want me to make these edits; by default I would leave this to you two champions, since you'll actually deliver this. But I can help if you don't have time. 2023-01-24 [14:46:26.0126] Shu added a schedule constraint for AsyncContext since he's getting the background on that magical API that Justin found. It's pretty weird, it was implemented in https://chromium-review.googlesource.com/c/v8/v8/+/2005849 but I can't find any usages. [14:47:16.0357] It'd be nice to identify where this implementation falls over for thenables, and whether that's something to fix at the V8 level, or whether it's something for each custom thenable to do something-or-other (I don't know exactly what) [14:48:37.0044] It's also worth pointing out that it should be possible to workaround with thenables by using something like Node.js' `AsyncResource`. Not elegant but it gets the job done. [14:53:37.0187] right, or as it's called in this proposal, `wrap` [14:54:13.0116] so, the question in my mind is: is Justin's example something that V8 should fix, or should it just be fixed by saying, Working As Intended, use `wrap` if you want it to have the other behavior [14:54:26.0698] (this is concretely something we should get back to V8 on) [15:14:58.0796] Yoav’s work is also using this API, and I can’t imagine that Task Attribution is happy with the way micro tasks aren’t properly tracked 2023-01-25 [16:54:46.0893] Justin Ridgewell: Do you have a link to the reference site? I couldn't find it [16:55:02.0199] I definitely expected that Yoav's work would use this [16:55:23.0797] Reference site? [16:57:31.0018] err, callsite [16:58:05.0152] just trying to find Yoav's patches or design docs or anything like that [16:59:16.0429] [third_party/blink/renderer/modules/scheduler/task_attribution_tracker_impl.cc](https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/modules/scheduler/task_attribution_tracker_impl.cc) [16:59:48.0943] ah I see thanks!! [17:00:05.0789] They're using continuation API, and there's no special handling for `enqueueMicrotask` that I can find [17:00:12.0988] So they're falling into the same thenable bug [17:01:02.0075] how is setTimeout handled here? [17:01:03.0841] does it work? [17:01:36.0535] Shu is skeptical of putting too much work into thenables. I don't understand the issue in enough detail to know how it would be solved, or whether it should be solved. [17:11:02.0624] Looks like `setTimeout` is handled on the Blink side [17:11:16.0520] TaskAttribution users: https://source.chromium.org/search?q=%22GetTaskAttributionTracker()%22&ss=chromium%2Fchromium%2Fsrc [17:13:40.0892] OK, good, this is all useful context [17:13:50.0421] thank you [17:14:02.0326] Shu will look into this and be in some level of contact with Yoav [17:14:30.0096]