Browse Source

Merge pull request #655 from paullouisageneau/refactor-dc-close-message

Refactor DataChannel close message
Paul-Louis Ageneau 3 years ago
parent
commit
37f5dc9654
3 changed files with 12 additions and 40 deletions
  1. 8 34
      src/impl/datachannel.cpp
  2. 3 3
      src/impl/peerconnection.cpp
  3. 1 3
      src/impl/sctptransport.cpp

+ 8 - 34
src/impl/datachannel.cpp

@@ -43,8 +43,7 @@ enum MessageType : uint8_t {
 	MESSAGE_OPEN_REQUEST = 0x00,
 	MESSAGE_OPEN_RESPONSE = 0x01,
 	MESSAGE_ACK = 0x02,
-	MESSAGE_OPEN = 0x03,
-	MESSAGE_CLOSE = 0x04
+	MESSAGE_OPEN = 0x03
 };
 
 enum ChannelType : uint8_t {
@@ -70,9 +69,6 @@ struct AckMessage {
 	uint8_t type = MESSAGE_ACK;
 };
 
-struct CloseMessage {
-	uint8_t type = MESSAGE_CLOSE;
-};
 #pragma pack(pop)
 
 bool DataChannel::IsOpenMessage(message_ptr message) {
@@ -120,33 +116,13 @@ void DataChannel::remoteClose() {
 }
 
 optional<message_variant> DataChannel::receive() {
-	while (auto next = mRecvQueue.tryPop()) {
-		message_ptr message = *next;
-		if (message->type != Message::Control)
-			return to_variant(std::move(*message));
-
-		auto raw = reinterpret_cast<const uint8_t *>(message->data());
-		if (!message->empty() && raw[0] == MESSAGE_CLOSE)
-			remoteClose();
-	}
-
-	return nullopt;
+	auto next = mRecvQueue.tryPop();
+	return next ? std::make_optional(to_variant(std::move(**next))) : nullopt;
 }
 
 optional<message_variant> DataChannel::peek() {
-	while (auto next = mRecvQueue.peek()) {
-		message_ptr message = *next;
-		if (message->type != Message::Control)
-			return to_variant(std::move(*message));
-
-		auto raw = reinterpret_cast<const uint8_t *>(message->data());
-		if (!message->empty() && raw[0] == MESSAGE_CLOSE)
-			remoteClose();
-
-		mRecvQueue.tryPop();
-	}
-
-	return nullopt;
+	auto next = mRecvQueue.peek();
+	return next ? std::make_optional(to_variant(std::move(**next))) : nullopt;
 }
 
 size_t DataChannel::availableAmount() const { return mRecvQueue.amount(); }
@@ -244,17 +220,15 @@ void DataChannel::incoming(message_ptr message) {
 				triggerOpen();
 			}
 			break;
-		case MESSAGE_CLOSE:
-			// The close message will be processed in-order in receive()
-			mRecvQueue.push(message);
-			triggerAvailable(mRecvQueue.size());
-			break;
 		default:
 			// Ignore
 			break;
 		}
 		break;
 	}
+	case Message::Reset:
+		remoteClose();
+		break;
 	case Message::String:
 	case Message::Binary:
 		mRecvQueue.push(message);

+ 3 - 3
src/impl/peerconnection.cpp

@@ -454,8 +454,8 @@ void PeerConnection::forwardMessage(message_ptr message) {
 	}
 
 	if (!channel) {
-		if (message->type == Message::Control) // ignore control messages like Close
-			return;
+		if (message->type == Message::Reset)
+			return; // ignore
 
 		// Invalid, close the DataChannel
 		PLOG_WARNING << "Got unexpected message on stream " << stream;
@@ -682,7 +682,7 @@ void PeerConnection::assignDataChannels() {
 			stream += 2;
 		}
 
-		PLOG_DEBUG << "Assigning stream " << stream  << " to DataChannel";
+		PLOG_DEBUG << "Assigning stream " << stream << " to DataChannel";
 
 		channel->assignStream(stream);
 		mDataChannels.emplace(std::make_pair(stream, channel));

+ 1 - 3
src/impl/sctptransport.cpp

@@ -886,11 +886,9 @@ void SctpTransport::processNotification(const union sctp_notification *notify, s
 		// corresponding outgoing stream.
 		// See https://www.rfc-editor.org/rfc/rfc8831.html#section-6.7
 		if (flags & SCTP_STREAM_RESET_INCOMING_SSN) {
-			const byte dataChannelCloseMessage{0x04};
 			for (int i = 0; i < count; ++i) {
 				uint16_t streamId = reset_event.strreset_stream_list[i];
-				recv(make_message(&dataChannelCloseMessage, &dataChannelCloseMessage + 1,
-				                  Message::Control, streamId));
+				recv(make_message(0, Message::Reset, streamId));
 			}
 		}
 		break;