Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

review: question about when cleanup is allowed #115

Open
ljharb opened this issue May 21, 2019 · 7 comments

Comments

@ljharb
Copy link
Member

commented May 21, 2019

(editor review per #110)

For example, https://tc39.github.io/proposal-weakrefs/#sec-weakmap:

if all references to a ECMAScript object are from a [[Target]] field or internal slot,

(also https://tc39.github.io/proposal-weakrefs/#sec-weakref-execution)

  • suggestion: if all references to a ECMAScript objectif all references to a ECMAScript object being used as a WeakMap key, to clarify it's talking about the key, not the value
  • "the [[Target]] field" - It seems like this should specify the [[Target]] field on a WeakRef object, specifically?
  • "or internal slot" - const p = Promise.resolve(obj); has p referring to obj from an internal slot - if i add obj as a WeakMap key, then but retain a strong reference to p but not to obj, then obj absolutely shouldn't be removed from the WeakMap even if the only reference to it is from an internal slot. Am I misreading this?

Re the last one - it occurs to me that an alternative reading of that is "if all references to a ECMAScript object are from a [[Target]] (field or internal slot),", in which case I'd be confused why both terms are used.

@ljharb ljharb referenced this issue May 21, 2019

Closed

Advance to stage 3 #110

8 of 8 tasks complete
@littledan

This comment has been minimized.

Copy link
Member

commented May 21, 2019

Related discussion: #105

To summarize that thread: @syg, @erights and I seem to agree that we want somewhat weaker guarantees than this sentence currently implies; it's unclear exactly how the guarantee should be worded, but as long as we have a general agreement on direction, we're OK with settling on the final wording during Stage 3.

WeakMaps are defined in terms of internal slots, so I'm not sure what you're getting at. Yes, [[Target]] is written thinking about WeakRefs, but nothing else uses that internal slot name, so it seems equivalent.

@erights

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

As discussed there, the "or internal slot" is just wrong and should be removed. This is independent of the issue of how to allow more collection than naive spec reachability. However we solve that, it'll treat internal slots and other forms of explicit strong pointing (variables, properties) equivalently.

@syg

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

That language is just very confusing, yeah. I think @littledan maybe actually meant something like "[[Target]] field or [[Target]] internal slot", but it reads like "[[Target]] field or any internal slot". If he did mean the latter, then it is simply wrong.

@littledan

This comment has been minimized.

Copy link
Member

commented May 21, 2019

I didn't write this spec text. Anyway, I think we should significantly rework it, as discussed in #105.

@syg

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

Ah, sorry for the misattribution!

@mhofman

This comment has been minimized.

Copy link
Contributor

commented May 21, 2019

I also was confused when I first read that, but understood it as "FinalizationGroup's Cell Record [[Target]] field or WeakRef's [[Target]] internal slot".

@ljharb

This comment has been minimized.

Copy link
Member Author

commented May 21, 2019

That expansion would address my confusion.

littledan added a commit to littledan/proposal-weakrefs that referenced this issue May 28, 2019

Normative: Refine definition of reachability
This attempts to address various issues with how reachability
is defined:
- If one WeakRef object points to an object, which points to
  another one, the whole subgraph may be collected, as raised in
  https://gist.github.com/jimblandy/0014dc11233d2d40df922af850b0489a
- An initial provisional definition of collectability is given:
  >   No possible future execution of the program would observably
  >   reference the object, except through a chain of references which
  >   includes a [[Target]] field or [[Target]] internal slot.
  This is intended to resolve tc39#31 (by concluding that the impact on
  host specifications is minimal, and spec graph reachability does not
  imply liveness) and provide a starting point for tc39#105
- The wording around 'field or internal slot' is clarified, resolving tc39#115

littledan added a commit that referenced this issue May 29, 2019

Normative: Refine definition of reachability
This attempts to address various issues with how reachability
is defined:
- If one WeakRef object points to an object, which points to
  another one, the whole subgraph may be collected, as raised in
  https://gist.github.com/jimblandy/0014dc11233d2d40df922af850b0489a
- An initial provisional definition of collectability is given:
  >   No possible future execution of the program would observably
  >   reference the object, except through a chain of references which
  >   includes a [[Target]] field or [[Target]] internal slot.
  This is intended to resolve #31 (by concluding that the impact on
  host specifications is minimal, and spec graph reachability does not
  imply liveness) and provide a starting point for #105
- The wording around 'field or internal slot' is clarified, resolving #115
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.