diff --git a/pc/peer_connection_simulcast_unittest.cc b/pc/peer_connection_simulcast_unittest.cc index 5ffbe34971..f975d370c7 100644 --- a/pc/peer_connection_simulcast_unittest.cc +++ b/pc/peer_connection_simulcast_unittest.cc @@ -176,11 +176,7 @@ const webrtc::RTCOutboundRtpStreamStats* FindOutboundRtpByRid( namespace webrtc { constexpr TimeDelta kDefaultTimeout = TimeDelta::Seconds(5); - -// Only used by tests disabled under ASAN, avoids unsued variable compile error. -#if !defined(ADDRESS_SANITIZER) constexpr TimeDelta kLongTimeoutForRampingUp = TimeDelta::Seconds(30); -#endif // !defined(ADDRESS_SANITIZER) class PeerConnectionSimulcastTests : public ::testing::Test { public: @@ -1173,12 +1169,6 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests, EXPECT_THAT(*outbound_rtps[0]->scalability_mode, StrEq("L1T1")); } -// TODO(https://crbug.com/webrtc/15018): Investigate heap-use-after free during -// shutdown of the test that is flakily happening on bots. It's not only -// happening on ASAN, but it is rare enough on non-ASAN that we don't have to -// disable everywhere. -#if !defined(ADDRESS_SANITIZER) - TEST_F(PeerConnectionSimulcastWithMediaFlowTests, SendingThreeEncodings_VP8_Simulcast) { rtc::scoped_refptr local_pc_wrapper = CreatePc(); @@ -1757,6 +1747,4 @@ TEST_F(PeerConnectionSimulcastWithMediaFlowTests, EXPECT_THAT(*outbound_rtps[2]->scalability_mode, StrEq("L1T3")); } -#endif // !defined(ADDRESS_SANITIZER) - } // namespace webrtc diff --git a/pc/test/fake_periodic_video_track_source.h b/pc/test/fake_periodic_video_track_source.h index 98a456f232..f91144d1cc 100644 --- a/pc/test/fake_periodic_video_track_source.h +++ b/pc/test/fake_periodic_video_track_source.h @@ -29,6 +29,7 @@ class FakePeriodicVideoTrackSource : public VideoTrackSource { ~FakePeriodicVideoTrackSource() = default; + FakePeriodicVideoSource& fake_periodic_source() { return source_; } const FakePeriodicVideoSource& fake_periodic_source() const { return source_; } diff --git a/pc/test/peer_connection_test_wrapper.cc b/pc/test/peer_connection_test_wrapper.cc index cd66eaec78..112f04d66b 100644 --- a/pc/test/peer_connection_test_wrapper.cc +++ b/pc/test/peer_connection_test_wrapper.cc @@ -39,7 +39,6 @@ #include "p2p/base/fake_port_allocator.h" #include "p2p/base/port_allocator.h" #include "pc/test/fake_periodic_video_source.h" -#include "pc/test/fake_periodic_video_track_source.h" #include "pc/test/fake_rtc_certificate_generator.h" #include "pc/test/mock_peer_connection_observers.h" #include "rtc_base/gunit.h" @@ -129,6 +128,9 @@ PeerConnectionTestWrapper::PeerConnectionTestWrapper( PeerConnectionTestWrapper::~PeerConnectionTestWrapper() { RTC_DCHECK_RUN_ON(&pc_thread_checker_); + // To avoid flaky bot failures, make sure fake sources are stopped prior to + // closing the peer connections. See https://crbug.com/webrtc/15018. + StopFakeVideoSources(); // Either network_thread or worker_thread might be active at this point. // Relying on ~PeerConnection to properly wait for them doesn't work, // as a vptr race might occur (before we enter the destruction body). @@ -392,6 +394,7 @@ PeerConnectionTestWrapper::GetUserMedia( auto source = rtc::make_ref_counted( config, /* remote */ false); + fake_video_sources_.push_back(source); std::string videotrack_label = stream_id + kVideoTrackLabelBase; rtc::scoped_refptr video_track( @@ -401,3 +404,10 @@ PeerConnectionTestWrapper::GetUserMedia( } return stream; } + +void PeerConnectionTestWrapper::StopFakeVideoSources() { + for (const auto& fake_video_source : fake_video_sources_) { + fake_video_source->fake_periodic_source().Stop(); + } + fake_video_sources_.clear(); +} diff --git a/pc/test/peer_connection_test_wrapper.h b/pc/test/peer_connection_test_wrapper.h index 23a6996c30..cfc423f81a 100644 --- a/pc/test/peer_connection_test_wrapper.h +++ b/pc/test/peer_connection_test_wrapper.h @@ -29,6 +29,7 @@ #include "api/video/resolution.h" #include "pc/test/fake_audio_capture_module.h" #include "pc/test/fake_periodic_video_source.h" +#include "pc/test/fake_periodic_video_track_source.h" #include "pc/test/fake_video_track_renderer.h" #include "rtc_base/third_party/sigslot/sigslot.h" #include "rtc_base/thread.h" @@ -115,6 +116,7 @@ class PeerConnectionTestWrapper webrtc::Resolution resolution = { .width = webrtc::FakePeriodicVideoSource::kDefaultWidth, .height = webrtc::FakePeriodicVideoSource::kDefaultHeight}); + void StopFakeVideoSources(); private: void SetLocalDescription(webrtc::SdpType type, const std::string& sdp); @@ -136,6 +138,8 @@ class PeerConnectionTestWrapper std::unique_ptr renderer_; int num_get_user_media_calls_ = 0; bool pending_negotiation_; + std::vector> + fake_video_sources_; }; #endif // PC_TEST_PEER_CONNECTION_TEST_WRAPPER_H_