commit d7fd0f9744bc0ed89857d1ec78c4299f19b9fb97
author Victor Boivie <boivie@webrtc.org> Mon May 02 15:15:57 2022
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Thu May 05 07:30:58 2022
tree 8ecd607a5978e5aa4f93eb8c3919fde3e4104604
parent 4ef2de62907f1c78b0294dcb08e4fb05f24e4abe

dcsctp: Handle rapid closing of streams

When streams were to be reset, but there was already an ongoing
stream reset command in-flight, those streams wouldn't be properly
reset. When multiple streams were reset close to each other (within
an RTT), some streams would not have their SSNs reset, which resulted
in the stream resuming the SSN sequence. This could result in ordered
streams not delivering all messages as the receiver wouldn't deliver any
messages with SSN different from the expected SSN=0.

In WebRTC data channels, this would be triggered if multiple channels
were closed at roughly the same time, then re-opened, and continued
to be used in ordered mode. Unordered messages would still be delivered,
but the stream state could be wrong as the DATA_CHANNEL_ACK message is
sent ordered, and possibly not delivered.

There were unit tests for this, but not on the socket level using
real components, but just on the stream reset handler using mocks,
where this issue wasn't found. Also, those mocks didn't validate that
the correct parameters were provided, so that's fixed now.

The root cause was the PrepareResetStreams was only called if there
wasn't an ongoing stream reset operation in progress. One may try to
solve it by always calling PrepareResetStreams also when there is an
ongoing request, or to call it when the request has finished. One would
then realize that when the response of the outgoing stream request is
received, and CommitResetStreams is called, it would reset all paused
and (prepared) to-be-reset streams - not just the ones in the outgoing
stream request.

One cause of this was the lack of a single source of truth of the stream
states. The SendQueue kept track of which streams that were paused, but
the stream reset handler kept track of which streams that were
resetting. As that's error prone, this CL moves the source of truth
completely to the SendQueue and defining explicit stream pause states. A
stream can be in one of these possible states:

  * Not paused. This is the default for an active stream.
  * Pending to be paused. This is when it's about to be reset, but
    there is a message that has been partly sent, with fragments
    remaining to be sent before it can be paused.
  * Paused, with no partly sent message. In this state, it's ready to
    be reset.
  * Resetting. A stream transitions into this state when it has been
    paused and has been included in an outgoing stream reset request.
    When this request has been responded to, the stream can really be
    reset (SSN=0, MID=0).

This CL also improves logging, and adds socket tests to catch this
issue.

Bug: webrtc:13994, chromium:1320194
Change-Id: I883570d1f277bc01e52b1afad62d6be2aca930a2
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261180
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Victor Boivie <boivie@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36771}