Fixing invalid operator< implementation.

It was possible that "A < B" and "B < A" both evaluated to true.
This manifested as an assert on Windows, and a memory leak on Linux.

Note that the concept of "less than" is meaningless for this object.
The operator is only needed so the object can be used as a key in an
std::map.

BUG=webrtc:6068
R=honghaiz@webrtc.org, kjellander@webrtc.org, skvlad@webrtc.org

Review URL: https://codereview.webrtc.org/2187913002 .

Cr-Commit-Position: refs/heads/master@{#13598}
This commit is contained in:
Taylor Brandstetter
2016-08-01 16:37:14 -07:00
parent 88532898db
commit 734262c765
5 changed files with 74 additions and 10 deletions

View File

@ -502,6 +502,7 @@ if (rtc_include_tests) {
"p2p/base/transportcontroller_unittest.cc",
"p2p/base/transportdescriptionfactory_unittest.cc",
"p2p/base/turnport_unittest.cc",
"p2p/base/turnserver_unittest.cc",
"p2p/client/basicportallocator_unittest.cc",
"p2p/stunprober/stunprober_unittest.cc",
]

View File

@ -10,6 +10,8 @@
#include "webrtc/p2p/base/turnserver.h"
#include <tuple> // for std::tie
#include "webrtc/p2p/base/asyncstuntcpsocket.h"
#include "webrtc/p2p/base/common.h"
#include "webrtc/p2p/base/packetsocketfactory.h"
@ -544,7 +546,7 @@ bool TurnServerConnection::operator==(const TurnServerConnection& c) const {
}
bool TurnServerConnection::operator<(const TurnServerConnection& c) const {
return src_ < c.src_ || dst_ < c.dst_ || proto_ < c.proto_;
return std::tie(src_, dst_, proto_) < std::tie(c.src_, c.dst_, c.proto_);
}
std::string TurnServerConnection::ToString() const {

View File

@ -0,0 +1,68 @@
/*
* Copyright 2016 The WebRTC Project Authors. All rights reserved.
*
* Use of this source code is governed by a BSD-style license
* that can be found in the LICENSE file in the root of the source
* tree. An additional intellectual property rights grant can be found
* in the file PATENTS. All contributing project authors may
* be found in the AUTHORS file in the root of the source tree.
*/
#include "webrtc/base/gunit.h"
#include "webrtc/base/physicalsocketserver.h"
#include "webrtc/base/virtualsocketserver.h"
#include "webrtc/p2p/base/basicpacketsocketfactory.h"
#include "webrtc/p2p/base/turnserver.h"
// NOTE: This is a work in progress. Currently this file only has tests for
// TurnServerConnection, a primitive class used by TurnServer.
namespace cricket {
class TurnServerConnectionTest : public testing::Test {
public:
TurnServerConnectionTest() : vss_(&pss_), ss_scope_(&vss_) {}
void ExpectEqual(const TurnServerConnection& a,
const TurnServerConnection& b) {
EXPECT_TRUE(a == b);
EXPECT_FALSE(a < b);
EXPECT_FALSE(b < a);
}
void ExpectNotEqual(const TurnServerConnection& a,
const TurnServerConnection& b) {
EXPECT_FALSE(a == b);
// We don't care which is less than the other, as long as only one is less
// than the other.
EXPECT_TRUE((a < b) != (b < a));
}
protected:
rtc::PhysicalSocketServer pss_;
rtc::VirtualSocketServer vss_;
rtc::SocketServerScope ss_scope_;
// Since this is constructed after |ss_scope_|, it will pick up |ss_scope_|'s
// socket server.
rtc::BasicPacketSocketFactory socket_factory_;
};
TEST_F(TurnServerConnectionTest, ComparisonOperators) {
std::unique_ptr<rtc::AsyncPacketSocket> socket1(
socket_factory_.CreateUdpSocket(rtc::SocketAddress("1.1.1.1", 1), 0, 0));
std::unique_ptr<rtc::AsyncPacketSocket> socket2(
socket_factory_.CreateUdpSocket(rtc::SocketAddress("2.2.2.2", 2), 0, 0));
TurnServerConnection connection1(socket2->GetLocalAddress(), PROTO_UDP,
socket1.get());
TurnServerConnection connection2(socket2->GetLocalAddress(), PROTO_UDP,
socket1.get());
TurnServerConnection connection3(socket1->GetLocalAddress(), PROTO_UDP,
socket2.get());
TurnServerConnection connection4(socket2->GetLocalAddress(), PROTO_TCP,
socket1.get());
ExpectEqual(connection1, connection2);
ExpectNotEqual(connection1, connection3);
ExpectNotEqual(connection1, connection4);
}
} // namespace cricket

View File

@ -1291,17 +1291,9 @@ TEST_F(BasicPortAllocatorTest, TestIPv6TurnPortPrunesIPv4TurnPorts) {
rtc::SocketAddress(kTurnUdpExtAddr.ipaddr(), 0));
}
// Test has an assert error on win_x64_dbg and win_dbg. See: webrtc:6068
#if defined(WEBRTC_WIN)
#define MAYBE_TestEachInterfaceHasItsOwnTurnPorts \
DISABLED_TestEachInterfaceHasItsOwnTurnPort
#else
#define MAYBE_TestEachInterfaceHasItsOwnTurnPorts \
TestEachInterfaceHasItsOwnTurnPorts
#endif
// Tests that if prune_turn_ports is set, each network interface
// will has its own set of TurnPorts based on their priorities.
TEST_F(BasicPortAllocatorTest, MAYBE_TestEachInterfaceHasItsOwnTurnPorts) {
TEST_F(BasicPortAllocatorTest, TestEachInterfaceHasItsOwnTurnPorts) {
turn_server_.AddInternalSocket(kTurnTcpIntAddr, PROTO_TCP);
turn_server_.AddInternalSocket(kTurnUdpIntIPv6Addr, PROTO_UDP);
turn_server_.AddInternalSocket(kTurnTcpIntIPv6Addr, PROTO_TCP);

View File

@ -117,6 +117,7 @@
'p2p/base/transportdescriptionfactory_unittest.cc',
'p2p/base/tcpport_unittest.cc',
'p2p/base/turnport_unittest.cc',
'p2p/base/turnserver_unittest.cc',
'p2p/client/basicportallocator_unittest.cc',
'p2p/stunprober/stunprober_unittest.cc',
],