From c68bb2879f57625259ad876fdf21bbac5603f232 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 23 Jun 2022 13:25:59 +0400 Subject: [PATCH 1/4] return early if the incoming stream was reset NOTE: we don't need to shutdown the stream as it was already done by the association minus sending the reset request, which is not needed in this case (i.e. if we've just received one, we don't need to send one back in response). Refs https://github.com/webrtc-rs/sctp/pull/14 --- src/data_channel/mod.rs | 18 ++++++++++++------ src/error.rs | 9 --------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/src/data_channel/mod.rs b/src/data_channel/mod.rs index 3903154..94f5e65 100644 --- a/src/data_channel/mod.rs +++ b/src/data_channel/mod.rs @@ -139,22 +139,28 @@ impl DataChannel { Ok(data_channel) } - /// Read reads a packet of len(p) bytes as binary data + /// Read reads a packet of len(p) bytes as binary data. + /// + /// See [`sctp::stream::Stream::read_sctp`]. pub async fn read(&self, buf: &mut [u8]) -> Result { self.read_data_channel(buf).await.map(|(n, _)| n) } - /// ReadDataChannel reads a packet of len(p) bytes + /// ReadDataChannel reads a packet of len(p) bytes. It returns the number of bytes read and + /// `true` if the data read is a string. + /// + /// See [`sctp::stream::Stream::read_sctp`]. pub async fn read_data_channel(&self, buf: &mut [u8]) -> Result<(usize, bool)> { loop { //TODO: add handling of cancel read_data_channel let (mut n, ppi) = match self.stream.read_sctp(buf).await { + Ok((0, PayloadProtocolIdentifier::Unknown)) => { + // The incoming stream was reset or the reading half was shutdown + return Ok((0, false)) + } Ok((n, ppi)) => (n, ppi), Err(err) => { - // When the peer sees that an incoming stream was - // reset, it also resets its corresponding outgoing stream. - self.stream.shutdown(Shutdown::Both).await?; - + self.close().await?; return Err(err.into()); } }; diff --git a/src/error.rs b/src/error.rs index 7146a2e..a96358a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -60,12 +60,3 @@ impl PartialEq for Error { false } } - -impl PartialEq for Error { - fn eq(&self, other: &sctp::Error) -> bool { - if let Error::Sctp(e) = self { - return e == other; - } - false - } -} From c970b95717fef016c9058214a03afcb2e38bf95d Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 23 Jun 2022 13:28:39 +0400 Subject: [PATCH 2/4] do not account for data, which we've failed to send --- src/data_channel/mod.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/data_channel/mod.rs b/src/data_channel/mod.rs index 94f5e65..81559d3 100644 --- a/src/data_channel/mod.rs +++ b/src/data_channel/mod.rs @@ -267,18 +267,19 @@ impl DataChannel { (true, _) => PayloadProtocolIdentifier::String, }; - self.messages_sent.fetch_add(1, Ordering::SeqCst); - self.bytes_sent.fetch_add(data_len, Ordering::SeqCst); - - if data_len == 0 { + let n = if data_len == 0 { let _ = self .stream .write_sctp(&Bytes::from_static(&[0]), ppi) .await?; - Ok(0) + 0 } else { - Ok(self.stream.write_sctp(data, ppi).await?) - } + self.bytes_sent.fetch_add(data_len, Ordering::SeqCst); + self.stream.write_sctp(data, ppi).await? + }; + + self.messages_sent.fetch_add(1, Ordering::SeqCst); + Ok(n) } async fn write_data_channel_ack(&self) -> Result { From 5ecaff76f23fff3fc3a9b01e0d72b693ef710f25 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 23 Jun 2022 13:53:25 +0400 Subject: [PATCH 3/4] format code --- src/data_channel/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/data_channel/mod.rs b/src/data_channel/mod.rs index 81559d3..3353f01 100644 --- a/src/data_channel/mod.rs +++ b/src/data_channel/mod.rs @@ -156,7 +156,7 @@ impl DataChannel { let (mut n, ppi) = match self.stream.read_sctp(buf).await { Ok((0, PayloadProtocolIdentifier::Unknown)) => { // The incoming stream was reset or the reading half was shutdown - return Ok((0, false)) + return Ok((0, false)); } Ok((n, ppi)) => (n, ppi), Err(err) => { From c440bc9fd1316054380444684c179af747ff40b3 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Thu, 23 Jun 2022 15:11:23 +0400 Subject: [PATCH 4/4] only count number of bytes actually sent not the number of bytes we're going to send https://github.com/webrtc-rs/data/pull/11/files#r904854127 --- src/data_channel/mod.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/data_channel/mod.rs b/src/data_channel/mod.rs index 3353f01..ca3d936 100644 --- a/src/data_channel/mod.rs +++ b/src/data_channel/mod.rs @@ -160,6 +160,7 @@ impl DataChannel { } Ok((n, ppi)) => (n, ppi), Err(err) => { + // Shutdown the stream and send the reset request to the remote. self.close().await?; return Err(err.into()); } @@ -274,8 +275,9 @@ impl DataChannel { .await?; 0 } else { - self.bytes_sent.fetch_add(data_len, Ordering::SeqCst); - self.stream.write_sctp(data, ppi).await? + let n = self.stream.write_sctp(data, ppi).await?; + self.bytes_sent.fetch_add(n, Ordering::SeqCst); + n }; self.messages_sent.fetch_add(1, Ordering::SeqCst);