Skip to content

Commit 786ee38

Browse files
Add memory management document (#115)
* Add memory management document This patch also adds a few clarifications and fixes to the web integration document. * Fix * Fix * Apply suggestions from code review Co-authored-by: Nicolò Ribaudo <[email protected]> * Clarify rAF recursion and weak maps * Some more rewording * Clarification on captureFallbackContext and other tweaks --------- Co-authored-by: Nicolò Ribaudo <[email protected]>
1 parent 77a3c0b commit 786ee38

File tree

2 files changed

+139
-2
lines changed

2 files changed

+139
-2
lines changed

MEMORY-MANAGEMENT.md

+119
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Memory management in AsyncContext
2+
3+
A context (sometimes called a snapshot; or in the spec, "a List of Async Context
4+
Mapping Records") is an immutable map from `AsyncContext.Variable` instances to
5+
arbitrary JS values (or possibly also spec-internal values; see "Using
6+
AsyncContext from web specs" in the main web integration document). Each agent
7+
will have a `[[AsyncContextMapping]]` field which is a context, which we will
8+
sometimes refer to as "the current context".
9+
10+
Given a variable `asyncVar`, which is an instance of `AsyncContext.Variable`,
11+
running `asyncVar.run(value, callback)` will:
12+
1. Create a new context which is a copy of the current context, except that
13+
`asyncVar` maps to `value`. The reference to `value` in the context is
14+
strongly held (not a weak reference), but see the
15+
[weak map section](#weak-maps) below.
16+
2. Set that new context as the current context.
17+
3. Run the callback.
18+
4. Restore the current context to the value it had before step 2.
19+
20+
By itself, this would only allow keeping memory alive implicitly within a call
21+
stack, which would be no different from local variables from a stack frame being
22+
kept alive while a function is running.
23+
24+
However, the thing that makes AsyncContext AsyncContext is that the context can
25+
be propagated across asynchronous operations, which eventually cause tasks or
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.
29+
30+
For many of these async operations (such as `setTimeout` and `.then`), a
31+
callback is run once or multiple times in a task or microtask. In those cases,
32+
the operation can be seen as keeping a strong reference to the callback, and it
33+
will also keep a strong reference to the context that was current at the time
34+
that the API was called to start that operation. When the operation is finished,
35+
that reference will be removed.
36+
37+
For events, we do not store the context in which `addEventListener` is
38+
called (though see the next paragraph on the fallback context). Instead, the
39+
context is propagated from the web API that caused it, if any. For APIs that
40+
cause events to fire asynchronously (e.g. XHR), this would involve storing a
41+
reference to the context when the API is called (e.g. `xhr.send()`), and keeping
42+
it alive until no events can be fired anymore from that asynchronous operation
43+
(e.g. until the XHR request finishes, errors out or is aborted).
44+
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.
63+
64+
The web integration document says that observers (such as MutationObserver,
65+
IntersectionObserver...) would use the registration context for their callbacks;
66+
which means when the observer is constructed, it would store a reference to the
67+
current context, which would never be released while the observer is alive.
68+
However, it seems like it might be possible to change this behavior so the
69+
context is not stored at all for observers; instead, the callbacks would be
70+
called with the empty context.
71+
72+
Although this document and the web integration one describe the context
73+
propagations that must happen due to the browser and JS engine's involvement,
74+
it is also important to have in mind how authors might propagate contexts
75+
implicitly. For example, from the browser's perspective, `requestAnimationFrame`
76+
only keeps the context referenced until the rAF callback is called. However, if
77+
the callback recursively calls `requestAnimationFrame`, which is often the case,
78+
the context is propagated with the callback in the recursion.
79+
80+
## The context as a weak map {#weak-maps}
81+
82+
Values associated to an `AsyncContext.Variable` must be strongly held (not weak
83+
references) because you can do `asyncVar.get()` inside that context and get the
84+
associated value, even if there are no other references to it.
85+
86+
However, the AsyncContext proposal purposefully gives JS code no way to get a
87+
list of the entries, or the `AsyncContext.Variable` keys, in a context. This is
88+
done to maintain encapsulation, but it also has the side effect that it allows
89+
implementing the context as a weak map.
90+
91+
If an `AsyncContext.Variable` key in the context could be GC'd other than
92+
because it's a key in the context, then there is no way for any JS code to be
93+
able to access that key at any future time. At that point, that whole entry in
94+
the context, including its value, could be deleted (or all references could be
95+
made weak). This would be implementing the context as a weak map (see the JS
96+
[`WeakMap`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/WeakMap)
97+
built-in).
98+
99+
In most uses of AsyncContext, we don't expect that `AsyncContext.Variable`s
100+
could become unreachable (i.e. GC-able) while the realm in which it was created
101+
remains alive. This is because most uses would store it in a (JavaScript)
102+
variable at the top level of a script or module, so any exported functions in
103+
the script/module will have it in its scope, and will keep it alive.
104+
105+
However, we do expect a weak map implementation to be useful in cases where a
106+
cross-realm interaction results in `AsyncContext.Variable` keys and object
107+
values of different realms in the same context, since otherwise that could
108+
result in leaking the realm. After all, we expect that in the general case
109+
`AsyncContext.Variable` keys from a realm would map to values that only contain
110+
objects from the same realm. So if the only remaining references to a realm are
111+
from entries in the context which have keys in the realm, the keys will be
112+
unreachable, and so the entries will be deleted.
113+
114+
The proposed JS spec for AsyncContext does not explicitly mandate that the
115+
context must be implemented as a weak map, but that is a possible
116+
implementation. However, garbage collecting weak maps takes a performance hit,
117+
and some folks have previously argued against it for that reason. If you think
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.

WEB-INTEGRATION.md

+20-2
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ since they will implement web APIs. However, the integration with the web
1818
platform is also expected to serve as a model for other APIs in other JavaScript
1919
environments.
2020

21+
For details on the memory management aspects of this proposal, see [this
22+
companion document](./MEMORY-MANAGEMENT.md).
23+
2124
## Background
2225

2326
The AsyncContext proposal allows associating state implicitly
@@ -387,6 +390,10 @@ registration context; that is, the context in which the class is constructed.
387390
- [`ReportingObserver`](https://w3c.github.io/reporting/#reportingobserver)
388391
[\[REPORTING\]](https://w3c.github.io/reporting/)
389392

393+
> TODO: Due to concerns about observers leading to memory leaks, an alternative
394+
> option is to not use the registration context, and instead call the observer's
395+
> callback with the empty context. This is still under discussion.
396+
390397
In some cases it might be useful to expose the causal context for individual
391398
observations, by exposing an `AsyncContext.Snapshot` property on the observation
392399
record. This should be the case for `PerformanceObserver`, where
@@ -432,8 +439,14 @@ The exceptions would be:
432439
- The `popstate` event
433440
- The `message` and `messageerror` events
434441
- All events dispatched on `XMLHttpRequest` or `XMLHttpRequestUpload` objects
435-
- The `error`, `unhandledrejection` and `rejectionhandled` events on the global
436-
object (see below)
442+
- The `unhandledrejection` and `rejectionhandled` events on the global object
443+
(see below)
444+
445+
> TODO: The exact principle for which event listeners are included in this list is still under discussion.
446+
447+
The list above is not meant to be hard-coded in the events machinery as a "is this event part of that list?" check. Instead, the spec text and browser code that fires
448+
each of these individual events would be modified so that it keeps track of the
449+
context in which these events were scheduled (e.g. the context of `window.postMessage` or `xhr.send()`), and so that that context is restored before firing the event.
437450

438451
### Fallback context
439452

@@ -595,6 +608,11 @@ attribute applied to callback types to control this.
595608

596609
## Implicit context propagation
597610

611+
> TODO: This section of the web integration proposal is not as baked as most of
612+
> the rest. Depending on which / how many asynchronous events we end up
613+
> propagating, we might not need to add this into the spec, and we could replace
614+
> it with manual context propagation at a number of places.
615+
598616
While tracking contexts along algorithm data flows is straightforward when it
599617
happens synchronously within a single event loop task, in some cases (such as
600618
for asynchronously dispatched events, or unhandled promise rejections) the

0 commit comments

Comments
 (0)