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

Use WeakPtr<Port> in Connection classes.

Browse Source 
This is to aid with catching issues whereby a connection object might
have a bad reference back to a port object, e.g. inside of an async
callback.

Bug: webrtc:13892
Change-Id: I56503fedc2865919713b10f236ce023554c68ded
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/257164
Reviewed-by: Niels Moller <nisse@webrtc.org>
Commit-Queue: Tomas Gunnarsson <tommi@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#36394}
This commit is contained in:
Tommi
2022-03-30 20:13:06 +02:00
committed by WebRTC LUCI CQ
parent 0cfaa610ca
commit d7e5cfb3cf
8 changed files with 43 additions and 22 deletions
Download Patch File Download Diff File Expand all files Collapse all files

15
p2p/base/connection.cc
View File

@ -283,12 +283,12 @@ int ConnectionRequest::resend_delay() {
return CONNECTION_RESPONSE_TIMEOUT;
}
Connection::Connection(Port* port,
Connection::Connection(rtc::WeakPtr<Port> port,
size_t index,
const Candidate& remote_candidate)
: network_thread_(port->thread()),
id_(rtc::CreateRandomId()),
port_(port),
port_(std::move(port)),
local_candidate_index_(index),
remote_candidate_(remote_candidate),
recv_rate_tracker_(100, 10u),
@ -298,7 +298,7 @@ Connection::Connection(Port* port,
connected_(true),
pruned_(false),
use_candidate_attr_(false),
requests_(port->thread()),
requests_(port_->thread()),
rtt_(DEFAULT_RTT),
last_ping_sent_(0),
last_ping_received_(0),
@ -1422,7 +1422,7 @@ void Connection::OnConnectionRequestSent(ConnectionRequest* request) {
}
void Connection::HandleRoleConflictFromPeer() {
port_->SignalRoleConflict(port_);
port_->SignalRoleConflict(port());
}
IceCandidatePairState Connection::state() const {
@ -1618,10 +1618,15 @@ void Connection::ForgetLearnedState() {
pings_since_last_response_.clear();
}
ProxyConnection::ProxyConnection(rtc::WeakPtr<Port> port,
size_t index,
const Candidate& remote_candidate)
: Connection(std::move(port), index, remote_candidate) {}
ProxyConnection::ProxyConnection(Port* port,
size_t index,
const Candidate& remote_candidate)
: Connection(port, index, remote_candidate) {}
: ProxyConnection(port->NewWeakPtr(), index, remote_candidate) {}
int ProxyConnection::Send(const void* data,
size_t size,

18
p2p/base/connection.h
View File

@ -29,6 +29,7 @@
#include "rtc_base/network.h"
#include "rtc_base/numerics/event_based_exponential_moving_average.h"
#include "rtc_base/rate_tracker.h"
#include "rtc_base/weak_ptr.h"
namespace cricket {
@ -310,8 +311,8 @@ class Connection : public CandidatePairInterface, public sigslot::has_slots<> {
void SendResponseMessage(const StunMessage& response);
// An accessor for unit tests.
Port* PortForTest() { return port_; }
const Port* PortForTest() const { return port_; }
Port* PortForTest() { return port_.get(); }
const Port* PortForTest() const { return port_.get(); }
// Public for unit tests.
uint32_t acked_nomination() const;
@ -319,7 +320,7 @@ class Connection : public CandidatePairInterface, public sigslot::has_slots<> {
protected:
// Constructs a new connection to the given remote port.
Connection(Port* port, size_t index, const Candidate& candidate);
Connection(rtc::WeakPtr<Port> port, size_t index, const Candidate& candidate);
// Called back when StunRequestManager has a stun packet to send
void OnSendStunPacket(const void* data, size_t size, StunRequest* req);
@ -348,8 +349,8 @@ class Connection : public CandidatePairInterface, public sigslot::has_slots<> {
void set_connected(bool value);
// The local port where this connection sends and receives packets.
Port* port() { return port_; }
const Port* port() const { return port_; }
Port* port() { return port_.get(); }
const Port* port() const { return port_.get(); }
// NOTE: A pointer to the network thread is held by `port_` so in theory we
// shouldn't need to hold on to this pointer here, but rather defer to
@ -358,7 +359,7 @@ class Connection : public CandidatePairInterface, public sigslot::has_slots<> {
// TODO(tommi): This ^^^ should be fixed.
webrtc::TaskQueueBase* const network_thread_;
const uint32_t id_;
Port* const port_;
rtc::WeakPtr<Port> port_;
size_t local_candidate_index_ RTC_GUARDED_BY(network_thread_);
Candidate remote_candidate_;
@ -470,6 +471,11 @@ class Connection : public CandidatePairInterface, public sigslot::has_slots<> {
// ProxyConnection defers all the interesting work to the port.
class ProxyConnection : public Connection {
public:
ProxyConnection(rtc::WeakPtr<Port> port,
size_t index,
const Candidate& remote_candidate);
// TODO(tommi): Remove this ctor once it's no longer needed.
ProxyConnection(Port* port, size_t index, const Candidate& remote_candidate);
int Send(const void* data,

3
p2p/base/port.h
View File

@ -384,6 +384,9 @@ class Port : public PortInterface,
const std::string& relay_protocol,
const rtc::SocketAddress& base_address);
// TODO(tommi): Make protected after updating ProxyConnection.
rtc::WeakPtr<Port> NewWeakPtr() { return weak_factory_.GetWeakPtr(); }
protected:
enum { MSG_DESTROY_IF_DEAD = 0, MSG_FIRST_AVAILABLE };

2
p2p/base/port_unittest.cc
View File

@ -200,7 +200,7 @@ class TestPort : public Port {
virtual Connection* CreateConnection(const Candidate& remote_candidate,
CandidateOrigin origin) {
Connection* conn = new ProxyConnection(this, 0, remote_candidate);
Connection* conn = new ProxyConnection(NewWeakPtr(), 0, remote_candidate);
AddOrReplaceConnection(conn);
// Set use-candidate attribute flag as this will add USE-CANDIDATE attribute
// in STUN binding requests.

2
p2p/base/stun_port.cc
View File

@ -275,7 +275,7 @@ Connection* UDPPort::CreateConnection(const Candidate& address,
mdns_name_registration_status() !=
MdnsNameRegistrationStatus::kNotStarted);
Connection* conn = new ProxyConnection(this, 0, address);
Connection* conn = new ProxyConnection(NewWeakPtr(), 0, address);
AddOrReplaceConnection(conn);
return conn;
}

14
p2p/base/tcp_port.cc
View File

@ -68,6 +68,7 @@
#include <errno.h>
#include <utility>
#include <vector>
#include "absl/algorithm/container.h"
@ -157,11 +158,11 @@ Connection* TCPPort::CreateConnection(const Candidate& address,
// so we need to hand off the "read packet" responsibility to
// TCPConnection.
socket->SignalReadPacket.disconnect(this);
conn = new TCPConnection(this, address, socket);
conn = new TCPConnection(NewWeakPtr(), address, socket);
} else {
// Outgoing connection, which will create a new socket for which we still
// need to connect SignalReadyToSend and SignalSentPacket.
conn = new TCPConnection(this, address);
conn = new TCPConnection(NewWeakPtr(), address);
if (conn->socket()) {
conn->socket()->SignalReadyToSend.connect(this, &TCPPort::OnReadyToSend);
conn->socket()->SignalSentPacket.connect(this, &TCPPort::OnSentPacket);
@ -343,16 +344,17 @@ void TCPPort::OnReadyToSend(rtc::AsyncPacketSocket* socket) {
// `ice_unwritable_timeout` in IceConfig when determining the writability state.
// Replace this constant with the config parameter assuming the default value if
// we decide it is also applicable here.
TCPConnection::TCPConnection(TCPPort* port,
TCPConnection::TCPConnection(rtc::WeakPtr<Port> tcp_port,
const Candidate& candidate,
rtc::AsyncPacketSocket* socket)
: Connection(port, 0, candidate),
: Connection(std::move(tcp_port), 0, candidate),
socket_(socket),
error_(0),
outgoing_(socket == NULL),
connection_pending_(false),
pretending_to_be_writable_(false),
reconnection_timeout_(cricket::CONNECTION_WRITE_CONNECT_TIMEOUT) {
RTC_DCHECK_EQ(port()->GetProtocol(), PROTO_TCP); // Needs to be TCPPort.
if (outgoing_) {
CreateOutgoingTcpSocket();
} else {
@ -360,7 +362,7 @@ TCPConnection::TCPConnection(TCPPort* port,
// what's being checked in OnConnect, but just DCHECKing here.
RTC_LOG(LS_VERBOSE) << ToString() << ": socket ipaddr: "
<< socket_->GetLocalAddress().ToSensitiveString()
<< ", port() Network:" << port->Network()->ToString();
<< ", port() Network:" << port()->Network()->ToString();
RTC_DCHECK(absl::c_any_of(
port_->Network()->GetIPs(), [this](const rtc::InterfaceAddress& addr) {
return socket_->GetLocalAddress().ipaddr() == addr;
@ -399,7 +401,7 @@ int TCPConnection::Send(const void* data,
}
stats_.sent_total_packets++;
rtc::PacketOptions modified_options(options);
static_cast<TCPPort*>(port_)->CopyPortInformationToPacketInfo(
tcp_port()->CopyPortInformationToPacketInfo(
&modified_options.info_signaled_after_sent);
int sent = socket_->Send(data, size, modified_options);
int64_t now = rtc::TimeMillis();

9
p2p/base/tcp_port.h
View File

@ -127,9 +127,9 @@ class TCPPort : public Port {
class TCPConnection : public Connection {
public:
// Connection is outgoing unless socket is specified
TCPConnection(TCPPort* port,
TCPConnection(rtc::WeakPtr<Port> tcp_port,
const Candidate& candidate,
rtc::AsyncPacketSocket* socket = 0);
rtc::AsyncPacketSocket* socket = nullptr);
~TCPConnection() override;
int Send(const void* data,
@ -169,6 +169,11 @@ class TCPConnection : public Connection {
const int64_t& packet_time_us);
void OnReadyToSend(rtc::AsyncPacketSocket* socket);
TCPPort* tcp_port() {
RTC_DCHECK_EQ(port()->GetProtocol(), PROTO_TCP);
return static_cast<TCPPort*>(port());
}
std::unique_ptr<rtc::AsyncPacketSocket> socket_;
int error_;
bool outgoing_;

2
p2p/base/turn_port.cc
View File

@ -588,7 +588,7 @@ Connection* TurnPort::CreateConnection(const Candidate& remote_candidate,
next_channel_number_++;
}
ProxyConnection* conn =
new ProxyConnection(this, index, remote_candidate);
new ProxyConnection(NewWeakPtr(), index, remote_candidate);
AddOrReplaceConnection(conn);
return conn;
}