From 6ca6190be2bdb6dc50a77e393e747cf8f55ea2c0 Mon Sep 17 00:00:00 2001 From: "jiayl@webrtc.org" Date: Wed, 12 Nov 2014 17:28:40 +0000 Subject: [PATCH] Fix a SCTP message reordering issue in datachannel.cc. Previously DataChannel::SendQueuedDataMessages continues the loop of sending queued messages if the channel is blocked, which will cause message reordering if the channel becomes unblocked during the loop, i.e. messages attempted after the unblocking will be sent earlier than the older messages attempted before the unblocking. BUG=3979 R=pthatcher@webrtc.org Review URL: https://webrtc-codereview.appspot.com/25149004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@7690 4adac7df-926f-26a2-2b94-8c16560cd09d --- talk/app/webrtc/datachannel.cc | 42 ++++++++++++++++++++++------------ talk/app/webrtc/datachannel.h | 2 +- 2 files changed, 28 insertions(+), 16 deletions(-) diff --git a/talk/app/webrtc/datachannel.cc b/talk/app/webrtc/datachannel.cc index 952f5bfb6e..6ff0941501 100644 --- a/talk/app/webrtc/datachannel.cc +++ b/talk/app/webrtc/datachannel.cc @@ -215,7 +215,7 @@ bool DataChannel::Send(const DataBuffer& buffer) { return true; } - bool success = SendDataMessage(buffer); + bool success = SendDataMessage(buffer, true); if (data_channel_type_ == cricket::DCT_RTP) { return success; } @@ -461,17 +461,17 @@ void DataChannel::DeliverQueuedReceivedData() { void DataChannel::SendQueuedDataMessages() { ASSERT(was_ever_writable_ && state_ == kOpen); - PacketQueue packet_buffer; - packet_buffer.Swap(&queued_send_data_); - - while (!packet_buffer.Empty()) { - rtc::scoped_ptr buffer(packet_buffer.Front()); - SendDataMessage(*buffer); - packet_buffer.Pop(); + while (!queued_send_data_.Empty()) { + rtc::scoped_ptr buffer(queued_send_data_.Front()); + if (!SendDataMessage(*buffer, false)) { + break; + } + queued_send_data_.Pop(); } } -bool DataChannel::SendDataMessage(const DataBuffer& buffer) { +bool DataChannel::SendDataMessage(const DataBuffer& buffer, + bool queue_if_blocked) { cricket::SendDataParams send_params; if (data_channel_type_ == cricket::DCT_SCTP) { @@ -494,14 +494,26 @@ bool DataChannel::SendDataMessage(const DataBuffer& buffer) { cricket::SendDataResult send_result = cricket::SDR_SUCCESS; bool success = provider_->SendData(send_params, buffer.data, &send_result); - if (!success && data_channel_type_ == cricket::DCT_SCTP) { - if (send_result != cricket::SDR_BLOCK || !QueueSendDataMessage(buffer)) { - LOG(LS_ERROR) << "Closing the DataChannel due to a failure to send data, " - << "send_result = " << send_result; - Close(); + if (success) { + return true; + } + + if (data_channel_type_ != cricket::DCT_SCTP) { + return false; + } + + if (send_result == cricket::SDR_BLOCK) { + if (!queue_if_blocked || QueueSendDataMessage(buffer)) { + return false; } } - return success; + // Close the channel if the error is not SDR_BLOCK, or if queuing the + // message failed. + LOG(LS_ERROR) << "Closing the DataChannel due to a failure to send data, " + << "send_result = " << send_result; + Close(); + + return false; } bool DataChannel::QueueSendDataMessage(const DataBuffer& buffer) { diff --git a/talk/app/webrtc/datachannel.h b/talk/app/webrtc/datachannel.h index 728d1df90c..1a455b772c 100644 --- a/talk/app/webrtc/datachannel.h +++ b/talk/app/webrtc/datachannel.h @@ -213,7 +213,7 @@ class DataChannel : public DataChannelInterface, void DeliverQueuedReceivedData(); void SendQueuedDataMessages(); - bool SendDataMessage(const DataBuffer& buffer); + bool SendDataMessage(const DataBuffer& buffer, bool queue_if_blocked); bool QueueSendDataMessage(const DataBuffer& buffer); void SendQueuedControlMessages();