tikv / grpc-rs

The gRPC library for Rust built on C Core library and futures
Apache License 2.0
1.81k stars 253 forks source link

CallOption does not have Sync #548

Closed Ten0 closed 3 years ago

Ten0 commented 3 years ago

https://docs.rs/grpcio/0.9.0/grpcio/struct.CallOption.html#impl-Sync

This is weird that we can't clone it from several threads, given that xxx_opt takes ownership when making a call anyway so there may not be any issue within gRPC Core.

What is the technical limitation that prevents us from having Sync on CallOption?

BusyJay commented 3 years ago

I totally agree that it should be Send and Sync. The reason it's not Sync is just we don't impl Sync for it explicilty. Rustc can explain more:

    = help: within `grpcio::CallOption`, the trait `Sync` is not implemented for `*mut grpcio_sys::bindings::grpc_metadata`
    = note: required because it appears within the type `grpcio_sys::bindings::grpc_metadata_array`
    = note: required because it appears within the type `grpcio::Metadata`
    = note: required because it appears within the type `std::option::Option<grpcio::Metadata>`
    = note: required because it appears within the type `grpcio::CallOption`

Implement Send and Sync for Metadata should solve the problem. I'm happy to accept a PR if you want to.