| 04:09 | <littledan> | For a permanently pending promise: wouldn’t terminate For AsyncContext.wrap which isn’t called: it’s not wrap which increments the counter, but rather a separate operation which is called when something is logically “queued” (this must be the case since the wrapped thing can’t decrement the counter since it can be called multiple times) |
| 04:13 | <Mathieu Hofman> | So only wrapped functions in some host queue holds the task alive? That sounds like special treatment for host queues. |
| 04:15 | <Mathieu Hofman> | And a wrapped thing can definitely increment a counter. Once it's executed the counter is decremented. during it execution it can create more wrappers, each incrementing the counter until they're called |
| 04:17 | <Mathieu Hofman> | But if the wrapped function is never called and collected instead, you also need to decrement. That's where GC comes in |
| 04:17 | <littledan> | So only wrapped functions in some host queue holds the task alive? That sounds like special treatment for host queues. |
| 04:18 | <littledan> | Wrap produces multiply callable things |
| 04:18 | <littledan> | Anyway if wrap produced single-shot things, then it also wouldn’t be a reason to depend on GC |
| 04:19 | <littledan> | If something gets leaked, then it just doesn’t have its completion callback called (maybe) |
| 04:19 | <littledan> | Or maybe it is called but this is definitely a bug somewhere if people are depending on it |
| 04:21 | <littledan> | Anyway this differs a lot from async hooks today where gc is the expected thing to call the destroy hook for async await, not backup/cleanup if you have a leak |
| 04:22 | <Mathieu Hofman> | Ok so you want to put the burden of task continuation on the user land for user land queues? It would be surprising to me that the context value could outlive the task unless userland takes explicit action, since that's counter to the transparent nature of this API for code unaware of it. |
| 04:22 | <Mathieu Hofman> | Since you can capture a context without using the AsyncContext API simply by code structure. |
| 04:24 | <littledan> | Sorry, what do you mean by that? |
| 04:24 | <Mathieu Hofman> | You can use plain promises to effectively snapshot the current context |
| 04:25 | <Mathieu Hofman> | That IMO should prevent the task termination even if the promise stays pending (nothing in host queues) |
| 04:26 | <Mathieu Hofman> | But the task should terminate once the promise gets collected |
| 04:27 | <littledan> | Oh yeah I agree. Creating the promise should internally call this increment operation |
| 04:27 | <littledan> | But this is different from wrap—increment makes sense since promises are single-shot |
| 04:28 | <littledan> | I think any mismatch here has to do with multiple senses of “queued” |
| 04:29 | <littledan> | Oh wait, sorry, I think we are talking about the .then operation, rather than creating the promise |
| 04:29 | <littledan> | That is the case where the AsyncContext is captured anyway |
| 04:30 | <Mathieu Hofman> | I remember writing a wrap multishot implementation using only promises. |
| 04:31 | <Mathieu Hofman> | Yes using then |
| 04:31 | <littledan> | Wasn’t that based on multiple then calls? |
| 04:32 | <littledan> | Anyway I need to step back and think about this more; I don’t have a very clear picture right now actually |
| 04:32 | <littledan> | Maybe I am mistaken |
| 04:34 | <littledan> | Oh right—you can restore the context multiple times with .then, but you only get one shot at “being outstanding”—the decrement operation only happens the first time. Maybe wrap could apply the same logic and so there actually isn’t a separate increment/decrement operation. |
| 04:36 | <Mathieu Hofman> | https://github.com/endojs/endo/blob/506a9685b62e5694a6a47a0efa05742e0c91fa71/packages/eventual-send/test/async-contexts/async-attack-tools.js |
| 04:36 | <Mathieu Hofman> | Yeah it relies on recreating a promise and calling then every time |
| 17:22 | <Justin Ridgewell> | In case you're not monitoring PR reviews, I suggested we use an AsyncContext namespace (so, AsyncContext.Local and AsyncContext.Snapshot) in https://github.com/tc39/proposal-async-context/pull/55#pullrequestreview-1415123019 |
| 17:22 | <Justin Ridgewell> | Anyone care to weigh in? |