search

microsoft / vs-streamjsonrpc

The StreamJsonRpc library offers JSON-RPC 2.0 over any .NET Stream, WebSocket, or Pipe. With bonus support for request cancellation, client proxy generation, and more.
Other
730 stars 149 forks source link

Consider using finalizer and weak references to avoid leaks when proxies are abandoned #1017

Open AArnott opened 5 months ago

AArnott commented 5 months ago

We might be able to allow finalization to recover leaked memory by:

  1. Generate proxies with a finalizer
  2. The finalizer should cause the connection to die (or an RPC marshaled object's resources to be recovered).
  3. If a proxy's backdoor interfaces are accessed such that another proxy or direct JsonRpc object access was obtained, shut off the self-destruct feature since we can no longer be confident that dropping the proxy indicates permanent loss of access to the JSON-RPC connection.
  4. To support proxies that raise events, JsonRpc should only retain a reference to it when there are actually handlers added to these events.

Note this is just a best-effort mitigation. The documentation should still emphasize the importance of disposing proxies.

jacob-vincent-mink commented 5 months ago

@AArnott +1 to this feature. Maybe you can tell me if the following situation below is related...

  1. Application A loads some Plugin.dll in a custom load context against common types (e.g. the DLL has SomeProducer, and implementation of IProducer that every DLL can use)
  2. Application B connects to Application A over RPC and calls a method over RPC that ends up getting an instance of SomeProducer (treated as IProducer everywhere necessary) as a proxy.
  3. Application A can no longer unload Plugin.dll due to references held within StreamJsonRpc (or maybe even lower layers?) by virtue of types from Plugin.dll having been serialized

Attached is a small toy example of the situation (zip of a 7z, sorry). load_context_demo.zip

Related: https://github.com/JamesNK/Newtonsoft.Json/issues/2253 https://github.com/dotnet/runtime/issues/13283

AArnott commented 4 months ago

@jacob-vincent-mink, no, that's not the issue at hand here. This issue isn't about being able to unload assemblies. It's about being able to recover memory in the GC heap occupied by the proxy and maybe even drop the I/O connection itself when the proxy is dropped from all accessibility to the program.

An assembly is not expected to be unloadable after it has been used in RPC, because serializers tend to statically store information about types in that assembly as a perf optimization. There are APIs (at least in .NET 8) that allow us to store information in such a way that it would be discarded when an assembly is unloaded, but that isn't used consistently throughout the stack. I expect it would be a significant effort to achieve what you're describing. If you want that feature, please file a separate issue to track it. I don't expect we'll prioritize it anytime soon though, just to set expectations.