zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

datachannel: Don't close a data channel when the queue is full

Browse Source 
According to https://w3c.github.io/webrtc-pc/#datachannel-send it should
return an error, definitely not close the data channel.
While we should probably return an RTCError will better information, this
would break the API and will be done later.

Bug: webrtc:13289
Change-Id: I90baf012440fbe2a38a826cf50b50b2b668fd7ff
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/237180
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Florent Castelli <orphis@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#35306}
This commit is contained in:
Florent Castelli
2021-11-03 16:09:46 +01:00
committed by WebRTC LUCI CQ
parent d2abeab308
commit 01343031cd
3 changed files with 19 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

10
api/data_channel_interface.h
View File

@ -183,12 +183,10 @@ class RTC_EXPORT DataChannelInterface : public rtc::RefCountInterface {
// Sends `data` to the remote peer. If the data can't be sent at the SCTP // Sends `data` to the remote peer. If the data can't be sent at the SCTP
// level (due to congestion control), it's buffered at the data channel level, // level (due to congestion control), it's buffered at the data channel level,
// up to a maximum of MaxSendQueueSize(). If Send is called while this buffer // up to a maximum of MaxSendQueueSize().
// is full, the data channel will be closed abruptly. // Returns false if the data channel is not in open state or if the send
// // buffer is full.
// So, it's important to use buffered_amount() and OnBufferedAmountChange to // TODO(webrtc:13289): Return an RTCError with information about the failure.
// ensure the data channel is used efficiently but without filling this
// buffer.
virtual bool Send(const DataBuffer& buffer) = 0; virtual bool Send(const DataBuffer& buffer) = 0;
// Amount of bytes that can be queued for sending on the data channel. // Amount of bytes that can be queued for sending on the data channel.

20
pc/data_channel_unittest.cc
View File

@ -544,23 +544,29 @@ TEST_F(SctpDataChannelTest, OpenAckRoleInitialization) {
EXPECT_EQ(webrtc::InternalDataChannelInit::kNone, init2.open_handshake_role); EXPECT_EQ(webrtc::InternalDataChannelInit::kNone, init2.open_handshake_role);
} }
// Tests that the DataChannel is closed if the sending buffer is full. // Tests that that Send() returns false if the sending buffer is full
TEST_F(SctpDataChannelTest, ClosedWhenSendBufferFull) { // and the channel stays open.
TEST_F(SctpDataChannelTest, OpenWhenSendBufferFull) {
SetChannelReady(); SetChannelReady();
rtc::CopyOnWriteBuffer buffer(1024); const size_t packetSize = 1024;
rtc::CopyOnWriteBuffer buffer(packetSize);
memset(buffer.MutableData(), 0, buffer.size()); memset(buffer.MutableData(), 0, buffer.size());
webrtc::DataBuffer packet(buffer, true); webrtc::DataBuffer packet(buffer, true);
provider_->set_send_blocked(true); provider_->set_send_blocked(true);
for (size_t i = 0; i < 16 * 1024 + 1; ++i) { for (size_t i = 0;
i < webrtc::DataChannelInterface::MaxSendQueueSize() / packetSize; ++i) {
EXPECT_TRUE(webrtc_data_channel_->Send(packet)); EXPECT_TRUE(webrtc_data_channel_->Send(packet));
} }
EXPECT_TRUE( // The sending buffer shoul be full, send returns false.
webrtc::DataChannelInterface::kClosed == webrtc_data_channel_->state() || EXPECT_FALSE(webrtc_data_channel_->Send(packet));
webrtc::DataChannelInterface::kClosing == webrtc_data_channel_->state());
EXPECT_TRUE(webrtc::DataChannelInterface::kOpen ==
webrtc_data_channel_->state());
} }
// Tests that the DataChannel is closed on transport errors. // Tests that the DataChannel is closed on transport errors.

8
pc/sctp_data_channel.cc
View File

@ -309,12 +309,8 @@ bool SctpDataChannel::Send(const DataBuffer& buffer) {
// so just add to the end of the queue and keep waiting. // so just add to the end of the queue and keep waiting.
if (!queued_send_data_.Empty()) { if (!queued_send_data_.Empty()) {
if (!QueueSendDataMessage(buffer)) { if (!QueueSendDataMessage(buffer)) {
RTC_LOG(LS_ERROR) << "Closing the DataChannel due to a failure to queue " // Queue is full
"additional data."; return false;
// https://w3c.github.io/webrtc-pc/#dom-rtcdatachannel-send step 5
// Note that the spec doesn't explicitly say to close in this situation.
CloseAbruptlyWithError(RTCError(RTCErrorType::RESOURCE_EXHAUSTED,
"Unable to queue data for sending"));
} }
return true; return true;
} }