Skip to content

Commit 8218055

Browse files
committed
Clarification on captureFallbackContext and other tweaks
1 parent f65feb3 commit 8218055

File tree

1 file changed

+23
-19
lines changed

1 file changed

+23
-19
lines changed

MEMORY-MANAGEMENT.md

+23-19
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,9 @@ kept alive while a function is running.
2323

2424
However, the thing that makes AsyncContext AsyncContext is that the context can
2525
be propagated across asynchronous operations, which eventually cause tasks or
26-
microtasks to be enqueued. Some of these operations are defined in TC39, such as
27-
`Promise.prototype.then` and `await`, but most of them are defined in web specs,
28-
such as `setTimeout` and the many other APIs listed above.
26+
microtasks to be enqueued. Some of these operations are defined in ECMA-262,
27+
such as `Promise.prototype.then` and `await`, but most of them are defined in
28+
web specs, such as `setTimeout` and the many other APIs listed above.
2929

3030
For many of these async operations (such as `setTimeout` and `.then`), a
3131
callback is run once or multiple times in a task or microtask. In those cases,
@@ -42,20 +42,24 @@ reference to the context when the API is called (e.g. `xhr.send()`), and keeping
4242
it alive until no events can be fired anymore from that asynchronous operation
4343
(e.g. until the XHR request finishes, errors out or is aborted).
4444

45-
`addEventListener`, however, would need changes with the fallback context
46-
proposal[^1]. The initial version of this proposal would allow
47-
`addEventListener` to associate a context with the listener (only if the current
48-
context contains a key that is set in a call to the fallback context API).
49-
However, rather than storing a whole context, a newer iteration of this proposal
50-
(described in https://github.com/tc39/proposal-async-context/issues/107#issuecomment-2659298381)
51-
would let the API only store the values of one or more `AsyncContext.Variable`s
52-
that would need to be passed to the API. This means that in practice, this
53-
context which would be associated to `addEventListener` would only reference one
54-
or a few objects.
55-
56-
[^1]: This proposal isn't described in any depth in the main web integration
57-
document because the details are still being worked out. See
58-
<https://github.com/tc39/proposal-async-context/issues/107>.
45+
`addEventListener`, however, would need changes if we add the
46+
`EventTarget.captureFallbackContext` API[^1]. With it, the context in which the
47+
passed callback is called also stores the current values of the given
48+
`AsyncContext.Variable`s at the time that `captureFallbackContext` is called,
49+
and any calls to `addEventListener` in that context *will* store those values
50+
alongside the event listener. This will likely leak the values associated to
51+
those variables, and we will need outreach to web platform educators to make
52+
sure that authors understand this, but it's the best solution we've found to
53+
cover one of the goals of this proposal, since the other options we've
54+
considered would cause a lot more leaks.
55+
56+
[^1]: This API isn't described in any depth in the main web integration document
57+
because the details are still being worked out. See
58+
<https://github.com/tc39/proposal-async-context/issues/107>. Note that this
59+
document describes the version described in
60+
[this comment](https://github.com/tc39/proposal-async-context/issues/107#issuecomment-2659298381),
61+
rather than the one in the OP, which would need storing the whole current
62+
context.
5963

6064
The web integration document says that observers (such as MutationObserver,
6165
IntersectionObserver...) would use the registration context for their callbacks;
@@ -111,5 +115,5 @@ The proposed JS spec for AsyncContext does not explicitly mandate that the
111115
context must be implemented as a weak map, but that is a possible
112116
implementation. However, garbage collecting weak maps takes a performance hit,
113117
and some folks have previously argued against it for that reason. If you think
114-
it's important that the context is a weak map, please let us, as well as the
115-
various JS engine implementers, know about it.
118+
it's important that the context is a weak map, please let us know so we can
119+
discuss it with the various JS engine implementers.

0 commit comments

Comments
 (0)