Skip to content
This repository was archived by the owner on Jan 25, 2022. It is now read-only.

Editorial: Registered objects hold FinalizationGroups alive#169

Closed
littledan wants to merge 1 commit intotc39:masterfrom
littledan:finalization-group-liveness
Closed

Editorial: Registered objects hold FinalizationGroups alive#169
littledan wants to merge 1 commit intotc39:masterfrom
littledan:finalization-group-liveness

Conversation

@littledan
Copy link
Copy Markdown
Member

Note that the existing logic implies that finalizer callbacks don't
get cancelled just because nothing points to the FinalizationGroup
object.

Closes #66

Note that the existing logic implies that finalizer callbacks don't
get cancelled just because nothing points to the FinalizationGroup
object.

Closes tc39#66
@syg
Copy link
Copy Markdown
Collaborator

syg commented Nov 22, 2019

Edit: reopened, there was some circular referencing of PRs that confused me. Restoring my question:

I'm not sure I understand the reasoning that the existing logic implies registered objects hold FGs alive. Is the reasoning that in Execution, since FGs that hold obj are passed to CleanupFinalizationGroup, that makes FG's identity observable and live? (I don't feel like that's true.) Further, I don't understand, without a normative requirement that hosts must eventually schedule the cleanup callback, how this is observable. My reading of the current Execution text is an implementation can observably collect something (observable via WeakRef.p.deref) but never schedule FG cleanup for a particular FG.

@littledan
Copy link
Copy Markdown
Member Author

I'm not sure I understand the reasoning that the existing logic implies registered objects hold FGs alive. Is the reasoning that in Execution, since FGs that hold obj are passed to CleanupFinalizationGroup, that makes FG's identity observable and live? (I don't feel like that's true.)

Hmm, I suppose this doesn't "observe the Object value". So probably the FG identity itself is not held alive, and this patch should be changed.

However, when it comes to calling the finalizer, there is logic in the specification that currently requires the finalizer to be queued right when the WeakRef goes null, and that we're thinking of changing to require it to be queued at some point in the future (#167).

Further, I don't understand, without a normative requirement that hosts must eventually schedule the cleanup callback, how this is observable

Yeah, I agree with you that the current definition of HostCleanupFinalizationGroup is pretty open, since it comes with the "if possible" caveat on whether CleanupFinalizationGroup will actually be called. In the proposed web semantics, HostCleanupFinalizationGroup is defined as follows:

Queue a task on the garbage collection task source to perform the following step:

  1. Perform ? CleanupFinalizationGroup(finalizationGroup). If this throws an exception, catch it, and report the exception.

I was imagining that this means that, on the web, the callback must be scheduled. (Well, maybe task queues have the flexibility of never actually scheduling? I don't know task queue expectations very well.)

Aside: Based on this thread as well as conversation in top-level await, I get the sense that it can be hard for implementers to find these cross-references. I wonder what we could do editorially to make that more straightforward, cc @annevk @domenic @Ms2ger.

I think the more relevant question than what the spec currently says is, what should the semantics be? In various threads here and offline conversations with @mhofman , @kmiller68 and @syg, I've heard different opinions about how much latitude implementations have about whether they can cancel finalization callbacks if nothing points to the FG's object value. Should this be required or not?

@syg
Copy link
Copy Markdown
Collaborator

syg commented Nov 23, 2019

I think the more relevant question than what the spec currently says is, what should the semantics be?

Indeed. FWIW it seems like V8 is the odd one out right now regarding both this and #168. I chatted with @hotsphink, and in SpiderMonkey, FG's targets keep the FG alive, and hold onto the unregisterToken weakly. I'm not sure about the exact implementation in JSC but ISTM that's what they'd like as well.

V8 does have some concerns about extending the lifetime of FGs, but my personal opinion is the right semantics here is that a FG's targets keep it alive.

@mhofman
Copy link
Copy Markdown
Member

mhofman commented Nov 23, 2019

I think the more relevant question than what the spec currently says is, what should the semantics be?

Indeed. FWIW it seems like V8 is the odd one out right now regarding both this and #168. I chatted with @hotsphink, and in SpiderMonkey, FG's targets keep the FG alive, and hold onto the unregisterToken weakly. I'm not sure about the exact implementation in JSC but ISTM that's what they'd like as well.

V8 does have some concerns about extending the lifetime of FGs, but my personal opinion is the right semantics here is that a FG's targets keep it alive.

I think this is definitely something the spec should clarify one way or another. The fact that an unreferrenced FG does call its callback is observable by the program (by having 2 fg with the same cells, one referenced the other not).

As mentioned in some feedback in #66, developers will expect the callback to be made even if they don't hold onto the FG instance based on the behavior of other Web APIs.

The FG only needs to be kept alive while it still has alive cells that may be collected in the future, or while it only has empty cells until it attempts to yield all the held values (aka cleanup scheduled). If the implementation can determine none of the remaining cells can ever be collected, and it has called the callback with the program not draining the iterator, then it can safely collect the fg and its cells.

I do not believe this would be a costly thing to implement as I managed to do it in my shim, but maybe engines have different constraints.

@annevk
Copy link
Copy Markdown
Member

annevk commented Nov 23, 2019

When there are cross-specification dependencies we'd usually cross-reference the relevant algorithms. This is harder with JavaScript as HTML is one of many possible hosts I suppose (though the only standardized one), but you could still link it as an example perhaps.

@littledan
Copy link
Copy Markdown
Member Author

If we land #167, it implies that this note is not true, that registered objects don't necessarily hold FinalizationGroups alive, and this PR should be closed. Several implementers have said that they either prefer or would accept these semantics.

@littledan
Copy link
Copy Markdown
Member Author

Closing per #169 (comment)

@littledan littledan closed this Feb 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Collection of FinalizationGroup objects

4 participants