Needs discussion / clarification: if cleanup doesn't iterate all weak cells, when will it be called again?

[tc39]

Needs discussion / clarification: if cleanup doesn't iterate all weak cells, when will it be called again?

[marjakh]

Yes. Listing it here for completeness :)

Btw, if the cleanup function doesn't iterate the WeakCells, but the program keeps creating new ones, memory will grow without limit. (Because we store the not-yet-iterated WeakCells.)

[littledan]

Huh, I never quite interpreted RunJobs that way. (But the interpretations that people made of RunJobs to make it correspond to actual JS environments always seemed quite esoteric to me, and I was likely missing something.) I always thought that program termination was a bit like running into resource limits: The environment can always just do it, with the specification saying nothing.

I think there are a lot of JS environments that exit while queues still have items in them (e.g., the web through navigation). This spec compliance issue will be cleared up by tc39/ecma262#1597 , which removes RunJobs and makes it clear that the host schedules jobs.

[littledan]

Yes, the intention is to land tc39/ecma262#1597 before WeakRefs (in part, because HTML integration depends on it).

About whether it's unfortunate to drop callbacks even if your intention is to run to completion: I recommend that, when it's time for the process to exit, that hosts don't wait for finalizers to run, for the reasons explained in #125 . I expect the Web and Node to sometimes skip finalizers in this case.

[marjakh]

There's another option:
3. Whenever we do GC
4. Something else

[fitzgen]

Immediately (i.e., post another cleanup task immediately to the microtask queue, i.e., let other already scheduled microtasks run and call cleanup again)

This has a pathology if the cleanup function doesn't processs anything and no forward progress is made. We wouldn't want to keep enqueuing clean up jobs over and over that don't do anything :-p

[fitzgen]

Another related idea: it may make sense for the reclaimed objects iterator to have a method to clear it out (which one would otherwise have to keep calling .next until it was finished). This would allow consumers to "gracefully" abandon cleanup.

[littledan]

I would suggest that the answer would be, "whenever the engine decides to, whether or not there are additional things collected". So this would permit 0, 1, 2 or 3. Any concerns with giving this sort of flexibility to engines?

[Jamesernator]

I think 1 would make the most sense as you can't be sure that the cleanup callback hasn't scheduled it's own work for when it wants to deal with the iterator. For example maybe the finalization callback is

const finalizer = holdings => requestIdleCallback(() => {
  for (const holding of holdings) {
    /* do some cleanup */
  }
})

or something like that.

[littledan]

I don't think we need much spec text for this issue, but we should come to a shared conclusion by Stage 3.

[littledan]

We promoted WeakRefs to Stage 3 based on the understanding that engines should schedule the finalizer to be re-called at a time that's good for them, based on whatever implementation-dependent data they have. Is there anything further that we should document?

[devsnek]

Why is the behaviour that the group would be re-queued at some point? Doesn't this just cause infinite loops?

[littledan]

@devsnek The callback will keep getting called if the iterator is not iterated through, but it won't get requeued immediately, but rather (in HTML) in another task, with possible further implementation-defined delay. Could you say more what you mean by "infinite loop"?

[devsnek]

@littledan "keep getting called" is what I mean by an infinite loop.

const g = new FinalizationGroup(() => {
  Promise.resolve().then(() => {}); // more items on queue
});

g.register({});

// agent can never exit...

[littledan]

There's lots of ways that JS code can repeatedly call itself. But it sounds like the real problem is exposed in your comment:

// agent can never exit...

As discussed in #125 , I wouldn't recommend designing an agent that way. In general, when an agent exits, then there's no need to run any finalization code--actually, trying to build partial reliability there may lead to more bugs, as finalizers are inherently unreliable: They should not be used for cleanup that must happen. See the README for more information on this unreliability aspect. On the web, I don't expect any finalizers to run when the page is navigated away from, even though this action causes lots of things to not be live anymore.

[devsnek]

@littledan I don't try to run them on exit. it sounds like you expect agents to contain some sort of external unspecified event loop. but within the spec itself, I'm not sure what behaviour is expected besides an agent that doesn't know when to exit.

