Add user-definable hook to prevent objects from being GC'ed

[bptato]

Hi,

I'm considering switching my project's¹ vendored QJS fork to QJS-ng. For this, I would need the following patch for adding a callback that allows cooperation with the QJS garbage collector.

Background: my project is written in Nim, and has a facility to automatically convert Nim objects (managed by the Nim garbage collector) to JS objects as follows:

• If a JS object is already associated with the Nim object, return that object.
• Otherwise, create a new JS object, and associate the JS object with the Nim object.

The problem lies in resource deallocation. The naive idea of using finalizers does not work because of cycles:

• We can try incrementing the JS refcount only. Does not work for obvious reasons (Nim object gets deallocated while JS object is still alive; we get UAF or null deref.)
• We can try incrementing both refcounts. Now the objects will never be collected; we have to implement a cycle collector that somehow communicates with the Nim and QJS cycle collectors too. (Unrealistic)
• We can try incrementing the Nim refcount only. Now the following breaks:

let po = new PlatformObject(); // backed by a Nim object
po.doSomethingInNim(); // doSomethingInNim is a JSCFunction calling the corresponding Nim function.
po.jsField = "abcd"; // not known to Nim; this is set on the JS object
window.platformObjectStore = po; // only stores Nim object!
po = null; // assume GC is now triggered, the JS object is destroyed
console.log(window.platformObjectStore.jsField); // undefined :(

Instead, my solution uses a hook in the QJS GC:

• Again, we only increment the refcount of the Nim object.
• If JS_FreeValue reaches a refcount of 0, call a callback that decreases the backing Nim object's refcount. If this does not free the Nim object, increase the QJS object's refcount and free it when the Nim object is destroyed.

(So basically, ownership after creating a platform object from JS is JS -> Nim. Ownership after JS would have been freed is Nim -> JS.)

The idea remains the same for cycle collection: for all objects that would be deleted, restore them if the callback returns true. Changes to the current cycle collector are:

• We add a second temporary object list, tmp_hook_obj_list.
• In gc_decref, put every object with a callback hook into tmp_hook_obj_list instead of tmp_obj_list.
• In gc_scan, go through every obj of tmp_hook_obj_list:
  • call the hook
  • if hook returns OK, move obj to tmp_obj_list; destruction of this object proceeds as normal
  • if hook returns cancel, move obj and all its descendants back to gc_obj_list (and re-add the refcount to them)

Performance considerations: the cycle collector modification is O(N) complexity where N is the number of objects with a hook. If the hook is not used, the only change is an additional NULL check every time an object is freed, and a NULL check for each object before being placed in tmp_obj_list.

Note: I guess this feature should be mentioned in the manual, I'll add it if this proposal gets accepted at all :)

Footnotes

1. https://sr.ht/~bptato/chawan/ ↩

[bnoordhuis]

For my understanding:

1. there can be multiple JS objects that point to the same Nim object, and

2. when a new JS object is created, you want to increment the Nim object's reference count, and

3. decrement its reference count when the JS object is freed?

Where does the cycle come from? Does the Nim object hold references to one or more of those JS objects? If so, would ephemerons or weak references work for your use case?

[bnoordhuis]

Or, for a different perspective, would lifecycle management work for your program if the JS objects were stored as keys with the Nim object as their value in a WeakMap?

[bptato]

For my understanding:

1. there can be multiple JS objects that point to the same Nim object, and

2. when a new JS object is created, you want to increment the Nim object's reference count, and

3. decrement its reference count when the JS object is freed?

Where does the cycle come from? Does the Nim object hold references to one or more of those JS objects?

1. No. It is a 1:1 relationship; there is only one JS object that points to a Nim object at any time, so JS code can pretend that Nim objects are "real" JS objects.

e.g. this evaluates to true:

document.querySelector("html") === document.querySelector("html")

because querySelector returns the same JS object for both calls.

2. Yes, that is the easy part :)

3. I want to check if the Nim object is still alive when the JS object would be freed. Then, I want to be able to say "no, actually, we cannot free this yet" depending on what the Nim GC says.

The cycle comes from the fact that Nim holds a reference to the JS object created for it, if said JS object exists. But also, that JS object must hold a reference to the Nim object for the case when JS outlives Nim.

It should be (conceptually) a pair of regular strong references that still participates in cycle collection.

If so, would ephemerons or weak references work for your use case?

Putting a weak reference to any side has the problem that whichever side holds the strong reference can go out of scope and be destroyed while still being referenced by the other side.

Or, for a different perspective, would lifecycle management work for your program if the JS objects were stored as keys with the Nim object as their value in a WeakMap?

