Normative: Add Weakrefs and FinalizationRegistry #2089

codehag · 2020-07-10T07:47:55Z

Specification text for https://github.com/tc39/proposal-weakrefs

This version of the specification is without cleanupSome (see https://github.com/codehag/proposal-cleanup-some), in anticipation of the discussion of the upcoming meeting. Adding this back in would be trivial if we decide against it.

Note: we're adding inline normative optional spec text.

codehag · 2020-07-10T07:54:47Z

spec.html

+        <h1>HostCleanupFinalizationRegistry(_finalizationRegistry_)</h1>
+
+        <p>
+          HostCleanupFinalizationRegistry is an implementation-defined abstract


@syg This might be important for the incumbent realm tracking.

For incumbents, see #2086 and whatwg/html#5722

As both an editor and a delegate, given the reality with Promises, I think it'd be fine to fix up the callbacks in this PR to use JobCallback after #2086 is merged, even for Stage 4. There is no need to gate on that.

codehag · 2020-07-10T08:57:12Z

I am not sure what the linter wants from me, any recommendations?

jmdyck · 2020-07-10T12:44:08Z

I am not sure what the linter wants from me, any recommendations?

There should be a space on either side of each square bracket in that heading, I believe.

bakkot · 2020-07-10T17:53:17Z

Ugh, my apologies for the terrible error messages from ecmarkup. I'll work on that.

I think the error you're seeing in the most recent commit is because of the inclusion of blank lines within s and an <emu-note>. That shouldn't be a hard error (just a lint failure), but the parser is pretty rigid right now.

codehag · 2020-07-10T18:08:52Z

I think I fixed it, should be good now

Edit: narrator: she forgot to commit the patch

ljharb · 2020-07-10T23:27:49Z

Tests: tc39/test262#2192

ljharb

I started to do the suggestions, but it seems like every paragraph is hard-wrapped; it's probably easier to halt my review until those are updated :-)

spec.html

devsnek · 2020-07-10T23:44:10Z

spec.html

+            1. Set _ref_.[[WeakRefTarget]] to ~empty~.
+          1. For each FinalizationRegistry _fg_ such that _fg_.[[Cells]] contains _cell_, such that _cell_.[[WeakRefTarget]] is _obj_, do
+            1. Set _cell_.[[WeakRefTarget]] to ~empty~.
+            1. Optionally, perform ! HostCleanupFinalizationRegistry(_fg_).


The implementation of HostCleanupFinalizationRegistry is already free to not clean things up so I don't think this needs to be optional.

Suggested change

1. Optionally, perform ! HostCleanupFinalizationRegistry(_fg_).

1. Perform ! HostCleanupFinalizationRegistry(_fg_).

I disagree with this suggestion. While it is true that the host hook reserves the right to not do anything, calling out the optionality means hosts don't need to remember to explicitly allow optionality.

Why don't we just explicitly call out the optionality in the hook then? Isn't the entire point of host hooks to abstract this kind of thing?

It's important to call out optionality here to avoid a misunderstanding that it would be mandatory to trigger FinalizationRegistry. The word "optionally" makes the intended interpretation unambiguous, and it does not sacrifice precision/correctness. Remember that specifications are communication devices, not just mathematical artifacts. Because @devsnek's suggestion is an editorial one, we can continue considering it after this PR lands and possibly make the change later.

As an editor I also prefer the "Optionally" phrasing.

devsnek · 2020-07-10T23:45:08Z

spec.html

+        </p>
+
+        <p>
+          Because calling HostCleanupFinalizationRegistry is optional, registered objects


Suggested change

Because calling HostCleanupFinalizationRegistry is optional, registered objects

Because calling CleanupFinalizationRegistry is optional, registered objects

I also disagree with this suggestion for the same reason as above.

spec.html

syg · 2020-07-11T00:29:27Z

spec.html

+            1. Set _ref_.[[WeakRefTarget]] to ~empty~.
+          1. For each FinalizationRegistry _fg_ such that _fg_.[[Cells]] contains _cell_, such that _cell_.[[WeakRefTarget]] is _obj_, do
+            1. Set _cell_.[[WeakRefTarget]] to ~empty~.
+            1. Optionally, perform ! HostCleanupFinalizationRegistry(_fg_).


I disagree with this suggestion. While it is true that the host hook reserves the right to not do anything, calling out the optionality means hosts don't need to remember to explicitly allow optionality.

syg · 2020-07-11T00:29:40Z

spec.html

+        </p>
+
+        <p>
+          Because calling HostCleanupFinalizationRegistry is optional, registered objects


I also disagree with this suggestion for the same reason as above.

spec.html

ljharb

LGTM pending comments

spec.html

syg

Looking very good already, mostly small wording changes now.

spec.html

codehag · 2020-07-14T11:58:47Z

Branch has also been rebased

littledan

This recent commit seems to fix the Realm-specificity bug.

spec.html

gibson042 · 2020-07-25T14:49:28Z

spec.html

+        Colloquially, we say that an individual object is live if every set of objects containing it is live.
+      </emu-note>
+
+      <p>At any point during evaluation, a set of objects _S_ is considered <dfn>live</dfn> if either of the following conditions is met: </p>


Is "live" sufficiently specific to serve as a linkable definition id? It's not used elsewhere now, but I could see it cropping up in prose. Perhaps something like "liveness" (which is already used) would be better?

I think it's reasonable to define "live" this way within the JavaScript specification. I don't think "liveness" is really any more specific. This proposal successfully links with the term "live" where relevant in normative text ("liveness" is just used in notes) and the rest of ecma262 does not seem to use "live" in a conflicting way.

"live" is also the spelling of a common English word (ranked somewhere between 100 and 300 AFAICT) that could easily find a place in the spec, as opposed to "liveness" which will only ever have this meaning.

I think it would be fine to iterate on this in an editorial PR after landing this one.

gibson042 · 2020-07-25T15:02:33Z

spec.html

+
+  <emu-clause id="sec-clear-kept-objects" aoid=ClearKeptObjects>
+    <h1>ClearKeptObjects ( )</h1>
+    <p>The abstract operation ClearKeptObjects takes no arguments. ECMAScript implementations are expected to call ClearKeptObjects when a synchronous sequence of ECMAScript executions completes. It performs the following steps when called:</p>


Given the plausible future addition of Symbols, it would be nice to give these operations initial names that do not directly reference the Object type. "KeptObject" could be replaced with something like "KeptAlive" or "KeptList".

A similar comment was brought up above, and it was agreed that we can change this after the Symbols in WeakMap proposal moved forward. I would rather do this is a group change, as there will be other edits, rather than anticipating an unrelated proposal in this one.

Renaming operations requires introducing oldids, which is a bit of an ugly hack that shouldn't be necessary if the initial names do not include "Object". I'm not suggesting that you make the definitions generic right now, just that the names theirselves are not overly specific.

As the champion of the Symbols as WeakMap keys proposal, I would prefer to not generalize this prematurely. oldids are fine.

devsnek · 2020-07-27T17:30:06Z

I think these need to be added to the intrinsics table and to the "constructor properties of the global object" section.

codehag · 2020-08-03T16:01:40Z

I think I still need one more editor to approve this, so pinging @bakkot and @michaelficarra for one last look before I rebase and squash

bakkot · 2020-08-05T16:26:18Z

spec.html

+    <emu-clause id="sec-weakref-invariants">
+      <h1>Objectives</h1>
+
+      <p> This specification does not make any guarantees that any object will be garbage collected. Objects which are not live may be released after long periods of time, or never at all. For this reason, this specification uses the term "may" when describing behaviour triggered by garbage collection.</p>


Suggested change

 This specification does not make any guarantees that any object will be garbage collected. Objects which are not live may be released after long periods of time, or never at all. For this reason, this specification uses the term "may" when describing behaviour triggered by garbage collection.

This specification does not make any guarantees that any object will be garbage collected. Objects which are not live may be released after long periods of time, or never at all. For this reason, this specification uses the term "may" when describing behaviour triggered by garbage collection.

bakkot

LGTM other than the stray space above.

bakkot · 2020-08-05T16:29:52Z

@codehag We've been letting @ljharb handle the merging process because he has idiosyncratic preferences for how that should happen.

…table (tc39#2126) Unintentionally omitted from tc39#2089. Also added the <dfn>s for easier linking.

(from PR tc39#2089)

(Fixes spec bug from PR tc39#2089)

- Put 2 emu-clause attributes in quotes (We always quote element attributes -- see PR tc39#1868) - Use title-case in clause heading - Tweak some algorithm syntax - Change "a finalization registry object" to "a FinalizationRegistry" (The former term isn't defined.)

- Use title-case in clause heading - Tweak some algorithm syntax - Put 2 emu-clause attributes in quotes (We always quote element attributes -- see PR tc39#1868) - Change "a finalization registry object" to "a FinalizationRegistry" (The former term isn't defined.)

Fixes spec bug from PR tc39#2089.

- Use title-case in clause heading - Tweak some algorithm syntax - Put 2 emu-clause attributes in quotes (We always quote element attributes -- see PR tc39#1868) - Change "a finalization registry object" to "a FinalizationRegistry" (The former term isn't defined.)

Fixes spec bug from PR tc39#2089.

- Use title-case in clause heading - Tweak some algorithm syntax - Put 2 emu-clause attributes in quotes (We always quote element attributes -- see PR tc39#1868) - Change "a finalization registry object" to "a FinalizationRegistry" (The former term isn't defined.)

codehag commented Jul 10, 2020

View reviewed changes

codehag force-pushed the weakrefs-sans-cleanupsome branch from 00ce5e9 to 53fe666 Compare July 10, 2020 08:02

codehag requested review from a team, syg, ljharb, michaelficarra and bakkot July 10, 2020 08:06

codehag force-pushed the weakrefs-sans-cleanupsome branch from 53fe666 to 78fbc63 Compare July 10, 2020 08:42

codehag force-pushed the weakrefs-sans-cleanupsome branch from 78fbc63 to e7bf912 Compare July 10, 2020 09:02

codehag force-pushed the weakrefs-sans-cleanupsome branch from e7bf912 to e6fd679 Compare July 10, 2020 17:28

codehag force-pushed the weakrefs-sans-cleanupsome branch from e6fd679 to 7600747 Compare July 10, 2020 18:07

codehag force-pushed the weakrefs-sans-cleanupsome branch from 7600747 to a65af73 Compare July 10, 2020 18:37

ljharb added the has test262 tests label Jul 10, 2020

ljharb requested changes Jul 10, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

spec.html Outdated Show resolved Hide resolved

devsnek reviewed Jul 10, 2020

View reviewed changes

syg requested changes Jul 11, 2020

View reviewed changes

ljharb reviewed Jul 12, 2020

View reviewed changes

codehag force-pushed the weakrefs-sans-cleanupsome branch from 63bb481 to 913646b Compare July 13, 2020 13:14

ljharb approved these changes Jul 13, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

spec.html Outdated Show resolved Hide resolved

syg reviewed Jul 13, 2020

View reviewed changes

codehag force-pushed the weakrefs-sans-cleanupsome branch from 913646b to fcead87 Compare July 14, 2020 11:57

ljharb added has stage 4 This PR represents a proposal that has achieved stage 4, and is ready to merge. and removed pending stage 4 This proposal has not yet achieved stage 4, but may otherwise be ready to merge. labels Jul 21, 2020

littledan approved these changes Jul 24, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

bakkot reviewed Jul 24, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

bakkot reviewed Jul 24, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

gibson042 reviewed Jul 25, 2020

View reviewed changes

ljharb self-assigned this Aug 3, 2020

ljharb requested review from bakkot and a team August 3, 2020 16:28

bakkot reviewed Aug 5, 2020

View reviewed changes

bakkot approved these changes Aug 5, 2020

View reviewed changes

Normative: Add Weakrefs and FinalizationRegistry (tc39#2089)

71c0897

ljharb force-pushed the weakrefs-sans-cleanupsome branch from fe67804 to 71c0897 Compare August 5, 2020 21:34

ljharb merged commit 71c0897 into tc39:master Aug 5, 2020

ljharb mentioned this pull request Aug 5, 2020

Editorial: add missing WeakRef/Finalization well-known intrinsics to table #2126

Merged

ljharb added a commit to ljharb/ecma262 that referenced this pull request Aug 5, 2020

Editorial: add missing WeakRef/Finalization well-known intrinsics to …

566f39c

…table (tc39#2126) Unintentionally omitted from tc39#2089. Also added the <dfn>s for easier linking.

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Aug 9, 2020

Editorial: Tweak some algorithm syntax

e464887

(from PR tc39#2089)

jmdyck added a commit to jmdyck/ecma262 that referenced this pull request Aug 11, 2020

Editorial: Change _set_ to _set_.[[WeakSetData]]

f34e4f6

(Fixes spec bug from PR tc39#2089)

ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Aug 11, 2020

Editorial: Change _set_ to _set_.[[WeakSetData]] (tc39#2129)

79ee196

Fixes spec bug from PR tc39#2089.

h2oche pushed a commit to h2oche/ecma262 that referenced this pull request Aug 16, 2020

Editorial: Change _set_ to _set_.[[WeakSetData]] (tc39#2129)

fd581f1

Fixes spec bug from PR tc39#2089.

ExE-Boss mentioned this pull request Mar 9, 2021

[jsdom] Use base.d.ts file DefinitelyTyped/DefinitelyTyped#51657

Merged

8 tasks

ExE-Boss mentioned this pull request Mar 26, 2021

[jsdom] Add WeakRef and FinalizationRegistry DefinitelyTyped/DefinitelyTyped#52022

Merged

8 tasks

gibson042 mentioned this pull request Feb 15, 2024

Use of "normative optional" annotation should be more consistent #3280

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Normative: Add Weakrefs and FinalizationRegistry #2089

Normative: Add Weakrefs and FinalizationRegistry #2089

codehag commented Jul 10, 2020 •

edited

Loading

codehag Jul 10, 2020

syg Jul 10, 2020

codehag commented Jul 10, 2020

jmdyck commented Jul 10, 2020

bakkot commented Jul 10, 2020

codehag commented Jul 10, 2020 •

edited

Loading

ljharb commented Jul 10, 2020

ljharb left a comment

devsnek Jul 10, 2020

syg Jul 11, 2020

devsnek Jul 11, 2020

littledan Jul 11, 2020

bakkot Jul 11, 2020

devsnek Jul 10, 2020

syg Jul 11, 2020

syg Jul 11, 2020

syg Jul 11, 2020

ljharb left a comment

syg left a comment

codehag commented Jul 14, 2020

littledan left a comment

gibson042 Jul 25, 2020

littledan Jul 27, 2020 •

edited

Loading

gibson042 Jul 27, 2020

littledan Jul 27, 2020

gibson042 Jul 25, 2020

codehag Jul 27, 2020

gibson042 Jul 27, 2020

littledan Jul 27, 2020

devsnek commented Jul 27, 2020

codehag commented Aug 3, 2020 •

edited

Loading

bakkot Aug 5, 2020

bakkot left a comment

bakkot commented Aug 5, 2020

	1. Optionally, perform ! HostCleanupFinalizationRegistry(_fg_).
	1. Perform ! HostCleanupFinalizationRegistry(_fg_).

	Because calling HostCleanupFinalizationRegistry is optional, registered objects
	Because calling CleanupFinalizationRegistry is optional, registered objects

	<p> This specification does not make any guarantees that any object will be garbage collected. Objects which are not live may be released after long periods of time, or never at all. For this reason, this specification uses the term "may" when describing behaviour triggered by garbage collection.</p>
	<p>This specification does not make any guarantees that any object will be garbage collected. Objects which are not live may be released after long periods of time, or never at all. For this reason, this specification uses the term "may" when describing behaviour triggered by garbage collection.</p>

Normative: Add Weakrefs and FinalizationRegistry #2089

Normative: Add Weakrefs and FinalizationRegistry #2089

Conversation

codehag commented Jul 10, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codehag commented Jul 10, 2020

jmdyck commented Jul 10, 2020

bakkot commented Jul 10, 2020

codehag commented Jul 10, 2020 • edited Loading

ljharb commented Jul 10, 2020

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb left a comment

Choose a reason for hiding this comment

syg left a comment

Choose a reason for hiding this comment

codehag commented Jul 14, 2020

littledan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan Jul 27, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

devsnek commented Jul 27, 2020

codehag commented Aug 3, 2020 • edited Loading

Choose a reason for hiding this comment

bakkot left a comment

Choose a reason for hiding this comment

bakkot commented Aug 5, 2020

codehag commented Jul 10, 2020 •

edited

Loading

codehag commented Jul 10, 2020 •

edited

Loading

littledan Jul 27, 2020 •

edited

Loading

codehag commented Aug 3, 2020 •

edited

Loading