diff --git a/pc/rtcstats_integrationtest.cc b/pc/rtcstats_integrationtest.cc index 46687bab5f..ef29cbc71e 100644 --- a/pc/rtcstats_integrationtest.cc +++ b/pc/rtcstats_integrationtest.cc @@ -809,11 +809,11 @@ TEST_F(RTCStatsIntegrationTest, GetStatsWithInvalidReceiverSelector) { EXPECT_FALSE(report->size()); } -// TODO(bugs.webrtc.org/9847) Remove this test altogether if a proper fix cannot -// be found. For now it is lying, as we cannot safely gets stats while -// destroying PeerConnection. +// TODO(bugs.webrtc.org/10041) For now this is equivalent to the following +// test GetsStatsWhileClosingPeerConnection, because pc() is closed by +// PeerConnectionTestWrapper. See: bugs.webrtc.org/9847 TEST_F(RTCStatsIntegrationTest, - DISABLED_GetsStatsWhileDestroyingPeerConnection) { + DISABLED_GetStatsWhileDestroyingPeerConnection) { StartCall(); rtc::scoped_refptr stats_obtainer = diff --git a/pc/test/peerconnectiontestwrapper.cc b/pc/test/peerconnectiontestwrapper.cc index 0010edb0f8..a1db9ed9e6 100644 --- a/pc/test/peerconnectiontestwrapper.cc +++ b/pc/test/peerconnectiontestwrapper.cc @@ -23,6 +23,7 @@ #include "pc/test/mockpeerconnectionobservers.h" #include "pc/test/peerconnectiontestwrapper.h" #include "rtc_base/gunit.h" +#include "rtc_base/thread_checker.h" #include "rtc_base/timeutils.h" using webrtc::FakeConstraints; @@ -65,9 +66,20 @@ PeerConnectionTestWrapper::PeerConnectionTestWrapper( rtc::Thread* worker_thread) : name_(name), network_thread_(network_thread), - worker_thread_(worker_thread) {} + worker_thread_(worker_thread) { + pc_thread_checker_.DetachFromThread(); +} -PeerConnectionTestWrapper::~PeerConnectionTestWrapper() {} +PeerConnectionTestWrapper::~PeerConnectionTestWrapper() { + RTC_DCHECK_RUN_ON(&pc_thread_checker_); + // 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). + // See: bugs.webrtc.org/9847 + if (pc()) { + pc()->Close(); + } +} bool PeerConnectionTestWrapper::CreatePc( const webrtc::PeerConnectionInterface::RTCConfiguration& config, @@ -76,6 +88,8 @@ bool PeerConnectionTestWrapper::CreatePc( std::unique_ptr port_allocator( new cricket::FakePortAllocator(network_thread_, nullptr)); + RTC_DCHECK_RUN_ON(&pc_thread_checker_); + fake_audio_capture_module_ = FakeAudioCaptureModule::Create(); if (fake_audio_capture_module_ == NULL) { return false; diff --git a/pc/test/peerconnectiontestwrapper.h b/pc/test/peerconnectiontestwrapper.h index 21ba89acbf..b6a57f35d3 100644 --- a/pc/test/peerconnectiontestwrapper.h +++ b/pc/test/peerconnectiontestwrapper.h @@ -20,6 +20,7 @@ #include "pc/test/fakeaudiocapturemodule.h" #include "pc/test/fakevideotrackrenderer.h" #include "rtc_base/third_party/sigslot/sigslot.h" +#include "rtc_base/thread_checker.h" class PeerConnectionTestWrapper : public webrtc::PeerConnectionObserver, @@ -106,6 +107,7 @@ class PeerConnectionTestWrapper std::string name_; rtc::Thread* const network_thread_; rtc::Thread* const worker_thread_; + rtc::ThreadChecker pc_thread_checker_; rtc::scoped_refptr peer_connection_; rtc::scoped_refptr peer_connection_factory_;