WeakRef and FinalizationRegistry example

[ptomato]

An embedding must call JS::ClearKeptObjects() and set a callback for the HostCleanupFinalizationRegistry operation in order for WeakRef and FinalizationRegistry to work properly. This example illustrates the minimum that an embedding should do.

[ptomato]

This is one thing I'm not really sure about. I guess we could just clear kept objects and run finalization registry cleanups in the gc() function and the example would still work?

[arai-a]

Given the FinalizationRegistry is defined based on job, I think it's better having some kind of job queue, but yes, it doesn't have to be JS::JobQueue-based one, and just "running the cleanup functions at some time future" is only the requirement here.

Also, for the purpose of example, the regular promise job handling can be omitted (or maybe add some comment that m_queue handling is optional?)

[ptomato]

Yes, this is a good point. I decided to keep the integration with JS::JobQueue but added some comments about it not being necessary if you want a more minimal FinalizationRegistry queue.

[ptomato]

I'm not sure if this is in the right place. Should this be called as part of the job queue also?

[arai-a]

This function does 3 separate things, where they're supposed to happen in different timing in practice:

• (1) perform ClearKeptObjects by JS::ClearKeptObjects to remove obj from [[KeptAlive]] field, so that obj will be collected in the next GC call
  • In practice, this is supposed to happen after executing job (microtask checkpoint also does so after draining job queue), and js::RunJobs does so. Thus, draining job queue by js::RunJobs is another (and more closer to the actual case) option
• (2) perform GC by JS_GC, which collects obj and:
  • set the WeakRef's [[WeakRefTarget]] to EMPTY (https://tc39.es/ecma262/#sec-weakref-execution step 1.a.1)
    • at this point, weakRef.deref() becomes undefined
  • enqueues FinalizationRegistry cleanup job for obj to the job queue (https://tc39.es/ecma262/#sec-weakref-execution step 1.b.ii)
• (3) drain job queue by js::RunJobs, and run the FinalizationRegistry cleanup job
  • at this point valueFinalized is set to "marker"

So, the "GC" name sounds misleading, and I think it's better having 3 separate functions (or 2 if (1) is done by js::RunJobs) for each and make it clearer why each of them are necessary in the example.

If step (2) and (3) above becomes separate function, if (weakRef.deref() !== undefined) throw new Error("WeakRef"); can be put between them, so that it also illustrates what happens at which point.

[ptomato]

This is great advice. I understand it a lot better after your explanation. I made 2 separate functions (one that only calls js::RunJobs and one that only calls JS_GC) and added comments inside the snippet of JS code.

[arai-a]

Thank you for adding example for this!
The basic structure looks good, but splitting the testing function can make the example clearer (see below)

[arai-a]

Given the FinalizationRegistry is defined based on job, I think it's better having some kind of job queue, but yes, it doesn't have to be JS::JobQueue-based one, and just "running the cleanup functions at some time future" is only the requirement here.

Also, for the purpose of example, the regular promise job handling can be omitted (or maybe add some comment that m_queue handling is optional?)

[arai-a]

This function does 3 separate things, where they're supposed to happen in different timing in practice:

• (1) perform ClearKeptObjects by JS::ClearKeptObjects to remove obj from [[KeptAlive]] field, so that obj will be collected in the next GC call
  • In practice, this is supposed to happen after executing job (microtask checkpoint also does so after draining job queue), and js::RunJobs does so. Thus, draining job queue by js::RunJobs is another (and more closer to the actual case) option
• (2) perform GC by JS_GC, which collects obj and:
  • set the WeakRef's [[WeakRefTarget]] to EMPTY (https://tc39.es/ecma262/#sec-weakref-execution step 1.a.1)
    • at this point, weakRef.deref() becomes undefined
  • enqueues FinalizationRegistry cleanup job for obj to the job queue (https://tc39.es/ecma262/#sec-weakref-execution step 1.b.ii)
• (3) drain job queue by js::RunJobs, and run the FinalizationRegistry cleanup job
  • at this point valueFinalized is set to "marker"

So, the "GC" name sounds misleading, and I think it's better having 3 separate functions (or 2 if (1) is done by js::RunJobs) for each and make it clearer why each of them are necessary in the example.

If step (2) and (3) above becomes separate function, if (weakRef.deref() !== undefined) throw new Error("WeakRef"); can be put between them, so that it also illustrates what happens at which point.

[ptomato]

@arai-a Thanks for the review, I think your suggestions really improved the example. Could you take another look at the updated version?

[arai-a]

Thanks! the code looks good.

some nits on the comments.

[ptomato]

@arai-a I think your feedback made the comments clearer, so thanks! Would you like to take another look?

[arai-a]

Sorry for the long delay.
I overlooked this notification.