Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

We should document and test the case of sending the same RpcMarshalable object across several top-level RPC methods #1012

Closed
AArnott opened this issue Feb 28, 2024 · 0 comments · Fixed by #1013
Assignees

Comments

@AArnott
Copy link
Member

AArnott commented Feb 28, 2024

Eric Maupin suggested that he might be passing the same RpcMarshalable object across the same RPC connection multiple times. That is, the creator of the real object sends it as a method argument to several methods.

This pattern concerns me, because if the methods run concurrently I think JsonRpc will see the same object reference and store it only once in our marshalable object handle table, and reuse the same handle each time the object appears. On the receiving side, it will see the same handle each time and use the same proxy. Given the docs require the receiver to dispose of a marshalable object, each of these several methods will presumably dispose the object when they are done. But the first method to do this may end up removing the handle from the marshalable tables kept by both sides, zombifying the other proxies.

If so, this behavior is by design. But we should verify that our docs make abundantly clear that when an RpcMarshalable object is sent over RPC, the sender should truly consider that they don't own the object any more and should certainly not send it again.

Let's review the docs and possibly write a test for this scenario.

@AArnott AArnott self-assigned this Mar 5, 2024
AArnott added a commit to AArnott/vs-streamjsonrpc that referenced this issue Mar 5, 2024
AArnott added a commit to AArnott/vs-streamjsonrpc that referenced this issue Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant