admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fire SentPacket in a PostTask when recursive

Browse Source 
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}
This commit is contained in:
Harald Alvestrand 2023-11-03 15:30:17 +00:00 committed by WebRTC LUCI CQ
parent 97f3fb08a0
commit 2931ddd2e9
4 changed files with 44 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
pc/rtp_transport.cc
View File

@ -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,

1
pc/rtp_transport.h
View File

@ -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_;
};

23
pc/rtp_transport_unittest.cc
View File

@ -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

13
pc/test/rtp_transport_test_util.h
View File

@ -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