Skip to content

Conversation

Twey
Copy link
Contributor

@Twey Twey commented Jun 4, 2025

Motivation

The boxed Decoder and Body inside Streaming need not be Sync since #804. Unfortunately, that makes Streaming non-Sync, meaning that all the generated tonic futures cannot be awaited in Sync futures, i.e. it regresses #81.

Solution

In fact, the only times we use self.decoder or self.body, we have a &mut unique reference to it, so we are guaranteed not to require synchronization.

The sync_wrapper crate encodes this reasoning, allowing us to safely make the Streaming type Sync regardless of whether the contained Decoder and Body are Sync or not.

@Twey Twey force-pushed the sync-streaming branch from c6eb817 to b65a7c7 Compare June 4, 2025 16:14
@Twey Twey marked this pull request as draft June 4, 2025 16:15
@Twey Twey force-pushed the sync-streaming branch from b65a7c7 to 0772ead Compare June 4, 2025 16:22
The boxed `Decoder` inside `Streaming` need not be `Sync` since
hyperium#804.  Unfortunately, that makes
`Streaming` non-`Sync`, meaning that all the generated `tonic` futures
cannot be awaited in `Sync` futures.

In fact, the only times we use the `Decoder`, we have a `&mut` unique
reference to it, so we are guaranteed not to require synchronization.

The `sync_wrapper` crate encodes this reasoning, allowing us to safely
make the `Streaming` type `Sync` regardless of whether the contained
`Decoder` is `Sync` or not.
@LucioFranco
Copy link
Member

Thanks

