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

implement this_cap() as a method of Params #473

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

dwrensha
Copy link
Member

Fixes #87.

@dwrensha dwrensha added the breaking change requires version bump label Jan 12, 2024
Copy link

codecov bot commented Jan 12, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (d4d96aa) 52.02% compared to head (3f18f6c) 52.02%.

Files Patch % Lines
capnp-rpc/src/local.rs 70.00% 3 Missing ⚠️
capnp-rpc/src/rpc.rs 70.00% 3 Missing ⚠️
capnp/src/capability.rs 0.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #473   +/-   ##
=======================================
  Coverage   52.02%   52.02%           
=======================================
  Files          69       69           
  Lines       33957    33976   +19     
=======================================
+ Hits        17665    17676   +11     
- Misses      16292    16300    +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@dwrensha
Copy link
Member Author

One downside to this approach: it adds an extra heap allocation on the server side of every remote procedure call, because we do client.add_ref() to populate the this_cap field of Params.

The natural way to avoid that allocation would be to somehow pass a &Box<dyn ClientHook> instead. That could be accomplished by adding a new &Box<dyn ClientHook> parameter to every Server method (i.e. in addition to the existing Params and Results parameters), but doing so would add a lot of boilterplate for end users. A better approach might be to add a new CallContext<'a> struct (similar to what capnproto-c++ has) that holds the Params, Results, and also a &'a Box<dyn ClientHook> representing this_cap.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change requires version bump
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[RPC] add way to get a self-reference from within a method call
1 participant