Skip to content

Commit

Permalink
Bug 1876843 - Vendor libwebrtc from 9f1e1925f3
Browse files Browse the repository at this point in the history
Upstream commit: https://webrtc.googlesource.com/src/+/9f1e1925f33a8dd0d7b84f805be6d7f15ff28b0e
    dcsctp: Fix not using iteraters after NackItem

    OutstandingData::NackItem nacks a chunk, and if that chunk reaches its
    partial reliability critera, it will abandon the entire message. If that
    message hasn't been sent in full, a placeholder "end" message is
    inserted (see https://crbug.com/webrtc/12812). And when the message is
    inserted, any iterators may be invalidated (if e.g. std::deque would
    want to grow the deque).

    So ensure that there are no iterators used after having called NackItem.
    By changing the interface of NackItem, and not passing an Item, but just
    the TSN, this is encouraged. NackAll was rewritten as a two-pass
    algorithm to first collect TSNs, then iterating that list, looking up
    the items in the second pass (constant complexity).

    (cherry picked from commit 161d2c84528ec9eb0c19bfb51024bca54353abc4)

    No-Try: True
    Bug: chromium:1510364
    Change-Id: I5156b6d6a683184f290e71c98f16bc68ea2a562f
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331320
    Reviewed-by: Harald Alvestrand <hta@webrtc.org>
    Commit-Queue: Victor Boivie <boivie@webrtc.org>
    Reviewed-by: Sam Zackrisson <saza@webrtc.org>
    Cr-Original-Commit-Position: refs/heads/main@{#41374}
    Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/331960
    Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
    Cr-Commit-Position: refs/branch-heads/6167@{#2}
    Cr-Branched-From: ece5cb83715dea85617114b6d4e981fdee2623ba-refs/heads/main@{#41315}
  • Loading branch information
jan-ivar committed Feb 10, 2024
1 parent 416e5cf commit 1455c8d
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 9 deletions.
3 changes: 3 additions & 0 deletions third_party/libwebrtc/README.moz-ff-commit
Original file line number Diff line number Diff line change
Expand Up @@ -27675,3 +27675,6 @@ ece5cb8371
# MOZ_LIBWEBRTC_SRC=/Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc MOZ_LIBWEBRTC_BRANCH=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh
# base of lastest vendoring
004a624023
# MOZ_LIBWEBRTC_SRC=/Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc MOZ_LIBWEBRTC_BRANCH=mozpatches bash dom/media/webrtc/third_party_build/fast-forward-libwebrtc.sh
# base of lastest vendoring
9f1e1925f3
2 changes: 2 additions & 0 deletions third_party/libwebrtc/README.mozilla
Original file line number Diff line number Diff line change
Expand Up @@ -18474,3 +18474,5 @@ libwebrtc updated from /Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc c
libwebrtc updated from /Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2024-02-10T23:01:26.358981.
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
libwebrtc updated from /Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2024-02-10T23:02:30.081484.
# ./mach python dom/media/webrtc/third_party_build/vendor-libwebrtc.py --from-local /Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc --commit mozpatches libwebrtc
libwebrtc updated from /Users/jan-ivar/moz/elm/.moz-fast-forward/moz-libwebrtc commit mozpatches on 2024-02-10T23:04:03.473809.
16 changes: 11 additions & 5 deletions third_party/libwebrtc/net/dcsctp/tx/outstanding_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -240,9 +240,8 @@ void OutstandingData::NackBetweenAckBlocks(
for (UnwrappedTSN tsn = prev_block_last_acked.next_value();
tsn < cur_block_first_acked && tsn <= max_tsn_to_nack;
tsn = tsn.next_value()) {
Item& item = GetItem(tsn);
ack_info.has_packet_loss |=
NackItem(tsn, item, /*retransmit_now=*/false,
NackItem(tsn, /*retransmit_now=*/false,
/*do_fast_retransmit=*/!is_in_fast_recovery);
}
prev_block_last_acked = UnwrappedTSN::AddTo(cumulative_tsn_ack, block.end);
Expand All @@ -255,9 +254,9 @@ void OutstandingData::NackBetweenAckBlocks(
}

bool OutstandingData::NackItem(UnwrappedTSN tsn,
Item& item,
bool retransmit_now,
bool do_fast_retransmit) {
Item& item = GetItem(tsn);
if (item.is_outstanding()) {
unacked_bytes_ -= GetSerializedChunkSize(item.data());
--unacked_items_;
Expand Down Expand Up @@ -446,13 +445,20 @@ absl::optional<UnwrappedTSN> OutstandingData::Insert(

void OutstandingData::NackAll() {
UnwrappedTSN tsn = last_cumulative_tsn_ack_;
// A two-pass algorithm is needed, as NackItem will invalidate iterators.
std::vector<UnwrappedTSN> tsns_to_nack;
for (Item& item : outstanding_data_) {
tsn.Increment();
if (!item.is_acked()) {
NackItem(tsn, item, /*retransmit_now=*/true,
/*do_fast_retransmit=*/false);
tsns_to_nack.push_back(tsn);
}
}

for (UnwrappedTSN tsn : tsns_to_nack) {
NackItem(tsn, /*retransmit_now=*/true,
/*do_fast_retransmit=*/false);
}

RTC_DCHECK(IsConsistent());
}

Expand Down
9 changes: 5 additions & 4 deletions third_party/libwebrtc/net/dcsctp/tx/outstanding_data.h
Original file line number Diff line number Diff line change
Expand Up @@ -330,10 +330,11 @@ class OutstandingData {
// many times so that it should be retransmitted, this will schedule it to be
// "fast retransmitted". This is only done just before going into fast
// recovery.
bool NackItem(UnwrappedTSN tsn,
Item& item,
bool retransmit_now,
bool do_fast_retransmit);
//
// Note that since nacking an item may result in it becoming abandoned, which
// in turn could alter `outstanding_data_`, any iterators are invalidated
// after having called this method.
bool NackItem(UnwrappedTSN tsn, bool retransmit_now, bool do_fast_retransmit);

// Given that a message fragment, `item` has been abandoned, abandon all other
// fragments that share the same message - both never-before-sent fragments
Expand Down

0 comments on commit 1455c8d

Please sign in to comment.