capnproto / capnproto-rust

Cap'n Proto for Rust
MIT License
2.02k stars 221 forks source link

Simple blocking (non-async) RPC example over unix domain sockets #232

Open jmaargh opened 2 years ago

jmaargh commented 2 years ago

I was just thinking about whether it would be possible to use Cap'n Proto RPC in a non-async application (in particular, I was interested in using unix domain sockets as transport). I've thrown together a simple example which I'd love to get your thoughts on: gist.

Observations:

Specific questions:

  1. Am I doing anything wrong here? Obviously, this is an exceptionally simple example, so I know that this seeming to work doesn't necessarily validate the approach.
  2. I used async_io::block_on as the executor. I'm no expert, but this seemed to be among the simplest executors available for simple blocking usage (and I was already importing async_io for the Async trait anyways).
  3. Obviously, this was written quickly and a lot of best-practices are missing, but how careful do I need to be about tearing down RpcSystems nicely?

Finally (and this may be better for a different issue), I don't understand why there isn't an option to generate a more idiomatic version of the Server trait? Is it just that it would be too much work? I'm thinking something that would look like

impl Counter for Server {
    fn count(&mut self) -> capnp::capability::Promise<u64, capnp::Error> {
        // ... do something ...
        Promise::ok(0)
    }
}

rather than messing around with Params and Results objects. My guess is that the answer is "you'd just be hiding the boilerplate and potentially doing unnecessary work", but surely it's much more common to want access to all params and results?

dwrensha commented 2 years ago

Thanks for the feedback! I am well aware that the capnp-rpc has a lot of rough edges, and it does help to hear about your specific experiences.

  1. Am I doing anything wrong here? Obviously, this is an exceptionally simple example, so I know that this seeming to work doesn't necessarily validate the approach.
  2. I used async_io::block_on as the executor. I'm no expert, but this seemed to be among the simplest executors available for simple blocking usage (and I was already importing async_io for the Async trait anyways).

I don't see anything obviously wrong with your example code. I agree that it's annoying that you need to select() on the RpcSystem. Usually I end up spawning the RpcSystem future on some separate task so that I don't need to worry about that. Does async_io let you do that? Maybe it would be possible to use futures::executor::LocalPool together with async_io?

  1. Obviously, this was written quickly and a lot of best-practices are missing, but how careful do I need to be about tearing down RpcSystems nicely?

On a client, if all of the calls have completed, there's no harm in dropping the RpcSystem. On a server, dropping an RpcSystem might interrupt some in-progress calls. Those clients would get a Disconnected error.

Finally (and this may be better for a different issue), I don't understand why there isn't an option to generate a more idiomatic version of the Server trait? Is it just that it would be too much work?

What would your idiomatic version look like in the case where the method's return type a struct?

struct CountContext {
   name @0 : String;
  ...
}
interface Counter {
    count @0 () -> (count :UInt64, context: CountContext);
}

To make this feel like idiomatic Rust, I think we would need to figure out an automatic mapping between capnp types and Rust-native types. That would be very cool to have, but it would take a lot of work.

jmaargh commented 2 years ago

I don't see anything obviously wrong with your example code.

Great, thanks so much for looking over it.

I agree that it's annoying that you need to select() on the RpcSystem. Usually I end up spawning the RpcSystem future on some separate task so that I don't need to worry about that. Does async_io let you do that? Maybe it would be possible to use futures::executor::LocalPool together with async_io?

It would definitely be possible to spawn a task, but I was trying to stay as far away from async as possible, ideally so that when any of the library functions return nothing else is left running, hence looking for the simplest possible blocking executor. Perhaps this isn't a sensible goal, but it was a goal I had in mind.

On a client, if all of the calls have completed, there's no harm in dropping the RpcSystem. On a server, dropping an RpcSystem might interrupt some in-progress calls. Those clients would get a Disconnected error.

Thanks, good to know. It would be great if this were added to doc comments

What would your idiomatic version look like in the case where the method's return type a struct?

struct CountContext {
   name @0 : String;
  ...
}
interface Counter {
    count @0 () -> (count :UInt64, context: CountContext);
}

To make this feel like idiomatic Rust, I think we would need to figure out an automatic mapping between capnp types and Rust-native types. That would be very cool to have, but it would take a lot of work.

Yeah, I guess this is where the work would balloon somewhat. My first thought would be to generate rust struct definitions for each capn proto struct. Since all built-in types seem to have direct Rust equivalents, this should always be possible, the members are always going to be rust primitives, String, Vec, or another generated struct.

dwrensha commented 2 years ago

My first thought would be to generate rust struct definitions for each capn proto struct. Since all built-in types seem to have direct Rust equivalents, this should always be possible, the members are always going to be rust primitives, String, Vec, or another generated struct.

See #157 for some work in this direction.