Memory management guidelines / use of unref()

ceymard commented 3 years ago

Hello,

I have come across something curious.

I'm using a webkit web view and have some javascript send stuff to my node code through script-message-received

In the callback I do something akin to :

  let v = recv.getJsValue()
  let val = JSON.parse(v.toJson(0))

  // This is important, we want to allow js from collecting this value since it seems node-gtk does not do so
  // automatically. Or does it ? Should it do it ?
  v.unref()

  this.handleRpc(val)

If I don't call .unref() on the JscValue, it's -- I think -- not collected if I were to believe the increased memory usage of my application. With .unref(), it stays more or less at the same memory size.

Which leads me to the question ; is the whole gobject ref counting to be done by hand or does node-gtk has stuff in place using V8's finalizers in some way ? Are there any "official" guidelines around this ? Should there be something about memory management in the doc ?

Or rather more simply put ; is my .unref() call expected or is this some oversight from node-gtk ?

Great lib either way.

romgrk commented 3 years ago

Hey, thanks for the report. Node-Gtk does use V8 weak references/finalizer callbacks to call either g_object_unref() on GObjects or g_boxed_free()/delete on C structs, so the call to .unref() should be unnecessary. I'm not sure why the value isn't collected, here are a few reasons I can see:

~~GLib doesn't have a concept of weak reference (as understood in JS)~~ I've just seen that GLib does support weak references, but the original author of node-gtk wrote it using toggle references which are only called when the reference is the last one. This obviously doesn't work if there are 2 toggle refs cause they'll wait undefinitely. Normally this is not an issue, but given that you're using values coming from JSC it could be possible that there are other toggle refs.
In certain cases, the GC is not running, I'm not exactly sure why but possibly because we have to do weird things to the event loop in order to integrate GLib & nodejs loops.

Could you try running your application with node --expose-gc, and run it manually at some point with global.gc(), and see if the memory is released? If so, we know it's because the GC isn't running. If not, can you share a reproducible example or something runnnable (a full repo if need be) that I can use to inspect the problem?

ceymard commented 3 years ago

Oh, I had run it with expose-gc and called gc at every callback, this did not change anything.

I think the problem lies more on the gobject/gtk side.

Note that the thing that prompted me to use unref was on the documentation of JSC Values ; https://webkitgtk.org/reference/jsc-glib/stable/JSCValue.html where on the description of a JSC Value, it says « JSCValue represents a reference to a value in a JSCContext. The JSCValue protects the referenced value from being garbage collected. »

I suppose it protects the reference in its JavascriptCore running context, not in gtk/gobject, but still. Maybe there are some non-obvious things going on related to memory in this case ?

romgrk commented 3 years ago

Normally the V8 value is GC'ed and when it is, node-gtk will call g_object_unref() on it. The additional .unref() should not be needed as far as I understand, but if it is, it's because there is an unmatched g_object_ref() somewhere.

The docs for getJsValue says that the value has transfer: none, which means the ownership of the object is not passed to us. IIRC, node-gtk would be reffing the value anyway because it needs to remain alive for the lifetime of the associated V8 value. But I still don't see where the additional ref is.

You can inspect the refcount of the object with gi.System.getMemoryContent(jscValue) (the field will be here), but at this point I probably need to plug all that into a debugger and check what's going on. If you can provide runnable code I'll take a look.

romgrk / node-gtk

Memory management guidelines / use of unref() #282