No, with a WeakMap the JS object could still be collected while the Nim object is alive. Whenever a JS object pair is created for a Nim object, it must stay alive while the Nim object is referenced by Nim code, but also, the Nim object must stay alive while the JS object is referenced by JS code.

To give a more concrete example of what the can_destroy hook enables:

A JS object is created whenever a Nim object is referenced from JS code and there is no JS counterpart of said Nim object yet. This does not necessarily have to happen in a constructor. e.g. say we have an HTML document like this:

<div id=test>hello, world</div>

Also, that there is a querySelector Nim function exposed to JS. Now, if we do this:

/* (1. it is implied that this is in a <script> tag after the previously
 * described HTML.) */
let test = document.querySelector("#test"); /* 2. create new JS object */
console.log(document.querySelector("#test") === test); /* 3. true */
test.remove(); /* 4. detach #test from document; Nim no longer holds a reference,
                * BUT we can still do this: */
console.log(test); /* [object Element] */
test.jscanary = "chirp"; /* add a member to the JS object */
/* 5. and we can re-attach it to the document */
document.querySelector("body").appendChild(test);
/* 6. and delete it in JS */
test = null;
/* 7. and then, magic happens */
console.log(document.querySelector("#test").jscanary); /* "chirp" */
/* 8. clean up */
document.querySelector("#test").remove();

In this example, the objects are created as follows:

1. (Obviously) when the document is parsed, a Nim GC'ed object for test is created. But we do not create a JS object yet, that would be wasteful; most DOM nodes are never referenced from JS, so they do not need a JS counterpart. Refcounts: Nim 1, JS 0
2. An automatically generated wrapper function takes the return value of querySelector and "converts" it to JS. This is a JSCFunction that creates a new JS object, sets the JS object's opaque to the Nim object, registers the JS object as the Nim object's JS counterpart, adds a reference to the Nim object, and returns the JS object. Refcounts: Nim 2, JS 1
3. querySelector is called again, returning the same object as before. In this case, we do not create a new reference, just call JS_DupValue on the same object as before. So when the temporary value goes out of scope after the console.log, it does not really free anything, just decrements the same JS object's refcount. Refcounts: Nim 2, JS 1
4. #test is removed from the document, so Nim unrefs it automatically. However, JS still needs access to it, so JS is not deleted. (e.g. the stringifier still works, and it is implemented in Nim.) Refcounts: Nim 1, JS 1
5. Add the same Nim object that we had removed back to the DOM. This is possible because Nim refcount was still 1 in the previous step. Refcounts: Nim 2, JS 1
6. Delete the JS object. Now the JS refcount reaches 0, so can_destroy is called, which "exchanges" a Nim reference for a JS reference.¹ Refcounts: Nim 1, JS 1
7. Again, call querySelector. We get new reference to the JS object that is still alive (JS refcount here is 2), thus we can print the canary. Then the reference to the JS object goes out of scope, reducing the JS refcount to 1 again. Refcounts: Nim 1, JS 1
8. Finally, #test is removed from the document. When it goes out of scope in QJS, can_destroy is called, setting the Nim object's refcount to 0, so the hook returns true and QJS destroys the object as well. Refcounts: Nim 0, JS 0

Footnotes

1. Note that if Nim refcount were 1 before step 6, it would have been reduced to 0 by can_destroy, immediately deleting the object both in Nim and QJS. ↩

[bnoordhuis]

That example clears it up, thanks. Do I summarize correctly that the ultimate goal is to ensure that mutations to the JS object persist across incarnations?

If you take a different tack you can probably make that work without quickjs changes, by defining JSClassExoticMethods for your objects. That turns them into transparent proxies and lets you store properties on the Nim side. That's how the DOM in Chrome or the vm module in Node.js work, basically.

I'm somewhat reluctant to add a special case to the GC but, if exotic methods don't work for you, there's probably a way to make finalizers general enough to cover your use case.

[bptato]

That example clears it up, thanks. Do I summarize correctly that the ultimate goal is to ensure that mutations to the JS object persist across incarnations?

Yes. Also, I want them to be identical in JS terms. (I can deal with the JS object pointers being different, but only if the scripts do not know of this.)

If you take a different tack you can probably make that work without quickjs changes, by defining JSClassExoticMethods for your objects. That turns them into transparent proxies and lets you store properties on the Nim side.

I originally did not take this approach because it looked like I would have to re-implement GetProperty etc. myself. One idea of how to implement this I've just had is:

1. Create two JS objects instead of one.
2. The first one will be an interface object, this is returned.
3. The second one is an internal object, the Nim object holds a strong ref on this until the Nim finalizer is called.
4. The interface object gets exotics that just call the appropriate operation on the internal object, e.g. for SetProperty/GetProperty etc.
5. When the interface object is finalized, decrement the Nim object's refcount.
6. When the Nim object is finalized, destroy the second object.

