Throw if FinalizationGroupCleanupIterator is iterated outside of the FinalizationGroupCleanupJob

[gsathya]

Leaking the iterator is bad as we could potentially cause the same holding to be iterated twice if the user iterates over a leaked iterator (from a previous turn) and a newly created iterator (in the following turn).

Fixes #29

[mhofman]

My reading of the spec is that it's not possible to iterate over the same holding twice since the corresponding record is removed before being yielded by the iterator.

Since there is cleanupSome, there is probably no reason for the iterator to be used outside of the callback.

My main concern is if the callback is an async function. This change effectively prevents that.

[littledan]

I am comfortable preventing async functions from being used here. A turn may happen in the middle of an async function, and we don't want to prevent more GC work from happening then.

[mhofman]

What do you mean by "prevent more GC work from happening"?

It might be surprising from the developer's point of view to have code like this fail:

finalizationGroup.cleanupSome(async (items) => {
  for (const holding of items) {
    await freeStuff(holding);
  }
});

[littledan]

LGTM. I was picturing punning [[Cells]] or [[FinalizationGroup]] to hold this state, but your approach is cleaner and better.

[littledan]

When will this condition be true? I assumed that this was some vestigial logic for the same case (or is vestigial logic for the shutdown method?)

[gsathya]

In case the iterator is leaked, then the finalization group doesn't need to leak as well. With this condition the finalization group is weakly held and can be collected independent of the iterator.

[littledan]

I don't understand what makes it weakly held. Should we update the "abstract jobs" section to indicate that this is weak somehow?

[gsathya]

Yeah, we should update that too. Both are required.