prost icon indicating copy to clipboard operation
prost copied to clipboard

Make `Message` object safe

Open LucioFranco opened this issue 2 years ago • 5 comments

Following up on https://github.com/tokio-rs/prost/issues/9, I think that we can update Message to accept &mut dyn Buf/&mut dyn BufMut. This would allow us to remove the fn generic and make the message trait object safe.

LucioFranco avatar Nov 02 '22 19:11 LucioFranco

cc @danburkert @neoeinstein

LucioFranco avatar Nov 02 '22 19:11 LucioFranco

It looks like technically Message is already object safe—we have a const in the message.rs file to assert that it is safe to put in dyn. However, I am in favor of adding some non-generic functions that can be called through dyn Message. Right now, only clear() and encoded_len() are callable on dyn Message. We should be able to add some object-safe encode functionality to this.

Note: because Message::decode and friends don't take &self or &mut self, those functions still won't be object safe, and will still require Self: Sized constraints in order to allow Message to be object-safe. However, we can add Message::decode_in_place(&mut self, &mut dyn Buf). This would allow us to require that the user provide the Default value and then we would decode on top of that prototype (which is effectively what decode currently does, but we'd need a version of merge that was non-generic):

let mut message = MyMessage::default();
message.decode_in_place(&mut buffer);

There will likely be some performance penalty if we remove the generic members, as then the largely inline-able functions on the Buf and BufMut impls will become function calls. If we can keep both, then we can offer flexibility or speed, according to the consumer's preference.

An alternate, that would be more disruptive, is to split Message into two traits: EncodeProto<B> where B: BufMut and DecodeProto<B> where B: Buf. By moving the generic up, these traits could still be object safe if the consumer knows what type of buffer they are going to be encoding to or decoding from.

neoeinstein avatar Nov 07 '22 04:11 neoeinstein

I wonder if encode/decode should become free fn and that would allow decode to be obj safe? Splitting the generics is an idea though not sure how useful in practice.

LucioFranco avatar Nov 07 '22 15:11 LucioFranco

Hi,

I'm currently blocked by this. Here is my use case. I want to provide a trait that looks like the following:

trait Command: prost::Message {}

trait MessageBus {
    fn send(&mut self, cmd: &dyn Command) -> Result<()>;
}

You said it, Message is indeed obejct-safe, but it's not very useful if you are restricted to only 2 method calls, and can't even encode a message through a dyn Message.

My only workaround right now is to make send generic, but then I have the same problem I'm trying to avoid, users won't be able to call send() through a dyn MessageBus

oktal avatar Jan 21 '23 14:01 oktal

I have the same problem and found a workaround wich is based of the "Type Erasure" trick described here: https://www.possiblerust.com/pattern/3-things-to-try-when-you-can-t-make-a-trait-object

Example:

trait EncodableMessage {
    fn encode_to_vec(&self) -> Vec<u8>;
}

impl<M: prost::Message> EncodableMessage for M {
    fn encode_to_vec(&self) -> Vec<u8> {
        self.encode_to_vec()
    }
}

#[derive(::prost::Message)]
pub struct TestStruct {
    #[prost(uint32, tag = "1")]
    pub t1: u32,
}

fn process_message(message: &dyn EncodableMessage) {
    let encoded_data = message.encode_to_vec();
    println!("Encoded data: {:?}", encoded_data);
}

fn main() {
    let test = TestStruct { t1: 42 };
    let trait_object: Box<dyn EncodableMessage> = Box::new(test);

    process_message(trait_object.as_ref());
}

tripplet avatar May 28 '23 13:05 tripplet