@LucioFranco LucioFranco merged commit 87d2d4c into hyperium:master Jun 20, 2025
32 of 34 checks passed
dfawley added a commit to dfawley/tonic that referenced this pull request Jul 11, 2025
* chore: Use symbolic link for license file (hyperium#2241)

* chore: Use inline format argument (hyperium#2260)

* chore: Add `flake.nix` (hyperium#2261)

* chore: Fix interop test certs (hyperium#2262)

* chore: Fix interop test certs

* fix bash script:

* fix: tls config overwrite in endpoint (hyperium#2252)

* fix: tls config overwrite in endpoint

PR hyperium#1866 fixed the breaking change introduced in hyperium#1731, but
resets the TLS config without checking if `tls` is set. This
patch resolves the regression and restores expected behaviour.

* fix: cargo fmt whitespace check

---------

Co-authored-by: vigneshwar.sm <[email protected]>
Co-authored-by: Lucio Franco <[email protected]>

* chore(tonic-bench): Fix failing bench (hyperium#2207)

Co-authored-by: Lucio Franco <[email protected]>

* feat: expose creation of HealthService and HealthReporter (hyperium#2251)

* Expose creation of HealthService and HealthReporter

* add default impl for HealthReporter

* [spr] initial version (hyperium#2264)

Created using spr 1.3.6-beta.1

* Revert "[spr] initial version (hyperium#2264)" (hyperium#2265)

* chore: Prepare `v0.13.1` release



Reviewers: 

Pull Request: hyperium#2266

* chore: Disable unused tower feature (hyperium#2270)

* chore(ci): Set RUSTFLAGS only on check job (hyperium#2271)

* chore: Update to webpki-roots 1 (hyperium#2269)

* chore(ci): Update to cargo-check-external-types 0.2 (hyperium#2272)

* chore(ci): Set token permission to read (hyperium#2275)

* chore(test): Remove cargo-machete config (hyperium#2278)

* chore: Remove unused dependency (hyperium#2277)

* feat: add support for tower's load-shed layer (hyperium#2189)

Refs: hyperium#1616

* chore: box Status contents (hyperium#2253) (hyperium#2282)

* chore: box Status contents (hyperium#2253)

* chore: use private into_status method

* chore(doc): Fix outdated limit in comment (hyperium#2297)

* fix outdated limit in comment

This mention was missed when the default changed in hyperium#1335

* Add backquotes

Co-authored-by: tottoto <[email protected]>

---------

Co-authored-by: tottoto <[email protected]>

* feat: Update to prost 0.14 (hyperium#2300)

* chore: Start development of version 0.14 (hyperium#2303)

* chore(build): Make empty client and server modules private (hyperium#2291)

Co-authored-by: Lucio Franco <[email protected]>
Co-authored-by: tottoto <[email protected]>

* chore(test): Simplify skip debug test (hyperium#2305)

* chore(ci): Exclude semver check of unreleased crate (hyperium#2304)

* feat(types): Update error_details.proto to a56cbf3b (hyperium#2286) (hyperium#2306)

* chore(test): Remove unnecessary prost-build dependency (hyperium#2307)

* feat(transport): Allow setting TCP_KEEPINTVL and TCP_KEEPCNT (hyperium#2299)

* Allow setting TCP_KEEPINTVL and TCP_KEEPCNT

* fix windows

* fix clippy windows

* fix(tonic): make `Streaming` `Sync` again (hyperium#2293)

The boxed `Decoder` inside `Streaming` need not be `Sync` since
hyperium#804.  Unfortunately, that makes
`Streaming` non-`Sync`, meaning that all the generated `tonic` futures
cannot be awaited in `Sync` futures.

In fact, the only times we use the `Decoder`, we have a `&mut` unique
reference to it, so we are guaranteed not to require synchronization.

The `sync_wrapper` crate encodes this reasoning, allowing us to safely
make the `Streaming` type `Sync` regardless of whether the contained
`Decoder` is `Sync` or not.

* feat: preserve user-agent header (hyperium#2290)

Co-authored-by: Lucio Franco <[email protected]>

* chore: Make publish script portable (hyperium#2313)

* feat(tls): Add tls handshake timeout support (hyperium#2309)

Co-authored-by: tottoto <[email protected]>

* docs(tonic-build): remove doc-difference between `lib.rs` and Readme (hyperium#2308)

* doc: try to improve the doc-disparity between `tonic-build`'s `lib.rs` and the readme

* Fix indentation

* fix the doctests

* fix a typo and retes an assumption

* Change `compile_fail` to `ignore`

Co-authored-by: tottoto <[email protected]>

---------

Co-authored-by: tottoto <[email protected]>

* Merge changes from next to master branch (hyperium#2315)

Co-authored-by: Arjan Singh Bal <[email protected]>
Co-authored-by: Easwar Swaminathan <[email protected]>

* chore(interop): Update to console 0.16 (hyperium#2318)

* chore(test): Remove unnecessary tokio-stream dependency (hyperium#2311)

* chore(test): Refactor default stubs test (hyperium#2310)

* chore(test): Use tempfile crate to handle temporary file (hyperium#2325)

* chore(grpc): Update to rand 0.9 (hyperium#2324)

---------

Co-authored-by: tottoto <[email protected]>
Co-authored-by: Lucio Franco <[email protected]>
Co-authored-by: Vigneshwar S <[email protected]>
Co-authored-by: vigneshwar.sm <[email protected]>
Co-authored-by: Rafael RL <[email protected]>
Co-authored-by: Leon Hartley <[email protected]>
Co-authored-by: Joe Roback <[email protected]>
Co-authored-by: Raphael Taylor-Davies <[email protected]>
Co-authored-by: Alexis Darrasse <[email protected]>
Co-authored-by: Alex Steele <[email protected]>
Co-authored-by: Kristopher Wuollett <[email protected]>
Co-authored-by: Makro <[email protected]>
Co-authored-by: James Kay <[email protected]>
Co-authored-by: Darren Bolduc <[email protected]>
Co-authored-by: Honsun Zhu <[email protected]>
Co-authored-by: Frank Elsinga <[email protected]>
Co-authored-by: Arjan Singh Bal <[email protected]>
Co-authored-by: Easwar Swaminathan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants