commit 2931ddd2e991d31958a1f59066ecb8c5923af892
author Harald Alvestrand <hta@webrtc.org> Fri Nov 03 15:30:17 2023
committer WebRTC LUCI CQ <webrtc-scoped@luci-project-accounts.iam.gserviceaccount.com> Fri Nov 03 16:41:34 2023
tree 1fba215d5026fb4bd7efc3ab7a15962194c83528
parent 97f3fb08a02cf39d33d601b0fb677d5ae13e161c

Fire SentPacket in a PostTask when recursive

Speculative fix; test included.

Bug: chromium:1496240
Change-Id: I9cb8953653e9d45adbc8694b67b0d5399cf9fde9
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/326020
Reviewed-by: Victor Boivie <boivie@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41078}

pc/rtp_transport.cc

diff --git a/pc/rtp_transport.cc b/pc/rtp_transport.cc
index 653b51f..cddfaca 100644
--- a/pc/rtp_transport.cc
+++ b/pc/rtp_transport.cc
@@ -226,7 +226,14 @@
                                 const rtc::SentPacket& sent_packet) {
   RTC_DCHECK(packet_transport == rtp_packet_transport_ ||
              packet_transport == rtcp_packet_transport_);
+  if (processing_sent_packet_) {
+    TaskQueueBase::Current()->PostTask(SafeTask(
+        safety_.flag(), [this, &sent_packet] { SendSentPacket(sent_packet); }));
+    return;
+  }
+  processing_sent_packet_ = true;
   SendSentPacket(sent_packet);
+  processing_sent_packet_ = false;
 }
 
 void RtpTransport::OnRtpPacketReceived(rtc::CopyOnWriteBuffer packet,

pc/rtp_transport.h

diff --git a/pc/rtp_transport.h b/pc/rtp_transport.h
index 456c91c..490a9ef 100644
--- a/pc/rtp_transport.h
+++ b/pc/rtp_transport.h
@@ -140,6 +140,7 @@
   RtpHeaderExtensionMap header_extension_map_;
   // Guard against recursive "ready to send" signals
   bool processing_ready_to_send_ = false;
+  bool processing_sent_packet_ = false;
   ScopedTaskSafety safety_;
 };
 

pc/rtp_transport_unittest.cc

diff --git a/pc/rtp_transport_unittest.cc b/pc/rtp_transport_unittest.cc
index 5b6a830..d3eb666 100644
--- a/pc/rtp_transport_unittest.cc
+++ b/pc/rtp_transport_unittest.cc
@@ -349,4 +349,27 @@
   EXPECT_FALSE(observer.ready_to_send());
 }
 
+TEST(RtpTransportTest, RecursiveOnSentPacketDoesNotCrash) {
+  const int kShortTimeout = 100;
+  test::RunLoop loop;
+  RtpTransport transport(kMuxEnabled);
+  rtc::FakePacketTransport fake_rtp("fake_rtp");
+  transport.SetRtpPacketTransport(&fake_rtp);
+  fake_rtp.SetDestination(&fake_rtp, true);
+  TransportObserver observer(&transport);
+  const rtc::PacketOptions options;
+  const int flags = 0;
+  rtc::CopyOnWriteBuffer rtp_data(kRtpData, kRtpLen);
+
+  fake_rtp.SetWritable(true);
+  observer.SetActionOnSentPacket([&]() {
+    if (observer.sent_packet_count() < 2) {
+      transport.SendRtpPacket(&rtp_data, options, flags);
+    }
+  });
+  transport.SendRtpPacket(&rtp_data, options, flags);
+  EXPECT_EQ(observer.sent_packet_count(), 1);
+  EXPECT_EQ_WAIT(observer.sent_packet_count(), 2, kShortTimeout);
+}
+
 }  // namespace webrtc

pc/test/rtp_transport_test_util.h

diff --git a/pc/test/rtp_transport_test_util.h b/pc/test/rtp_transport_test_util.h
index 593ee00..8aeaf07 100644
--- a/pc/test/rtp_transport_test_util.h
+++ b/pc/test/rtp_transport_test_util.h
@@ -36,6 +36,13 @@
         [this](webrtc::RtpPacketReceived& packet) {
           OnUndemuxableRtpPacket(packet);
         });
+    rtp_transport->SubscribeSentPacket(this,
+                                       [this](const rtc::SentPacket& packet) {
+                                         sent_packet_count_++;
+                                         if (action_on_sent_packet_) {
+                                           action_on_sent_packet_();
+                                         }
+                                       });
   }
 
   // RtpPacketInterface override.
@@ -57,6 +64,7 @@
   int rtp_count() const { return rtp_count_; }
   int un_demuxable_rtp_count() const { return un_demuxable_rtp_count_; }
   int rtcp_count() const { return rtcp_count_; }
+  int sent_packet_count() const { return sent_packet_count_; }
 
   rtc::CopyOnWriteBuffer last_recv_rtp_packet() {
     return last_recv_rtp_packet_;
@@ -81,16 +89,21 @@
   void SetActionOnReadyToSend(absl::AnyInvocable<void(bool)> action) {
     action_on_ready_to_send_ = std::move(action);
   }
+  void SetActionOnSentPacket(absl::AnyInvocable<void()> action) {
+    action_on_sent_packet_ = std::move(action);
+  }
 
  private:
   bool ready_to_send_ = false;
   int rtp_count_ = 0;
   int un_demuxable_rtp_count_ = 0;
   int rtcp_count_ = 0;
+  int sent_packet_count_ = 0;
   int ready_to_send_signal_count_ = 0;
   rtc::CopyOnWriteBuffer last_recv_rtp_packet_;
   rtc::CopyOnWriteBuffer last_recv_rtcp_packet_;
   absl::AnyInvocable<void(bool)> action_on_ready_to_send_;
+  absl::AnyInvocable<void()> action_on_sent_packet_;
 };
 
 }  // namespace webrtc