does XS allow finalizers to be called outside of gcAndFinalize?

[warner]

Part of our sufficiently-deterministic-GC story depends upon finalizers (FinalizationRegistry callbacks) not being called outside of our deliberate gcAndFinalize() sequence. The JS spec requires that they get run in their own turn, but doesn't otherwise constrain their scheduling. For us, the CPU meter consumed by the callback code will count against the vat, which means if 1: organic GC is not consistent across all validators and 2: finalizers could run any time organic GC releases an object, then 3: the CPU meter consumed by a vat will not be consistent across all validators, which will lead very quickly to a consensus failure. We can't effectively wrap the callbacks in an "unmetered box", as we can for GC-sensitive deserialization code.

I'm tracing through the code in XS, and I think finalizers are called from within xsAPI.c fxCleanupFinalizationRegistries(). This is called by fxEndJob, which is called from fxEndHost, which is called from:

• xsModule.c: fxRunImport and Compartment.prototype.import(), which suggests that a dynamic import statement might be able to trigger finalizers
• xsRun.c via a dozen functions like fxRunForOf, which seem to be called from the (giant) main bytecode interpreter loop in that same time

The task is to write a test program which registers an object with a finalizer, deletes it, calls gc(), and then performs various things (dynamic import, for of) to see if they might trigger the finalizer.

[warner]

Peter at Moddable told me that fxEndHost only calls fxEndJob when the JS stack is empty, which means all those fxEndHost calls I found aren't a problem. He says that finalizers should only run when the host application (xsnap) says to, and xsnap only does it in the empty-promise-queue state that we allow through gcAndFinalize(). (note to self: email on 18-Sep-2021 and 21-Sep-2021).

When we move to bringOutYourDead (#3767) we need to examine xsnap to see whether it calls fxEndJob/fxEndHost at the end-of-crank stage outside of an explicit dispatch.bringOutYourDead delivery, and whether this might allow finalizers to run at a surprising time. We might need to tweak xsnap to make sure this doesn't happen. Our goal is to have finalizers run only during the explicit gcAndFinalize() call performed within a dispatch.bringOutYourDead, where metering is disabled.

[warner]

@mhofman I think you're about to be looking at this code for #6795 , could you also see if the (old) concern I raised above is still an issue, and close this if not?

[mhofman]

xsnap only does it in the empty-promise-queue state that we allow through gcAndFinalize()

That is not correct. xsnap-worker will run finalizers after the promise queue has drained, so yes after the user code has completed, but before setImmediate trigger which I believe is when we start running unmetered again. This is why I filed #6795

Our goal is to have finalizers run only during the explicit gcAndFinalize() call performed within a dispatch.bringOutYourDead, where metering is disabled.

I believe this goal is mistaken. We should disable metering of finalizers at the xsnap level, and reflect that by wrapping finalizer callbacks with meterControl.unmetered() so that user-land can assert it's running unmetered as expected. Then there is no strict dependency on when finalizers run.

if the (old) concern I raised above is still an issue

Very much still a concern and the same issue I identified in #6795. We should close one or the other as duplicate.

[mhofman]

Our goal is to have finalizers run only during the explicit gcAndFinalize() call performed within a dispatch.bringOutYourDead, where metering is disabled.

I believe this goal is mistaken. We should disable metering of finalizers at the xsnap level, and reflect that by wrapping finalizer callbacks with meterControl.unmetered() so that user-land can assert it's running unmetered as expected. Then there is no strict dependency on when finalizers run.

Note that with the workaround about WeakRef being considered strong while we investigate #6784, we effectively have finalizers run only during bringOutYourDead through gcAndFinalize. Technically the first delivery after a snapshot may have finalizers run too, since snapshotting forces a full GC, but since we now force bringOutYourDead before snapshot, there should be no more finalization targets found empty in the snapshot forced gc.

if 1: organic GC is not consistent across all validators

For now we have decided to expect that all validators have the same organic gc schedule, and thus we will not fix the lack of metering on finalizers for now. However we know how we could do it, and that is documented in #6795