...but after further consideration, I think this is broken.

let obj1 = document.querySelector("#obj1");
let obj2 = document.querySelector("#obj2");
obj1.strongref = obj2;
obj2.strongref = obj1;
obj1.remove();
obj2.remove();
obj1 = null;
obj2 = null;
/* Memory leak? */

Please correct me if I'm wrong, but I believe if the cycle collector now tries to free obj1 & obj2, it won't work because the strong refs obj1_Nim -> obj1_JSinternal -> obj2_JSinterface -> obj2_Nim -> obj2_JSinternal -> obj1_JSinterface -> obj1_Nim still exist.

Or if we just cut the internal thing:
obj1_Nim -> obj2_JS -> obj2_Nim -> obj1_JS -> obj1_Nim

It seems very familiar... I don't think this solves the problem :(

Less importantly, I think this would break too:

const x = new WeakMap()
x.set(document.querySelector("html"), "hello, world");
console.log(x.get(document.querySelector("html"))); /* hello, world */

because the interface object is recreated for each conversion.
Perhaps an exotic equals overload could help? But I feel like that wouldn't be very performant...

That's how the DOM in Chrome or the vm module in Node.js work, basically.

I suppose Chrome and Node.js do not use objects from two separate GCs? (I'm not familiar with Chrome/Node.js internals, so please correct me if I'm wrong. In that case I guess I should just go study those codebases :P)

I'm somewhat reluctant to add a special case to the GC but, if exotic methods don't work for you, there's probably a way to make finalizers general enough to cover your use case.

I'd be happy to get this working without that ugly tmp_hook_obj_list handling inside the otherwise rather neat GC :) Unfortunately this is the only solution I could come up with so far that also works.

FWIW, before patching QJS I used to do this:

1. Only add a refcount to the JS object.
2. In the Nim finalizer, check whether the JS object still has a refcount >1. If yes, shallow copy the Nim object into a new object with a refcount of 1, and set that as the JS object's opaque. Otherwise, set the opaque to NULL. Then, unref the JS object.
3. In the JS finalizer, unref the opaque if it is not NULL.

Aside from the obvious inefficiency of unnecessarily copying the Nim object, this almost worked. I say almost, because Nim finalizers don't really support memory allocation inside them, so I was getting random crashes.

The obvious idea is to do something similar with QJS finalizers, but we'd have to figure out a way to restore children. IIRC last time I checked my conclusion was "this is impossible because of how the cycle collector works." But maybe I'm wrong.

[bnoordhuis]

...but after further consideration, I think this is broken.

I believe the internal/external object approach should be memory leak-free if you store the internal object in a WeakMap, keyed on the external object. I'd consider it a bug if that leaks.

I suppose Chrome and Node.js do not use objects from two separate GCs?

They don't. That's a fair point.

(Chrome has oilpan a.k.a. cppgc, an integrated C++ & JS garbage collector. Node.js is a mix of weak references and manual reference counting but may switch to oilpan eventually: nodejs/node#40786)

Unfortunately this is the only solution I could come up with so far that also works

Would it help if finalizers could either:

1. query/retrieve the object's properties so you can evacuate them to the nim side, or

2. nick-of-time revive the object by adding a strong reference?

[bptato]

I believe the internal/external object approach should be memory leak-free if you store the internal object in a WeakMap, keyed on the external object. I'd consider it a bug if that leaks.

Hmm. Currently, this is how I understand it:

The Nim object must hold a strong ref to the internal object, or the internal object could be destroyed while the Nim object still exists.
The external object must hold a strong ref to the Nim object, or the Nim object could be destroyed while the external object still exists.

So when internal indirectly references external, collecting that cycle would involve the Nim object in QJS cycle collection, and that's broken. But I have no WeakMap from external -> internal here, so I'm probably misunderstanding you.

Would it help if finalizers could either:

1. query/retrieve the object's properties so you can evacuate them to the nim side, or

Only if there is a guarantee that the object's properties are still alive; then I could just copy the properties to a new JS object. Though we would still be left with the problem that JS weak refs to the old object would become invalid. (And obviously doing this is sub-optimal concerning performance.)

2. nick-of-time revive the object by adding a strong reference?

That would be the best, yes :) The can_destroy hook in this patch serves the same purpose, so if finalizers could also do this then that would be perfect for my use case.

[bnoordhuis]

The WeakMap is used to break the cycle, so I think it should Just Work(TM), including your other example with the circular dependency.

Apropos reviving in finalizers: I'll need to think about what guarantees quickjs can make, what invariants need to be upheld, etc.