fix(codec): Enforce encoders/decoders are Sync#84
Conversation
|
Yeah...this will be a bit harder than anticipated... |
|
Yeah, this is the other option. I guess you're in the best position to know which bounds you will be able to satisfy in tonic. For this implementation route we'd also want to add a test that ensures You might also want to add a |
|
@seanmonstar would you be able to take a look at this? |
|
What I'm about to say sounds dumb: for trait objects, the order of additional auto traits matters, and most places write them as |
|
@seanmonstar all i can say is lol |
|
Does this put #82 back on the table? |
|
@NeoLegends we still want to go with this one but need to change the order of the bounds. |
|
@seanmonstar can I get your +1 on this before we merge? |
## Motivation Currently we have a lot of implementations in `linera-views` that don't work on the Web because they require `Sync` bounds. For example, `MapView` and `QueueView` are not usable on the Web. ## Proposal Conditionally use `trait-variant` to add bounds to the traits themselves rather than the implementations. [Futures should ‘always’ be `Sync`](https://internals.rust-lang.org/t/what-shall-sync-mean-across-an-await/12020/18) anyway. `linera_storage_service::client` contains an implementation of `KeyValueStore` that is not `Sync` even though it isn't on the Web, due to a trait object inside `tonic` that [has](hyperium/tonic#81) [caused](hyperium/tonic#82) [some](hyperium/tonic#84) [contention](hyperium/tonic#117). For that implementation, follow the solution [given there](hyperium/tonic#117 (comment)) and use [`sync_wrapper::SyncFuture`](https://docs.rs/sync_wrapper/latest/sync_wrapper/struct.SyncFuture.html) to make the futures `Sync` based on the fact that they can only be used through `Pin<&mut Self>` anyway. Ditto `aws-smithy-async`. ## Test Plan CI. ## Release Plan - Nothing to do / These changes follow the usual release cycle. ## Links - [reviewer checklist](https://github.com/linera-io/linera-protocol/blob/main/CONTRIBUTING.md#reviewer-checklist)
3 participants
cc @NeoLegends
Closes #81