zqz/platform-external-webrtc
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Address vptr race condition while PeerConnection is destructed.

Browse Source 
Auxiliary threads (worker, network) are still active
while PeerConnection is destructed, leading to race condition
in tests such as:
  * RTCStatsIntegrationTest.GetStatsFromCaller
  * RTCStatsIntegrationTest.GetsStatsWhileDestroyingPeerConnection

This CL prevents the conflict to happen by explicitly
closing the PeerConnection.

Bug: webrtc:9847
Change-Id: I40880bb9b193201711031b8c4563c6bbd4983c71
Reviewed-on: https://webrtc-review.googlesource.com/c/104820
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25801}
This commit is contained in:
Yves Gerey
2018-11-26 16:22:20 +01:00
committed by Commit Bot
parent 09d6588d93
commit 59cfd35438
3 changed files with 22 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
pc/rtcstats_integrationtest.cc
View File

@ -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<RTCStatsObtainer> stats_obtainer =

18
pc/test/peerconnectiontestwrapper.cc
View File

@ -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<cricket::PortAllocator> 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;

2
pc/test/peerconnectiontestwrapper.h
View File

@ -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<webrtc::PeerConnectionInterface> peer_connection_;
rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface>
peer_connection_factory_;