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

Unflake P2PTransportChannelTest.TurnToTurnPresumedWritable.

Browse Source 
Some messages were processed after involved objects were destructed,
a.k.a. 'use after free'.

This CL fixes that by disconnecting signals before fixture destruction,
honoring CreateChannel/DestroyChannel symmetry and following what is
done in similar test cases.

Bug: webrtc:11269
Change-Id: I122aca70a9978b752edc01e5f31583f4425f3624
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/165685
Reviewed-by: Qingsi Wang <qingsi@webrtc.org>
Commit-Queue: Yves Gerey <yvesg@google.com>
Cr-Commit-Position: refs/heads/master@{#30214}
This commit is contained in:
Yves Gerey
2020-01-10 17:39:11 +01:00
committed by Commit Bot
parent b408bb7b95
commit bcea217667
1 changed files with 15 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
p2p/base/p2p_transport_channel_unittest.cc
View File

@ -426,6 +426,7 @@ class P2PTransportChannelTestBase : public ::testing::Test,
channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker()); channel->SetIceTiebreaker(GetEndpoint(endpoint)->GetIceTiebreaker());
return channel; return channel;
} }
void DestroyChannels() { void DestroyChannels() {
main_.Clear(this); main_.Clear(this);
ep1_.cd1_.ch_.reset(); ep1_.cd1_.ch_.reset();
@ -737,6 +738,7 @@ class P2PTransportChannelTestBase : public ::testing::Test,
ep2_ch1()->selected_connection()); ep2_ch1()->selected_connection());
TestSendRecv(&clock); TestSendRecv(&clock);
DestroyChannels();
} }
void TestPacketInfoIsSet(rtc::PacketInfo info) { void TestPacketInfoIsSet(rtc::PacketInfo info) {
@ -1545,6 +1547,7 @@ class P2PTransportRegatherAllNetworksTest : public P2PTransportChannelTest {
// Make sure we can communicate over the new connection too. // Make sure we can communicate over the new connection too.
TestSendRecv(&clock); TestSendRecv(&clock);
DestroyChannels();
} }
}; };
@ -1961,6 +1964,7 @@ TEST_F(P2PTransportChannelTest, TestDefaultDscpValue) {
GetEndpoint(1)->cd1_.ch_->SetOption(rtc::Socket::OPT_DSCP, rtc::DSCP_AF41); GetEndpoint(1)->cd1_.ch_->SetOption(rtc::Socket::OPT_DSCP, rtc::DSCP_AF41);
EXPECT_EQ(rtc::DSCP_AF41, GetEndpoint(0)->cd1_.ch_->DefaultDscpValue()); EXPECT_EQ(rtc::DSCP_AF41, GetEndpoint(0)->cd1_.ch_->DefaultDscpValue());
EXPECT_EQ(rtc::DSCP_AF41, GetEndpoint(1)->cd1_.ch_->DefaultDscpValue()); EXPECT_EQ(rtc::DSCP_AF41, GetEndpoint(1)->cd1_.ch_->DefaultDscpValue());
DestroyChannels();
} }
// Verify IPv6 connection is preferred over IPv4. // Verify IPv6 connection is preferred over IPv4.
@ -2088,6 +2092,7 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionBeforeDoneGathering) {
Contains(ep1_ch1()->selected_connection()->PortForTest())); Contains(ep1_ch1()->selected_connection()->PortForTest()));
EXPECT_THAT(pooled_ports_2, EXPECT_THAT(pooled_ports_2,
Contains(ep2_ch1()->selected_connection()->PortForTest())); Contains(ep2_ch1()->selected_connection()->PortForTest()));
DestroyChannels();
} }
// Test that a connection succeeds when the P2PTransportChannel uses a pooled // Test that a connection succeeds when the P2PTransportChannel uses a pooled
@ -2130,6 +2135,7 @@ TEST_F(P2PTransportChannelTest, TestUsingPooledSessionAfterDoneGathering) {
Contains(ep1_ch1()->selected_connection()->PortForTest())); Contains(ep1_ch1()->selected_connection()->PortForTest()));
EXPECT_THAT(pooled_ports_2, EXPECT_THAT(pooled_ports_2,
Contains(ep2_ch1()->selected_connection()->PortForTest())); Contains(ep2_ch1()->selected_connection()->PortForTest()));
DestroyChannels();
} }
// Test that when the "presume_writable_when_fully_relayed" flag is set to // Test that when the "presume_writable_when_fully_relayed" flag is set to
@ -2170,6 +2176,8 @@ TEST_F(P2PTransportChannelTest, TurnToTurnPresumedWritable) {
const char* data = "test"; const char* data = "test";
int len = static_cast<int>(strlen(data)); int len = static_cast<int>(strlen(data));
EXPECT_EQ(len, SendData(ep1_ch1(), data, len)); EXPECT_EQ(len, SendData(ep1_ch1(), data, len));
// Prevent pending messages to access endpoints after their destruction.
DestroyChannels();
} }
// Test that a TURN/peer reflexive candidate pair is also presumed writable. // Test that a TURN/peer reflexive candidate pair is also presumed writable.
@ -2294,6 +2302,7 @@ TEST_F(P2PTransportChannelTest, SignalReadyToSendWithPresumedWritable) {
virtual_socket_server()->SetSendingBlocked(false); virtual_socket_server()->SetSendingBlocked(false);
EXPECT_TRUE(GetEndpoint(0)->ready_to_send_); EXPECT_TRUE(GetEndpoint(0)->ready_to_send_);
EXPECT_EQ(len, SendData(ep1_ch1(), data, len)); EXPECT_EQ(len, SendData(ep1_ch1(), data, len));
DestroyChannels();
} }
// Test that role conflict error responses are sent as expected when receiving a // Test that role conflict error responses are sent as expected when receiving a
@ -2961,6 +2970,7 @@ TEST_F(P2PTransportChannelMultihomedTest, TestGetState) {
ep1_ch1()->GetState(), kShortTimeout, clock); ep1_ch1()->GetState(), kShortTimeout, clock);
EXPECT_EQ_SIMULATED_WAIT(IceTransportState::STATE_COMPLETED, EXPECT_EQ_SIMULATED_WAIT(IceTransportState::STATE_COMPLETED,
ep2_ch1()->GetState(), kShortTimeout, clock); ep2_ch1()->GetState(), kShortTimeout, clock);
DestroyChannels();
} }
// Tests that when a network interface becomes inactive, if Continual Gathering // Tests that when a network interface becomes inactive, if Continual Gathering
@ -5235,6 +5245,7 @@ TEST_F(P2PTransportChannelTest,
EXPECT_EQ( EXPECT_EQ(
"1.1.1.1:1", "1.1.1.1:1",
ep1_ch1()->connections()[0]->remote_candidate().address().ToString()); ep1_ch1()->connections()[0]->remote_candidate().address().ToString());
DestroyChannels();
} }
class MockMdnsResponder : public webrtc::MdnsResponderInterface { class MockMdnsResponder : public webrtc::MdnsResponderInterface {
@ -5346,6 +5357,7 @@ TEST_F(P2PTransportChannelTest,
kDefaultTimeout, clock); kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE, EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type()); ep1_ch1()->selected_connection()->remote_candidate().type());
DestroyChannels();
} }
// A similar test as SurfaceHostCandidateOnCandidateFilterChangeFromRelayToAll, // A similar test as SurfaceHostCandidateOnCandidateFilterChangeFromRelayToAll,
@ -5412,6 +5424,7 @@ TEST_F(P2PTransportChannelTest,
kDefaultTimeout, clock); kDefaultTimeout, clock);
EXPECT_EQ(RELAY_PORT_TYPE, EXPECT_EQ(RELAY_PORT_TYPE,
ep1_ch1()->selected_connection()->remote_candidate().type()); ep1_ch1()->selected_connection()->remote_candidate().type());
DestroyChannels();
} }
// This is the complement to // This is the complement to
@ -5450,6 +5463,7 @@ TEST_F(P2PTransportChannelTest,
ep2->allocator_->SetCandidateFilter(CF_ALL); ep2->allocator_->SetCandidateFilter(CF_ALL);
EXPECT_EQ(RELAY_PORT_TYPE, EXPECT_EQ(RELAY_PORT_TYPE,
ep2_ch1()->selected_connection()->local_candidate().type()); ep2_ch1()->selected_connection()->local_candidate().type());
DestroyChannels();
} }
// Test that when the candidate filter is updated to be more restrictive, // Test that when the candidate filter is updated to be more restrictive,
@ -5517,6 +5531,7 @@ TEST_F(P2PTransportChannelTest,
ep1->allocator_->SetCandidateFilter(CF_NONE); ep1->allocator_->SetCandidateFilter(CF_NONE);
SIMULATED_WAIT(false, kDefaultTimeout, clock); SIMULATED_WAIT(false, kDefaultTimeout, clock);
test_invariants(); test_invariants();
DestroyChannels();
} }
TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) { TEST_F(P2PTransportChannelPingTest, TestInitialSelectDampening0) {