From 2931ddd2e991d31958a1f59066ecb8c5923af892 Mon Sep 17 00:00:00 2001
From: Harald Alvestrand <hta@webrtc.org>
Date: Fri, 3 Nov 2023 15:30:17 +0000
Subject: [PATCH] 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               |  7 +++++++
 pc/rtp_transport.h                |  1 +
 pc/rtp_transport_unittest.cc      | 23 +++++++++++++++++++++++
 pc/test/rtp_transport_test_util.h | 13 +++++++++++++
 4 files changed, 44 insertions(+)

diff --git a/pc/rtp_transport.cc b/pc/rtp_transport.cc
index 653b51fd9e..cddfaca4fc 100644
--- a/pc/rtp_transport.cc
+++ b/pc/rtp_transport.cc
@@ -226,7 +226,14 @@ void RtpTransport::OnSentPacket(rtc::PacketTransportInternal* packet_transport,
                                 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,
diff --git a/pc/rtp_transport.h b/pc/rtp_transport.h
index 456c91c370..490a9ef171 100644
--- a/pc/rtp_transport.h
+++ b/pc/rtp_transport.h
@@ -140,6 +140,7 @@ class RtpTransport : public RtpTransportInternal {
   RtpHeaderExtensionMap header_extension_map_;
   // Guard against recursive "ready to send" signals
   bool processing_ready_to_send_ = false;
+  bool processing_sent_packet_ = false;
   ScopedTaskSafety safety_;
 };
 
diff --git a/pc/rtp_transport_unittest.cc b/pc/rtp_transport_unittest.cc
index 5b6a8309e0..d3eb666c9f 100644
--- a/pc/rtp_transport_unittest.cc
+++ b/pc/rtp_transport_unittest.cc
@@ -349,4 +349,27 @@ TEST(RtpTransportTest, RecursiveSetSendDoesNotCrash) {
   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
diff --git a/pc/test/rtp_transport_test_util.h b/pc/test/rtp_transport_test_util.h
index 593ee002c9..8aeaf07c1d 100644
--- a/pc/test/rtp_transport_test_util.h
+++ b/pc/test/rtp_transport_test_util.h
@@ -36,6 +36,13 @@ class TransportObserver : public RtpPacketSinkInterface {
         [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 @@ class TransportObserver : public RtpPacketSinkInterface {
   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 @@ class TransportObserver : public RtpPacketSinkInterface {
   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