[mhofman]

@devsnek The callback will keep getting called if the iterator is not iterated through, but it won't get requeued immediately, but rather (in HTML) in another task, with possible further implementation-defined delay. Could you say more what you mean by "infinite loop"?

I actually think the currently implemented behavior is right: if no new cell is found dirty, I don't expect my callback to be invoked. If my program chose not to perform cleanup immediately but rather to schedule it for later, I don't need to be constantly told "you have items pending".

However this doesn't prevent the implementation to call me again if it wishes. Whether thats when a new gc cycle runs, when new empty cells are found in another group, or simply later. A program that schedules a delayed cleanup will have to handle multiple callback invocations anyway.

Basically the behavior 1 is fine, if it also allows 2, 3, 4 and technically 3, but that's arguably pretty spammy of the engine.

The one case where I've argued before the engine should make an effort to reschedule a callback invocation is if the engine decides to not yield all empty cells even though the program asked for them (drained the iterator), but that's not what's discussed here.

[littledan]

@devsnek

it sounds like you expect agents to contain some sort of external unspecified event loop.

What the spec expects, and this is very explicit, is that there's a host hook for finalization, which is expected to call back into a JS algorithm. Where do you get an expectation of more detailed behavior? I was talking about how the web would do things, but I don't see how this relates to an unspecified event loop in general.

but within the spec itself, I'm not sure what behaviour is expected besides an agent that doesn't know when to exit.

I don't know what you mean. Why when would a host be in a state where it "doesn't know when to exit"? How would that relate to the WeakRefs proposal?

Note, we've discussed giving stronger expectations to when the host would call back, and we decided to keep these sorts of guarantees weak.

@mhofman

The one case where I've argued before the engine should make an effort to reschedule a callback invocation is if the engine decides to not yield all empty cells even though the program asked for them (drained the iterator), but that's not what's discussed here.

I believe the current spec text allows engines to do this kind of thing. Do you agree?

[devsnek]

@littledan a host is supposed to requeue when the items aren't handled, and in this example the items are never handled. As far as I can tell the only solution there is a job queue outside the spec that the agent doesn't normatively have to process all items from.

[littledan]

@devsnek I'm having trouble understanding how much of a burden this suggestion for implementations to requeue is. If you could describe a system that doesn't fit into the mold and would have trouble requeueing these callbacks, it'd be easier for me to understand the concrete problem. (I'm not really familiar with the world of JS implementations that don't have a job queue. On the other hand, the current spec does talk about job queues, e.g., for Promises.)

[devsnek]

@littledan concretely, i had to modify engine262 from "while the queue has items" to "while the queue has items and some of those items are not finalization cleanup callbacks", which i think is sub-optimal in a few ways: It's not spec compliant, you miss out on callbacks that could possibly do something meaningful, and the engine has to make a bunch of changes to deal with these broken callbacks, when i think the proper behaviour would be to ignore them.

[littledan]

@devsnek I'm having trouble understanding what you mean. What queue? What is the spec non-compliance? (A link to an issue or PR or patch could make this more clear.)

[devsnek]

@littledan https://tc39.es/ecma262/#job-queue

The spec non-compliance is that RunJobs does not allow you to do something implementation defined (read as exit) while a queue still has items in it.

[devsnek]

@littledan right you can technically exit at any point, the issue is more if your intent is not to exit, but to run to completion. tc39/ecma262#1597 looks interesting, hopefully it is merged before weakrefs.

Like I said above though, I also think it is unfortunate that the callbacks have to be dropped even if your intention is to run to completion.

[devsnek]

@littledan so you're saying I should ignore the items when I want to exit, but I'm saying I want to exit after all the items are run. I realize that in the grand scheme of things there is not much of a reason to run all the items if they're just finalization callbacks. My point is more that it feels weird to have to specialize on this, and that it is not immediately obvious from an implementer perspective. In #60 I also mention some of the user-facing reasons I dislike this.

[littledan]

Maybe we could add more notes to the specification and explainer to make it clear that these callbacks are not expected to be run in the shutdown case. My attempt was #184, but maybe that wasn't clear enough.