From 57db65255cc6004e3d82225fa4d2df565f169497 Mon Sep 17 00:00:00 2001 From: perkj Date: Fri, 8 Apr 2016 08:16:33 -0700 Subject: [PATCH] Changed PeerConnectionEndToEndTest to use a separate worker thread. This is a follow up to https://codereview.webrtc.org/1859933002 to change this test also to use a separate worker thread. PeerConnectionEndToEndTest currently use the current thread both as a signaling thread and a worker thread. Although convenient while debugging, it can also hide real bugs. An example is https://codereview.webrtc.org/1766653002/#ps420001 where the worker thread is deadlocked in the track proxy due to that the worker thread waits for the signaling thread but the proxy in turns invokes the worker thread..... That bug was only discovered on Android. BUG= webrtc:5426 Review URL: https://codereview.webrtc.org/1860423002 Cr-Commit-Position: refs/heads/master@{#12295} --- webrtc/api/peerconnectionendtoend_unittest.cc | 13 ++++++++----- webrtc/api/test/peerconnectiontestwrapper.cc | 11 ++++++----- webrtc/api/test/peerconnectiontestwrapper.h | 4 +++- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/webrtc/api/peerconnectionendtoend_unittest.cc b/webrtc/api/peerconnectionendtoend_unittest.cc index be662886d3..20df17bc63 100644 --- a/webrtc/api/peerconnectionendtoend_unittest.cc +++ b/webrtc/api/peerconnectionendtoend_unittest.cc @@ -17,6 +17,7 @@ #include "webrtc/base/gunit.h" #include "webrtc/base/logging.h" #include "webrtc/base/ssladapter.h" +#include "webrtc/base/thread.h" #include "webrtc/base/sslstreamadapter.h" #include "webrtc/base/stringencode.h" #include "webrtc/base/stringutils.h" @@ -46,11 +47,12 @@ class PeerConnectionEndToEndTest typedef std::vector > DataChannelList; - PeerConnectionEndToEndTest() - : caller_(new rtc::RefCountedObject( - "caller")), - callee_(new rtc::RefCountedObject( - "callee")) { + PeerConnectionEndToEndTest() { + RTC_CHECK(worker_thread_.Start()); + caller_ = new rtc::RefCountedObject( + "caller", &worker_thread_); + callee_ = new rtc::RefCountedObject( + "callee", &worker_thread_); #ifdef WEBRTC_ANDROID webrtc::InitializeAndroidObjects(); #endif @@ -151,6 +153,7 @@ class PeerConnectionEndToEndTest } protected: + rtc::Thread worker_thread_; rtc::scoped_refptr caller_; rtc::scoped_refptr callee_; DataChannelList caller_signaled_data_channels_; diff --git a/webrtc/api/test/peerconnectiontestwrapper.cc b/webrtc/api/test/peerconnectiontestwrapper.cc index 2ff9b5d009..038980ce56 100644 --- a/webrtc/api/test/peerconnectiontestwrapper.cc +++ b/webrtc/api/test/peerconnectiontestwrapper.cc @@ -47,15 +47,16 @@ void PeerConnectionTestWrapper::Connect(PeerConnectionTestWrapper* caller, caller, &PeerConnectionTestWrapper::ReceiveAnswerSdp); } -PeerConnectionTestWrapper::PeerConnectionTestWrapper(const std::string& name) - : name_(name) {} +PeerConnectionTestWrapper::PeerConnectionTestWrapper(const std::string& name, + rtc::Thread* worker_thread) + : name_(name), worker_thread_(worker_thread) {} PeerConnectionTestWrapper::~PeerConnectionTestWrapper() {} bool PeerConnectionTestWrapper::CreatePc( const MediaConstraintsInterface* constraints) { rtc::scoped_ptr port_allocator( - new cricket::FakePortAllocator(rtc::Thread::Current(), nullptr)); + new cricket::FakePortAllocator(worker_thread_, nullptr)); fake_audio_capture_module_ = FakeAudioCaptureModule::Create(); if (fake_audio_capture_module_ == NULL) { @@ -63,8 +64,8 @@ bool PeerConnectionTestWrapper::CreatePc( } peer_connection_factory_ = webrtc::CreatePeerConnectionFactory( - rtc::Thread::Current(), rtc::Thread::Current(), - fake_audio_capture_module_, NULL, NULL); + worker_thread_, rtc::Thread::Current(), fake_audio_capture_module_, NULL, + NULL); if (!peer_connection_factory_) { return false; } diff --git a/webrtc/api/test/peerconnectiontestwrapper.h b/webrtc/api/test/peerconnectiontestwrapper.h index 25510b2de0..f744b57c22 100644 --- a/webrtc/api/test/peerconnectiontestwrapper.h +++ b/webrtc/api/test/peerconnectiontestwrapper.h @@ -25,7 +25,8 @@ class PeerConnectionTestWrapper static void Connect(PeerConnectionTestWrapper* caller, PeerConnectionTestWrapper* callee); - explicit PeerConnectionTestWrapper(const std::string& name); + explicit PeerConnectionTestWrapper(const std::string& name, + rtc::Thread* worker_thread); virtual ~PeerConnectionTestWrapper(); bool CreatePc(const webrtc::MediaConstraintsInterface* constraints); @@ -88,6 +89,7 @@ class PeerConnectionTestWrapper bool video, const webrtc::FakeConstraints& video_constraints); std::string name_; + rtc::Thread* worker_thread_; rtc::scoped_refptr peer_connection_; rtc::scoped_refptr peer_connection_factory_;