Skip to content

Commit

Permalink
chore: deprecate WindowUpdateMode::OnReceive
Browse files Browse the repository at this point in the history
Continuation of #120.

Preparation for #175.

Also removes dead-lock warning for `WindowUpdateMode::OnRead`. With the restructuring of
`Connection::poll`, one reads from the socket when writing is blocked. Thus the deadlock can not
occur.
  • Loading branch information
mxinden committed Nov 23, 2023
1 parent 93b7062 commit 61d510c
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 10 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
# 0.12.1

- Deprecate `WindowUpdateMode::OnReceive`.
It does not enforce flow-control, i.e. breaks backpressure.
Use `WindowUpdateMode::OnRead` instead.
See [PR #XXX](https://github.com/libp2p/rust-yamux/pull/XXX).

# 0.12.0

- Remove `Control` and `ControlledConnection`.
Expand Down
3 changes: 2 additions & 1 deletion yamux/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "yamux"
version = "0.12.0"
version = "0.12.1"
authors = ["Parity Technologies <[email protected]>"]
license = "Apache-2.0 OR MIT"
description = "Multiplexer over reliable, ordered connections"
Expand All @@ -20,3 +20,4 @@ pin-project = "1.1.0"

[dev-dependencies]
quickcheck = "1.0"
futures = { version = "0.3.12", default-features = false, features = ["executor"] }
2 changes: 2 additions & 0 deletions yamux/src/connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -650,6 +650,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin> Active<T> {
shared.window = shared.window.saturating_sub(frame.body_len());
shared.buffer.push(frame.into_body());

#[allow(deprecated)]
if matches!(self.config.window_update_mode, WindowUpdateMode::OnReceive) {
if let Some(credit) = shared.next_window_update() {
shared.window += credit;
Expand Down Expand Up @@ -695,6 +696,7 @@ impl<T: AsyncRead + AsyncWrite + Unpin> Active<T> {
if let Some(w) = shared.reader.take() {
w.wake()
}
#[allow(deprecated)]
if matches!(self.config.window_update_mode, WindowUpdateMode::OnReceive) {
if let Some(credit) = shared.next_window_update() {
shared.window += credit;
Expand Down
2 changes: 2 additions & 0 deletions yamux/src/connection/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ impl Stream {
fn send_window_update(&mut self, cx: &mut Context) -> Poll<io::Result<()>> {
// When using [`WindowUpdateMode::OnReceive`] window update messages are
// send early on data receival (see [`crate::Connection::on_frame`]).
#[allow(deprecated)]
if matches!(self.config.window_update_mode, WindowUpdateMode::OnReceive) {
return Poll::Ready(Ok(()));
}
Expand Down Expand Up @@ -500,6 +501,7 @@ impl Shared {
}

let new_credit = match self.config.window_update_mode {
#[allow(deprecated)]
WindowUpdateMode::OnReceive => {
debug_assert!(self.config.receive_window >= self.window);

Expand Down
11 changes: 2 additions & 9 deletions yamux/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,20 +73,13 @@ pub enum WindowUpdateMode {
/// data in its buffer which may reach its limit (see `set_max_buffer_size`).
/// In this mode, window updates merely prevent head of line blocking but do not
/// effectively exercise back pressure on senders.
#[deprecated(note = "Use `WindowUpdateMode::OnRead` instead.")]
OnReceive,

/// Send window updates only when data is read on the receiving end.
///
/// This ensures that senders do not overwhelm receivers and keeps buffer usage
/// low. However, depending on the protocol, there is a risk of deadlock, namely
/// if both endpoints want to send data larger than the receivers window and they
/// do not read before finishing their writes. Use this mode only if you are sure
/// that this will never happen, i.e. if
///
/// - Endpoints *A* and *B* never write at the same time, *or*
/// - Endpoints *A* and *B* write at most *n* frames concurrently such that the sum
/// of the frame lengths is less or equal to the available credit of *A* and *B*
/// respectively.
/// low.
OnRead,
}

Expand Down

0 comments on commit 61d510c

Please sign in to comment.