From aed7164bde540ddc96ffedb7b8d87829f8eeb42e Mon Sep 17 00:00:00 2001 From: Seth Hampson Date: Mon, 11 Jun 2018 07:41:32 -0700 Subject: [PATCH] Updated PeerConnection integration test to fix race condition. The PeerConnection integration test was creating TurnServers on the stack on the signaling thread. This could cause a race condition problem when the test was being taken down. Since the turn server was destructed on the signaling thread, a socket might still try and send to it after it was destroyed causing a seg fault. This change creates/destroys the TestTurnServers on the network thread to fix this issue. Bug: None Change-Id: I080098502b737f0972ce2fa5357920de057a3312 Reviewed-on: https://webrtc-review.googlesource.com/81301 Reviewed-by: Qingsi Wang Reviewed-by: Steve Anton Commit-Queue: Seth Hampson Cr-Commit-Position: refs/heads/master@{#23590} --- p2p/base/testturnserver.h | 15 ++- p2p/base/turnport.h | 3 +- p2p/base/turnserver.cc | 25 +++++ p2p/base/turnserver.h | 42 ++++++-- pc/peerconnection_integrationtest.cc | 144 +++++++++++++++++---------- 5 files changed, 167 insertions(+), 62 deletions(-) diff --git a/p2p/base/testturnserver.h b/p2p/base/testturnserver.h index 067faddc86..d130560ff2 100644 --- a/p2p/base/testturnserver.h +++ b/p2p/base/testturnserver.h @@ -21,6 +21,7 @@ #include "rtc_base/ssladapter.h" #include "rtc_base/sslidentity.h" #include "rtc_base/thread.h" +#include "rtc_base/thread_checker.h" namespace cricket { @@ -65,17 +66,25 @@ class TestTurnServer : public TurnAuthInterface { server_.set_auth_hook(this); } + ~TestTurnServer() { RTC_DCHECK(thread_checker_.CalledOnValidThread()); } + void set_enable_otu_nonce(bool enable) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); server_.set_enable_otu_nonce(enable); } - TurnServer* server() { return &server_; } + TurnServer* server() { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + return &server_; + } void set_redirect_hook(TurnRedirectInterface* redirect_hook) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); server_.set_redirect_hook(redirect_hook); } void set_enable_permission_checks(bool enable) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); server_.set_enable_permission_checks(enable); } @@ -83,6 +92,7 @@ class TestTurnServer : public TurnAuthInterface { ProtocolType proto, bool ignore_bad_cert = true, const std::string& common_name = "test turn server") { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); if (proto == cricket::PROTO_UDP) { server_.AddInternalSocket( rtc::AsyncUDPSocket::Create(thread_->socketserver(), int_addr), @@ -115,6 +125,7 @@ class TestTurnServer : public TurnAuthInterface { // Finds the first allocation in the server allocation map with a source // ip and port matching the socket address provided. TurnServerAllocation* FindAllocation(const rtc::SocketAddress& src) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); const TurnServer::AllocationMap& map = server_.allocations(); for (TurnServer::AllocationMap::const_iterator it = map.begin(); it != map.end(); ++it) { @@ -130,11 +141,13 @@ class TestTurnServer : public TurnAuthInterface { // Obviously, do not use this in a production environment. virtual bool GetKey(const std::string& username, const std::string& realm, std::string* key) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); return ComputeStunCredentialHash(username, realm, username, key); } TurnServer server_; rtc::Thread* thread_; + rtc::ThreadChecker thread_checker_; }; } // namespace cricket diff --git a/p2p/base/turnport.h b/p2p/base/turnport.h index b0def2a898..5955aafda1 100644 --- a/p2p/base/turnport.h +++ b/p2p/base/turnport.h @@ -334,7 +334,8 @@ class TurnPort : public Port { rtc::AsyncInvoker invoker_; - // Optional TurnCustomizer that can modify outgoing messages. + // Optional TurnCustomizer that can modify outgoing messages. Once set, this + // must outlive the TurnPort's lifetime. webrtc::TurnCustomizer *turn_customizer_ = nullptr; friend class TurnEntry; diff --git a/p2p/base/turnserver.cc b/p2p/base/turnserver.cc index 2a154aa76b..350ebd0119 100644 --- a/p2p/base/turnserver.cc +++ b/p2p/base/turnserver.cc @@ -129,6 +129,7 @@ TurnServer::TurnServer(rtc::Thread* thread) } TurnServer::~TurnServer() { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); for (InternalSocketMap::iterator it = server_sockets_.begin(); it != server_sockets_.end(); ++it) { rtc::AsyncPacketSocket* socket = it->first; @@ -144,6 +145,7 @@ TurnServer::~TurnServer() { void TurnServer::AddInternalSocket(rtc::AsyncPacketSocket* socket, ProtocolType proto) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); RTC_DCHECK(server_sockets_.end() == server_sockets_.find(socket)); server_sockets_[socket] = proto; socket->SignalReadPacket.connect(this, &TurnServer::OnInternalPacket); @@ -151,6 +153,7 @@ void TurnServer::AddInternalSocket(rtc::AsyncPacketSocket* socket, void TurnServer::AddInternalServerSocket(rtc::AsyncSocket* socket, ProtocolType proto) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); RTC_DCHECK(server_listen_sockets_.end() == server_listen_sockets_.find(socket)); server_listen_sockets_[socket] = proto; @@ -160,17 +163,20 @@ void TurnServer::AddInternalServerSocket(rtc::AsyncSocket* socket, void TurnServer::SetExternalSocketFactory( rtc::PacketSocketFactory* factory, const rtc::SocketAddress& external_addr) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); external_socket_factory_.reset(factory); external_addr_ = external_addr; } void TurnServer::OnNewInternalConnection(rtc::AsyncSocket* socket) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); RTC_DCHECK(server_listen_sockets_.find(socket) != server_listen_sockets_.end()); AcceptConnection(socket); } void TurnServer::AcceptConnection(rtc::AsyncSocket* server_socket) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Check if someone is trying to connect to us. rtc::SocketAddress accept_addr; rtc::AsyncSocket* accepted_socket = server_socket->Accept(&accept_addr); @@ -187,6 +193,7 @@ void TurnServer::AcceptConnection(rtc::AsyncSocket* server_socket) { void TurnServer::OnInternalSocketClose(rtc::AsyncPacketSocket* socket, int err) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); DestroyInternalSocket(socket); } @@ -194,6 +201,7 @@ void TurnServer::OnInternalPacket(rtc::AsyncPacketSocket* socket, const char* data, size_t size, const rtc::SocketAddress& addr, const rtc::PacketTime& packet_time) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Fail if the packet is too small to even contain a channel header. if (size < TURN_CHANNEL_HEADER_SIZE) { return; @@ -219,6 +227,7 @@ void TurnServer::OnInternalPacket(rtc::AsyncPacketSocket* socket, void TurnServer::HandleStunMessage(TurnServerConnection* conn, const char* data, size_t size) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); TurnMessage msg; rtc::ByteBufferReader buf(data, size); if (!msg.Read(&buf) || (buf.Length() > 0)) { @@ -285,6 +294,7 @@ void TurnServer::HandleStunMessage(TurnServerConnection* conn, const char* data, } bool TurnServer::GetKey(const StunMessage* msg, std::string* key) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); const StunByteStringAttribute* username_attr = msg->GetByteString(STUN_ATTR_USERNAME); if (!username_attr) { @@ -299,6 +309,7 @@ bool TurnServer::CheckAuthorization(TurnServerConnection* conn, const StunMessage* msg, const char* data, size_t size, const std::string& key) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // RFC 5389, 10.2.2. RTC_DCHECK(IsStunRequestType(msg->type())); const StunByteStringAttribute* mi_attr = @@ -357,6 +368,7 @@ bool TurnServer::CheckAuthorization(TurnServerConnection* conn, void TurnServer::HandleBindingRequest(TurnServerConnection* conn, const StunMessage* req) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); StunMessage response; InitResponse(req, &response); @@ -371,6 +383,7 @@ void TurnServer::HandleBindingRequest(TurnServerConnection* conn, void TurnServer::HandleAllocateRequest(TurnServerConnection* conn, const TurnMessage* msg, const std::string& key) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Check the parameters in the request. const StunUInt32Attribute* transport_attr = msg->GetUInt32(STUN_ATTR_REQUESTED_TRANSPORT); @@ -400,6 +413,7 @@ void TurnServer::HandleAllocateRequest(TurnServerConnection* conn, } std::string TurnServer::GenerateNonce(int64_t now) const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Generate a nonce of the form hex(now + HMAC-MD5(nonce_key_, now)) std::string input(reinterpret_cast(&now), sizeof(now)); std::string nonce = rtc::hex_encode(input.c_str(), input.size()); @@ -410,6 +424,7 @@ std::string TurnServer::GenerateNonce(int64_t now) const { } bool TurnServer::ValidateNonce(const std::string& nonce) const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Check the size. if (nonce.size() != kNonceSize) { return false; @@ -435,6 +450,7 @@ bool TurnServer::ValidateNonce(const std::string& nonce) const { } TurnServerAllocation* TurnServer::FindAllocation(TurnServerConnection* conn) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); AllocationMap::const_iterator it = allocations_.find(*conn); return (it != allocations_.end()) ? it->second.get() : nullptr; } @@ -442,6 +458,7 @@ TurnServerAllocation* TurnServer::FindAllocation(TurnServerConnection* conn) { TurnServerAllocation* TurnServer::CreateAllocation(TurnServerConnection* conn, int proto, const std::string& key) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); rtc::AsyncPacketSocket* external_socket = (external_socket_factory_) ? external_socket_factory_->CreateUdpSocket(external_addr_, 0, 0) : NULL; if (!external_socket) { @@ -459,6 +476,7 @@ TurnServerAllocation* TurnServer::CreateAllocation(TurnServerConnection* conn, void TurnServer::SendErrorResponse(TurnServerConnection* conn, const StunMessage* req, int code, const std::string& reason) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); TurnMessage resp; InitErrorResponse(req, code, reason, &resp); RTC_LOG(LS_INFO) << "Sending error response, type=" << resp.type() @@ -469,6 +487,7 @@ void TurnServer::SendErrorResponse(TurnServerConnection* conn, void TurnServer::SendErrorResponseWithRealmAndNonce( TurnServerConnection* conn, const StunMessage* msg, int code, const std::string& reason) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); TurnMessage resp; InitErrorResponse(msg, code, reason, &resp); @@ -487,6 +506,7 @@ void TurnServer::SendErrorResponseWithRealmAndNonce( void TurnServer::SendErrorResponseWithAlternateServer( TurnServerConnection* conn, const StunMessage* msg, const rtc::SocketAddress& addr) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); TurnMessage resp; InitErrorResponse(msg, STUN_ERROR_TRY_ALTERNATE, STUN_ERROR_REASON_TRY_ALTERNATE_SERVER, &resp); @@ -496,6 +516,7 @@ void TurnServer::SendErrorResponseWithAlternateServer( } void TurnServer::SendStun(TurnServerConnection* conn, StunMessage* msg) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); rtc::ByteBufferWriter buf; // Add a SOFTWARE attribute if one is set. if (!software_.empty()) { @@ -508,11 +529,13 @@ void TurnServer::SendStun(TurnServerConnection* conn, StunMessage* msg) { void TurnServer::Send(TurnServerConnection* conn, const rtc::ByteBufferWriter& buf) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); rtc::PacketOptions options; conn->socket()->SendTo(buf.Data(), buf.Length(), conn->src(), options); } void TurnServer::OnAllocationDestroyed(TurnServerAllocation* allocation) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); // Removing the internal socket if the connection is not udp. rtc::AsyncPacketSocket* socket = allocation->conn()->socket(); InternalSocketMap::iterator iter = server_sockets_.find(socket); @@ -532,6 +555,7 @@ void TurnServer::OnAllocationDestroyed(TurnServerAllocation* allocation) { } void TurnServer::DestroyInternalSocket(rtc::AsyncPacketSocket* socket) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); InternalSocketMap::iterator iter = server_sockets_.find(socket); if (iter != server_sockets_.end()) { rtc::AsyncPacketSocket* socket = iter->first; @@ -547,6 +571,7 @@ void TurnServer::DestroyInternalSocket(rtc::AsyncPacketSocket* socket) { } void TurnServer::FreeSockets() { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); sockets_to_delete_.clear(); } diff --git a/p2p/base/turnserver.h b/p2p/base/turnserver.h index 3ead996c25..acba3d2845 100644 --- a/p2p/base/turnserver.h +++ b/p2p/base/turnserver.h @@ -25,6 +25,7 @@ #include "rtc_base/messagequeue.h" #include "rtc_base/sigslot.h" #include "rtc_base/socketaddress.h" +#include "rtc_base/thread_checker.h" namespace rtc { class ByteBufferWriter; @@ -178,30 +179,54 @@ class TurnServer : public sigslot::has_slots<> { ~TurnServer() override; // Gets/sets the realm value to use for the server. - const std::string& realm() const { return realm_; } - void set_realm(const std::string& realm) { realm_ = realm; } + const std::string& realm() const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + return realm_; + } + void set_realm(const std::string& realm) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + realm_ = realm; + } // Gets/sets the value for the SOFTWARE attribute for TURN messages. - const std::string& software() const { return software_; } - void set_software(const std::string& software) { software_ = software; } + const std::string& software() const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + return software_; + } + void set_software(const std::string& software) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + software_ = software; + } - const AllocationMap& allocations() const { return allocations_; } + const AllocationMap& allocations() const { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + return allocations_; + } // Sets the authentication callback; does not take ownership. - void set_auth_hook(TurnAuthInterface* auth_hook) { auth_hook_ = auth_hook; } + void set_auth_hook(TurnAuthInterface* auth_hook) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + auth_hook_ = auth_hook; + } void set_redirect_hook(TurnRedirectInterface* redirect_hook) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); redirect_hook_ = redirect_hook; } - void set_enable_otu_nonce(bool enable) { enable_otu_nonce_ = enable; } + void set_enable_otu_nonce(bool enable) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); + enable_otu_nonce_ = enable; + } // If set to true, reject CreatePermission requests to RFC1918 addresses. void set_reject_private_addresses(bool filter) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); reject_private_addresses_ = filter; } void set_enable_permission_checks(bool enable) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); enable_permission_checks_ = enable; } @@ -218,12 +243,14 @@ class TurnServer : public sigslot::has_slots<> { const rtc::SocketAddress& address); // For testing only. std::string SetTimestampForNextNonce(int64_t timestamp) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); ts_for_next_nonce_ = timestamp; return GenerateNonce(timestamp); } void SetStunMessageObserver( std::unique_ptr observer) { + RTC_DCHECK(thread_checker_.CalledOnValidThread()); stun_message_observer_ = std::move(observer); } @@ -282,6 +309,7 @@ class TurnServer : public sigslot::has_slots<> { ProtocolType> ServerSocketMap; rtc::Thread* thread_; + rtc::ThreadChecker thread_checker_; std::string nonce_key_; std::string realm_; std::string software_; diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc index 186da00b96..1e61604019 100644 --- a/pc/peerconnection_integrationtest.cc +++ b/pc/peerconnection_integrationtest.cc @@ -1116,12 +1116,26 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { } ~PeerConnectionIntegrationBaseTest() { + // The PeerConnections should deleted before the TurnCustomizers. + // A TurnPort is created with a raw pointer to a TurnCustomizer. The + // TurnPort has the same lifetime as the PeerConnection, so it's expected + // that the TurnCustomizer outlives the life of the PeerConnection or else + // when Send() is called it will hit a seg fault. if (caller_) { caller_->set_signaling_message_receiver(nullptr); + delete SetCallerPcWrapperAndReturnCurrent(nullptr); } if (callee_) { callee_->set_signaling_message_receiver(nullptr); + delete SetCalleePcWrapperAndReturnCurrent(nullptr); } + + // If turn servers were created for the test they need to be destroyed on + // the network thread. + network_thread()->Invoke(RTC_FROM_HERE, [this] { + turn_servers_.clear(); + turn_customizers_.clear(); + }); } bool SignalingStateStable() { @@ -1286,6 +1300,52 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { std::move(dependencies), nullptr); } + cricket::TestTurnServer* CreateTurnServer( + rtc::SocketAddress internal_address, + rtc::SocketAddress external_address, + cricket::ProtocolType type = cricket::ProtocolType::PROTO_UDP, + const std::string& common_name = "test turn server") { + rtc::Thread* thread = network_thread(); + std::unique_ptr turn_server = + network_thread()->Invoke>( + RTC_FROM_HERE, + [thread, internal_address, external_address, type, common_name] { + return rtc::MakeUnique( + thread, internal_address, external_address, type, + /*ignore_bad_certs=*/true, common_name); + }); + turn_servers_.push_back(std::move(turn_server)); + // Interactions with the turn server should be done on the network thread. + return turn_servers_.back().get(); + } + + cricket::TestTurnCustomizer* CreateTurnCustomizer() { + std::unique_ptr turn_customizer = + network_thread()->Invoke>( + RTC_FROM_HERE, + [] { return rtc::MakeUnique(); }); + turn_customizers_.push_back(std::move(turn_customizer)); + // Interactions with the turn customizer should be done on the network + // thread. + return turn_customizers_.back().get(); + } + + // Checks that the function counters for a TestTurnCustomizer are greater than + // 0. + void ExpectTurnCustomizerCountersIncremented( + cricket::TestTurnCustomizer* turn_customizer) { + unsigned int allow_channel_data_counter = + network_thread()->Invoke( + RTC_FROM_HERE, [turn_customizer] { + return turn_customizer->allow_channel_data_cnt_; + }); + EXPECT_GT(allow_channel_data_counter, 0u); + unsigned int modify_counter = network_thread()->Invoke( + RTC_FROM_HERE, + [turn_customizer] { return turn_customizer->modify_cnt_; }); + EXPECT_GT(modify_counter, 0u); + } + // Once called, SDP blobs and ICE candidates will be automatically signaled // between PeerConnections. void ConnectFakeSignaling() { @@ -1499,6 +1559,11 @@ class PeerConnectionIntegrationBaseTest : public testing::Test { // later. std::unique_ptr network_thread_; std::unique_ptr worker_thread_; + // The turn servers and turn customizers should be accessed & deleted on the + // network thread to avoid a race with the socket read/write that occurs + // on the network thread. + std::vector> turn_servers_; + std::vector> turn_customizers_; std::unique_ptr caller_; std::unique_ptr callee_; }; @@ -3799,17 +3864,19 @@ TEST_P(PeerConnectionIntegrationTest, EndToEndConnectionTimeWithTurnTurnPair) { 3478}; static const rtc::SocketAddress turn_server_2_external_address{"99.99.99.1", 0}; - cricket::TestTurnServer turn_server_1(network_thread(), - turn_server_1_internal_address, - turn_server_1_external_address); - cricket::TestTurnServer turn_server_2(network_thread(), - turn_server_2_internal_address, - turn_server_2_external_address); + cricket::TestTurnServer* turn_server_1 = CreateTurnServer( + turn_server_1_internal_address, turn_server_1_external_address); + cricket::TestTurnServer* turn_server_2 = CreateTurnServer( + turn_server_2_internal_address, turn_server_2_external_address); // Bypass permission check on received packets so media can be sent before // the candidate is signaled. - turn_server_1.set_enable_permission_checks(false); - turn_server_2.set_enable_permission_checks(false); + network_thread()->Invoke(RTC_FROM_HERE, [turn_server_1] { + turn_server_1->set_enable_permission_checks(false); + }); + network_thread()->Invoke(RTC_FROM_HERE, [turn_server_2] { + turn_server_2->set_enable_permission_checks(false); + }); PeerConnectionInterface::RTCConfiguration client_1_config; webrtc::PeerConnectionInterface::IceServer ice_server_1; @@ -3846,10 +3913,6 @@ TEST_P(PeerConnectionIntegrationTest, EndToEndConnectionTimeWithTurnTurnPair) { caller()->CreateAndSetAndSignalOffer(); EXPECT_TRUE_SIMULATED_WAIT(DtlsConnected(), total_connection_time_ms, fake_clock); - // Need to free the clients here since they're using things we created on - // the stack. - delete SetCallerPcWrapperAndReturnCurrent(nullptr); - delete SetCalleePcWrapperAndReturnCurrent(nullptr); } // Verify that a TurnCustomizer passed in through RTCConfiguration @@ -3864,12 +3927,10 @@ TEST_P(PeerConnectionIntegrationTest, TurnCustomizerUsedForTurnConnections) { 3478}; static const rtc::SocketAddress turn_server_2_external_address{"99.99.99.1", 0}; - cricket::TestTurnServer turn_server_1(network_thread(), - turn_server_1_internal_address, - turn_server_1_external_address); - cricket::TestTurnServer turn_server_2(network_thread(), - turn_server_2_internal_address, - turn_server_2_external_address); + CreateTurnServer(turn_server_1_internal_address, + turn_server_1_external_address); + CreateTurnServer(turn_server_2_internal_address, + turn_server_2_external_address); PeerConnectionInterface::RTCConfiguration client_1_config; webrtc::PeerConnectionInterface::IceServer ice_server_1; @@ -3878,8 +3939,8 @@ TEST_P(PeerConnectionIntegrationTest, TurnCustomizerUsedForTurnConnections) { ice_server_1.password = "test"; client_1_config.servers.push_back(ice_server_1); client_1_config.type = webrtc::PeerConnectionInterface::kRelay; - auto customizer1 = rtc::MakeUnique(); - client_1_config.turn_customizer = customizer1.get(); + auto* customizer1 = CreateTurnCustomizer(); + client_1_config.turn_customizer = customizer1; PeerConnectionInterface::RTCConfiguration client_2_config; webrtc::PeerConnectionInterface::IceServer ice_server_2; @@ -3888,8 +3949,8 @@ TEST_P(PeerConnectionIntegrationTest, TurnCustomizerUsedForTurnConnections) { ice_server_2.password = "test"; client_2_config.servers.push_back(ice_server_2); client_2_config.type = webrtc::PeerConnectionInterface::kRelay; - auto customizer2 = rtc::MakeUnique(); - client_2_config.turn_customizer = customizer2.get(); + auto* customizer2 = CreateTurnCustomizer(); + client_2_config.turn_customizer = customizer2; ASSERT_TRUE( CreatePeerConnectionWrappersWithConfig(client_1_config, client_2_config)); @@ -3904,16 +3965,8 @@ TEST_P(PeerConnectionIntegrationTest, TurnCustomizerUsedForTurnConnections) { caller()->CreateAndSetAndSignalOffer(); ASSERT_TRUE_WAIT(DtlsConnected(), kDefaultTimeout); - EXPECT_GT(customizer1->allow_channel_data_cnt_, 0u); - EXPECT_GT(customizer1->modify_cnt_, 0u); - - EXPECT_GT(customizer2->allow_channel_data_cnt_, 0u); - EXPECT_GT(customizer2->modify_cnt_, 0u); - - // Need to free the clients here since they're using things we created on - // the stack. - delete SetCallerPcWrapperAndReturnCurrent(nullptr); - delete SetCalleePcWrapperAndReturnCurrent(nullptr); + ExpectTurnCustomizerCountersIncremented(customizer1); + ExpectTurnCustomizerCountersIncremented(customizer2); } // Verifies that you can use TCP instead of UDP to connect to a TURN server and @@ -3924,9 +3977,8 @@ TEST_P(PeerConnectionIntegrationTest, TCPUsedForTurnConnections) { static const rtc::SocketAddress turn_server_external_address{"88.88.88.1", 0}; // Enable TCP for the fake turn server. - cricket::TestTurnServer turn_server( - network_thread(), turn_server_internal_address, - turn_server_external_address, cricket::PROTO_TCP); + CreateTurnServer(turn_server_internal_address, turn_server_external_address, + cricket::PROTO_TCP); webrtc::PeerConnectionInterface::IceServer ice_server; ice_server.urls.push_back("turn:88.88.88.0:3478?transport=tcp"); @@ -3971,10 +4023,8 @@ TEST_P(PeerConnectionIntegrationTest, // Enable TCP-TLS for the fake turn server. We need to pass in 88.88.88.0 so // that host name verification passes on the fake certificate. - cricket::TestTurnServer turn_server( - network_thread(), turn_server_internal_address, - turn_server_external_address, cricket::PROTO_TLS, - /*ignore_bad_certs=*/true, "88.88.88.0"); + CreateTurnServer(turn_server_internal_address, turn_server_external_address, + cricket::PROTO_TLS, "88.88.88.0"); webrtc::PeerConnectionInterface::IceServer ice_server; ice_server.urls.push_back("turns:88.88.88.0:3478?transport=tcp"); @@ -4023,11 +4073,6 @@ TEST_P(PeerConnectionIntegrationTest, EXPECT_GT(client_1_cert_verifier->call_count_, 0u); EXPECT_GT(client_2_cert_verifier->call_count_, 0u); - - // Need to free the clients here since they're using things we created on - // the stack. - delete SetCallerPcWrapperAndReturnCurrent(nullptr); - delete SetCalleePcWrapperAndReturnCurrent(nullptr); } TEST_P(PeerConnectionIntegrationTest, @@ -4038,10 +4083,8 @@ TEST_P(PeerConnectionIntegrationTest, // Enable TCP-TLS for the fake turn server. We need to pass in 88.88.88.0 so // that host name verification passes on the fake certificate. - cricket::TestTurnServer turn_server( - network_thread(), turn_server_internal_address, - turn_server_external_address, cricket::PROTO_TLS, - /*ignore_bad_certs=*/true, "88.88.88.0"); + CreateTurnServer(turn_server_internal_address, turn_server_external_address, + cricket::PROTO_TLS, "88.88.88.0"); webrtc::PeerConnectionInterface::IceServer ice_server; ice_server.urls.push_back("turns:88.88.88.0:3478?transport=tcp"); @@ -4095,11 +4138,6 @@ TEST_P(PeerConnectionIntegrationTest, EXPECT_GT(client_1_cert_verifier->call_count_, 0u); EXPECT_GT(client_2_cert_verifier->call_count_, 0u); - - // Need to free the clients here since they're using things we created on - // the stack. - delete SetCallerPcWrapperAndReturnCurrent(nullptr); - delete SetCalleePcWrapperAndReturnCurrent(nullptr); } // Test that audio and video flow end-to-end when codec names don't use the