mikeal / znode

Bi-directional RPC through any stream.
206 stars 13 forks source link

functions on result objects are never cleaned up #7

Open kumavis opened 6 years ago

kumavis commented 6 years ago

Being able to return objects that are also RPCs is a really neat feature, but i discovered a gotcha.

const rpc = {
  abc: () => {
    return { xyz: () => {} }
  }
}

Every time abc is called, a new remote reference xyz will be created. These references don't have a lifecycle so they can never be cleaned up. Not really anything you can do about it, but worth documenting.

mikeal commented 6 years ago

We could probably fix this by using a WeakMap for the registry of remote references.

thephoenixofthevoid commented 3 years ago

Actually we need to know when the returned proxy methods are garbage collected. As these events are happening in completely different runtimes (event on different machines), WeakMap alone is useless. I see 3 ways how this could be handled:

  1. Using native extensions to count references and in the finalization step send notification, that a method becomes inaccessible and the remote counterpart should be freed as well.
  2. Using WeakRef to do the same. There are some issues, but I did managed to make it work in toy cases.
  3. Introduce the notion of an owner (Rust way?): we need to explicitly add returned object with methods to a context. When we dispose the context, everything in it is disposed as a result (and remote side notified of it to do cleanup). I think it's reasonable to introduce RootContext + Contexts' hierarchy + Async Contexts (using async hooks) and dispose everything that is outside of them.

In real life most of the time returned objects are used in async contexts and should be disposed right after. Whenever the method should outlive async context, it must be explicitly assigned to some owner. So 3 looks like a solution.