From 9897649336d3cc3596a8b7ecd6e85531634ff822 Mon Sep 17 00:00:00 2001 From: Tommi Date: Mon, 31 Jan 2022 11:19:11 +0100 Subject: [PATCH] Thread checks for the Connection class. Following [1], add many more checks for safe access to member variables. This change is effectively a no-op, but landed separately from the earlier change that's smaller but contains a fundamental assumption gleaned from the implementation (and its use). [1]: https://webrtc-review.googlesource.com/c/src/+/249942 Bug: webrtc:11988 Change-Id: I1568e2160c9faa6993c5b68044312f83d00e4815 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249943 Auto-Submit: Tomas Gunnarsson Reviewed-by: Niels Moller Commit-Queue: Tomas Gunnarsson Cr-Commit-Position: refs/heads/main@{#35850} --- p2p/base/connection.cc | 234 +++++++++++++++++++++++++++++++++++++++-- p2p/base/connection.h | 201 ++++++++++++++++++----------------- 2 files changed, 326 insertions(+), 109 deletions(-) diff --git a/p2p/base/connection.cc b/p2p/base/connection.cc index e0dcf7f160..8575d65945 100644 --- a/p2p/base/connection.cc +++ b/p2p/base/connection.cc @@ -167,6 +167,7 @@ ConnectionRequest::ConnectionRequest(Connection* connection) : StunRequest(new IceMessage()), connection_(connection) {} void ConnectionRequest::Prepare(StunMessage* request) { + RTC_DCHECK_RUN_ON(connection_->network_thread_); request->SetType(STUN_BINDING_REQUEST); std::string username; connection_->port()->CreateStunUsername( @@ -207,10 +208,10 @@ void ConnectionRequest::Prepare(StunMessage* request) { request->AddAttribute( std::make_unique(STUN_ATTR_USE_CANDIDATE)); } - if (connection_->nomination() && - connection_->nomination() != connection_->acked_nomination()) { + if (connection_->nomination_ && + connection_->nomination_ != connection_->acked_nomination()) { request->AddAttribute(std::make_unique( - STUN_ATTR_NOMINATION, connection_->nomination())); + STUN_ATTR_NOMINATION, connection_->nomination_)); } } else if (connection_->port()->GetIceRole() == ICEROLE_CONTROLLED) { request->AddAttribute(std::make_unique( @@ -257,18 +258,22 @@ void ConnectionRequest::Prepare(StunMessage* request) { } void ConnectionRequest::OnResponse(StunMessage* response) { + RTC_DCHECK_RUN_ON(connection_->network_thread_); connection_->OnConnectionRequestResponse(this, response); } void ConnectionRequest::OnErrorResponse(StunMessage* response) { + RTC_DCHECK_RUN_ON(connection_->network_thread_); connection_->OnConnectionRequestErrorResponse(this, response); } void ConnectionRequest::OnTimeout() { + RTC_DCHECK_RUN_ON(connection_->network_thread_); connection_->OnConnectionRequestTimeout(this); } void ConnectionRequest::OnSent() { + RTC_DCHECK_RUN_ON(connection_->network_thread_); connection_->OnConnectionRequestSent(this); // Each request is sent only once. After a single delay , the request will // time out. @@ -305,7 +310,7 @@ Connection::Connection(Port* port, time_created_ms_(rtc::TimeMillis()), field_trials_(&kDefaultFieldTrials), rtt_estimate_(DEFAULT_RTT_ESTIMATE_HALF_TIME_MS) { - RTC_DCHECK_RUN_ON(network_thread()); + RTC_DCHECK_RUN_ON(network_thread_); // All of our connections start in WAITING state. // TODO(mallinath) - Start connections from STATE_FROZEN. // Wire up to send stun packets @@ -314,7 +319,7 @@ Connection::Connection(Port* port, } Connection::~Connection() { - RTC_DCHECK_RUN_ON(network_thread()); + RTC_DCHECK_RUN_ON(network_thread_); } webrtc::TaskQueueBase* Connection::network_thread() const { @@ -322,6 +327,7 @@ webrtc::TaskQueueBase* Connection::network_thread() const { } const Candidate& Connection::local_candidate() const { + RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK(local_candidate_index_ < port_->Candidates().size()); return port_->Candidates()[local_candidate_index_]; } @@ -364,6 +370,7 @@ uint64_t Connection::priority() const { } void Connection::set_write_state(WriteState value) { + RTC_DCHECK_RUN_ON(network_thread_); WriteState old_value = write_state_; write_state_ = value; if (value != old_value) { @@ -374,6 +381,7 @@ void Connection::set_write_state(WriteState value) { } void Connection::UpdateReceiving(int64_t now) { + RTC_DCHECK_RUN_ON(network_thread_); bool receiving; if (last_ping_sent() < last_ping_response_received()) { // We consider any candidate pair that has its last connectivity check @@ -399,6 +407,7 @@ void Connection::UpdateReceiving(int64_t now) { } void Connection::set_state(IceCandidatePairState state) { + RTC_DCHECK_RUN_ON(network_thread_); IceCandidatePairState old_state = state_; state_ = state; if (state != old_state) { @@ -407,6 +416,7 @@ void Connection::set_state(IceCandidatePairState state) { } void Connection::set_connected(bool value) { + RTC_DCHECK_RUN_ON(network_thread_); bool old_value = connected_; connected_ = value; if (value != old_value) { @@ -415,27 +425,74 @@ void Connection::set_connected(bool value) { } } +bool Connection::use_candidate_attr() const { + RTC_DCHECK_RUN_ON(network_thread_); + return use_candidate_attr_; +} + void Connection::set_use_candidate_attr(bool enable) { + RTC_DCHECK_RUN_ON(network_thread_); use_candidate_attr_ = enable; } +void Connection::set_nomination(uint32_t value) { + RTC_DCHECK_RUN_ON(network_thread_); + nomination_ = value; +} + +uint32_t Connection::remote_nomination() const { + RTC_DCHECK_RUN_ON(network_thread_); + return remote_nomination_; +} + +bool Connection::nominated() const { + RTC_DCHECK_RUN_ON(network_thread_); + return acked_nomination_ || remote_nomination_; +} + int Connection::unwritable_timeout() const { + RTC_DCHECK_RUN_ON(network_thread_); return unwritable_timeout_.value_or(CONNECTION_WRITE_CONNECT_TIMEOUT); } +void Connection::set_unwritable_timeout(const absl::optional& value_ms) { + RTC_DCHECK_RUN_ON(network_thread_); + unwritable_timeout_ = value_ms; +} + int Connection::unwritable_min_checks() const { + RTC_DCHECK_RUN_ON(network_thread_); return unwritable_min_checks_.value_or(CONNECTION_WRITE_CONNECT_FAILURES); } +void Connection::set_unwritable_min_checks(const absl::optional& value) { + RTC_DCHECK_RUN_ON(network_thread_); + unwritable_min_checks_ = value; +} + int Connection::inactive_timeout() const { + RTC_DCHECK_RUN_ON(network_thread_); return inactive_timeout_.value_or(CONNECTION_WRITE_TIMEOUT); } +void Connection::set_inactive_timeout(const absl::optional& value) { + RTC_DCHECK_RUN_ON(network_thread_); + inactive_timeout_ = value; +} + int Connection::receiving_timeout() const { + RTC_DCHECK_RUN_ON(network_thread_); return receiving_timeout_.value_or(WEAK_CONNECTION_RECEIVE_TIMEOUT); } +void Connection::set_receiving_timeout( + absl::optional receiving_timeout_ms) { + RTC_DCHECK_RUN_ON(network_thread_); + receiving_timeout_ = receiving_timeout_ms; +} + void Connection::SetIceFieldTrials(const IceFieldTrials* field_trials) { + RTC_DCHECK_RUN_ON(network_thread_); field_trials_ = field_trials; rtt_estimate_.SetHalfTime(field_trials->rtt_estimate_halftime_ms); } @@ -443,6 +500,7 @@ void Connection::SetIceFieldTrials(const IceFieldTrials* field_trials) { void Connection::OnSendStunPacket(const void* data, size_t size, StunRequest* req) { + RTC_DCHECK_RUN_ON(network_thread_); rtc::PacketOptions options(port_->StunDscpValue()); options.info_signaled_after_sent.packet_type = rtc::PacketType::kIceConnectivityCheck; @@ -459,6 +517,7 @@ void Connection::OnSendStunPacket(const void* data, void Connection::OnReadPacket(const char* data, size_t size, int64_t packet_time_us) { + RTC_DCHECK_RUN_ON(network_thread_); std::unique_ptr msg; std::string remote_ufrag; const rtc::SocketAddress& addr(remote_candidate_.address()); @@ -542,6 +601,7 @@ void Connection::OnReadPacket(const char* data, } void Connection::HandleStunBindingOrGoogPingRequest(IceMessage* msg) { + RTC_DCHECK_RUN_ON(network_thread_); // This connection should now be receiving. ReceivedPing(msg->transaction_id()); if (webrtc::field_trial::IsEnabled("WebRTC-ExtraICEPing") && @@ -641,6 +701,7 @@ void Connection::HandleStunBindingOrGoogPingRequest(IceMessage* msg) { } void Connection::SendStunBindingResponse(const StunMessage* request) { + RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK(request->type() == STUN_BINDING_REQUEST); // Retrieve the username from the request. @@ -697,6 +758,7 @@ void Connection::SendStunBindingResponse(const StunMessage* request) { } void Connection::SendGoogPingResponse(const StunMessage* request) { + RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK(request->type() == GOOG_PING_REQUEST); // Fill in the response message. @@ -708,6 +770,7 @@ void Connection::SendGoogPingResponse(const StunMessage* request) { } void Connection::SendResponseMessage(const StunMessage& response) { + RTC_DCHECK_RUN_ON(network_thread_); // Where I send the response. const rtc::SocketAddress& addr = remote_candidate_.address(); @@ -738,11 +801,28 @@ void Connection::SendResponseMessage(const StunMessage& response) { } } +uint32_t Connection::acked_nomination() const { + RTC_DCHECK_RUN_ON(network_thread_); + return acked_nomination_; +} + +void Connection::set_remote_nomination(uint32_t remote_nomination) { + RTC_DCHECK_RUN_ON(network_thread_); + remote_nomination_ = remote_nomination; +} + void Connection::OnReadyToSend() { + RTC_DCHECK_RUN_ON(network_thread_); SignalReadyToSend(this); } +bool Connection::pruned() const { + RTC_DCHECK_RUN_ON(network_thread_); + return pruned_; +} + void Connection::Prune() { + RTC_DCHECK_RUN_ON(network_thread_); if (!pruned_ || active()) { RTC_LOG(LS_INFO) << ToString() << ": Connection pruned"; pruned_ = true; @@ -752,6 +832,7 @@ void Connection::Prune() { } void Connection::Destroy() { + RTC_DCHECK_RUN_ON(network_thread_); // TODO(deadbeef, nisse): This may leak if an application closes a // PeerConnection and then quickly destroys the PeerConnectionFactory (along // with the networking thread on which this message is posted). Also affects @@ -764,16 +845,19 @@ void Connection::Destroy() { } void Connection::FailAndDestroy() { + RTC_DCHECK_RUN_ON(network_thread_); set_state(IceCandidatePairState::FAILED); Destroy(); } void Connection::FailAndPrune() { + RTC_DCHECK_RUN_ON(network_thread_); set_state(IceCandidatePairState::FAILED); Prune(); } void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) { + RTC_DCHECK_RUN_ON(network_thread_); rtc::StringBuilder oss; if (pings_since_last_response_.size() > max) { for (size_t i = 0; i < max; i++) { @@ -789,7 +873,28 @@ void Connection::PrintPingsSinceLastResponse(std::string* s, size_t max) { *s = oss.str(); } +bool Connection::reported() const { + RTC_DCHECK_RUN_ON(network_thread_); + return reported_; +} + +void Connection::set_reported(bool reported) { + RTC_DCHECK_RUN_ON(network_thread_); + reported_ = reported; +} + +bool Connection::selected() const { + RTC_DCHECK_RUN_ON(network_thread_); + return selected_; +} + +void Connection::set_selected(bool selected) { + RTC_DCHECK_RUN_ON(network_thread_); + selected_ = selected; +} + void Connection::UpdateState(int64_t now) { + RTC_DCHECK_RUN_ON(network_thread_); int rtt = ConservativeRTTEstimate(rtt_); if (RTC_LOG_CHECK_LEVEL(LS_VERBOSE)) { @@ -846,7 +951,13 @@ void Connection::UpdateState(int64_t now) { } } +int64_t Connection::last_ping_sent() const { + RTC_DCHECK_RUN_ON(network_thread_); + return last_ping_sent_; +} + void Connection::Ping(int64_t now) { + RTC_DCHECK_RUN_ON(network_thread_); last_ping_sent_ = now; ConnectionRequest* req = new ConnectionRequest(this); // If not using renomination, we use "1" to mean "nominated" and "0" to mean @@ -865,13 +976,38 @@ void Connection::Ping(int64_t now) { num_pings_sent_++; } +int64_t Connection::last_ping_response_received() const { + RTC_DCHECK_RUN_ON(network_thread_); + return last_ping_response_received_; +} + +const absl::optional& Connection::last_ping_id_received() const { + RTC_DCHECK_RUN_ON(network_thread_); + return last_ping_id_received_; +} + +// Used to check if any STUN ping response has been received. +int Connection::rtt_samples() const { + RTC_DCHECK_RUN_ON(network_thread_); + return rtt_samples_; +} + +// Called whenever a valid ping is received on this connection. This is +// public because the connection intercepts the first ping for us. +int64_t Connection::last_ping_received() const { + RTC_DCHECK_RUN_ON(network_thread_); + return last_ping_received_; +} + void Connection::ReceivedPing(const absl::optional& request_id) { + RTC_DCHECK_RUN_ON(network_thread_); last_ping_received_ = rtc::TimeMillis(); last_ping_id_received_ = request_id; UpdateReceiving(last_ping_received_); } void Connection::HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg) { + RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK(msg->type() == STUN_BINDING_REQUEST || msg->type() == GOOG_PING_REQUEST); const StunByteStringAttribute* last_ice_check_received_attr = @@ -892,10 +1028,21 @@ void Connection::HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg) { } } +int64_t Connection::last_send_data() const { + RTC_DCHECK_RUN_ON(network_thread_); + return last_send_data_; +} + +int64_t Connection::last_data_received() const { + RTC_DCHECK_RUN_ON(network_thread_); + return last_data_received_; +} + void Connection::ReceivedPingResponse( int rtt, const std::string& request_id, const absl::optional& nomination) { + RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK_GE(rtt, 0); // We've already validated that this is a STUN binding response with // the correct local and remote username for this connection. @@ -924,7 +1071,39 @@ void Connection::ReceivedPingResponse( rtt_samples_++; } +Connection::WriteState Connection::write_state() const { + RTC_DCHECK_RUN_ON(network_thread_); + return write_state_; +} + +bool Connection::writable() const { + RTC_DCHECK_RUN_ON(network_thread_); + return write_state_ == STATE_WRITABLE; +} + +bool Connection::receiving() const { + RTC_DCHECK_RUN_ON(network_thread_); + return receiving_; +} + +// Determines whether the connection has finished connecting. This can only +// be false for TCP connections. +bool Connection::connected() const { + RTC_DCHECK_RUN_ON(network_thread_); + return connected_; +} + +bool Connection::weak() const { + return !(writable() && receiving() && connected()); +} + +bool Connection::active() const { + RTC_DCHECK_RUN_ON(network_thread_); + return write_state_ != STATE_WRITE_TIMEOUT; +} + bool Connection::dead(int64_t now) const { + RTC_DCHECK_RUN_ON(network_thread_); if (last_received() > 0) { // If it has ever received anything, we keep it alive // - if it has recevied last DEAD_CONNECTION_RECEIVE_TIMEOUT (30s) @@ -968,6 +1147,11 @@ bool Connection::dead(int64_t now) const { return now > (time_created_ms_ + MIN_CONNECTION_LIFETIME); } +int Connection::rtt() const { + RTC_DCHECK_RUN_ON(network_thread_); + return rtt_; +} + bool Connection::stable(int64_t now) const { // A connection is stable if it's RTT has converged and it isn't missing any // responses. We should send pings at a higher rate until the RTT converges @@ -986,6 +1170,7 @@ uint32_t Connection::ComputeNetworkCost() const { } std::string Connection::ToString() const { + RTC_DCHECK_RUN_ON(network_thread_); const absl::string_view CONNECT_STATE_ABBREV[2] = { "-", // not connected (false) "C", // connected (true) @@ -1022,8 +1207,8 @@ std::string Connection::ToString() const { << ":" << remote.address().ToSensitiveString() << "|" << CONNECT_STATE_ABBREV[connected()] << RECEIVE_STATE_ABBREV[receiving()] << WRITE_STATE_ABBREV[write_state()] << ICESTATE[static_cast(state())] - << "|" << SELECTED_STATE_ABBREV[selected()] << "|" << remote_nomination() - << "|" << nomination() << "|" << priority() << "|"; + << "|" << SELECTED_STATE_ABBREV[selected_] << "|" << remote_nomination() + << "|" << nomination_ << "|" << priority() << "|"; if (rtt_ < DEFAULT_RTT) { ss << rtt_ << "]"; } else { @@ -1037,6 +1222,7 @@ std::string Connection::ToSensitiveString() const { } const webrtc::IceCandidatePairDescription& Connection::ToLogDescription() { + RTC_DCHECK_RUN_ON(network_thread_); if (log_description_.has_value()) { return log_description_.value(); } @@ -1060,6 +1246,12 @@ const webrtc::IceCandidatePairDescription& Connection::ToLogDescription() { return log_description_.value(); } +void Connection::set_ice_event_log(webrtc::IceEventLog* ice_event_log) { + RTC_DCHECK_RUN_ON(network_thread_); + ice_event_log_ = ice_event_log; +} + +// RTC_RUN_ON(network_thread_) void Connection::LogCandidatePairConfig( webrtc::IceCandidatePairConfigType type) { if (ice_event_log_ == nullptr) { @@ -1068,6 +1260,7 @@ void Connection::LogCandidatePairConfig( ice_event_log_->LogCandidatePairConfig(type, id(), ToLogDescription()); } +// RTC_RUN_ON(network_thread_) void Connection::LogCandidatePairEvent(webrtc::IceCandidatePairEventType type, uint32_t transaction_id) { if (ice_event_log_ == nullptr) { @@ -1078,6 +1271,7 @@ void Connection::LogCandidatePairEvent(webrtc::IceCandidatePairEventType type, void Connection::OnConnectionRequestResponse(ConnectionRequest* request, StunMessage* response) { + RTC_DCHECK_RUN_ON(network_thread_); // Log at LS_INFO if we receive a ping response on an unwritable // connection. rtc::LoggingSeverity sev = !writable() ? rtc::LS_INFO : rtc::LS_VERBOSE; @@ -1167,6 +1361,7 @@ void Connection::OnConnectionRequestTimeout(ConnectionRequest* request) { << request->Elapsed() << " ms"; } +// RTC_RUN_ON(network_thread_). void Connection::OnConnectionRequestSent(ConnectionRequest* request) { // Log at LS_INFO if we send a ping on an unwritable connection. rtc::LoggingSeverity sev = !writable() ? rtc::LS_INFO : rtc::LS_VERBOSE; @@ -1174,7 +1369,7 @@ void Connection::OnConnectionRequestSent(ConnectionRequest* request) { << StunMethodToString(request->msg()->type()) << ", id=" << rtc::hex_encode(request->id()) << ", use_candidate=" << use_candidate_attr() - << ", nomination=" << nomination(); + << ", nomination=" << nomination_; stats_.sent_ping_requests_total++; LogCandidatePairEvent(webrtc::IceCandidatePairEventType::kCheckSent, request->reduced_transaction_id()); @@ -1187,6 +1382,16 @@ void Connection::HandleRoleConflictFromPeer() { port_->SignalRoleConflict(port_); } +IceCandidatePairState Connection::state() const { + RTC_DCHECK_RUN_ON(network_thread_); + return state_; +} + +int Connection::num_pings_sent() const { + RTC_DCHECK_RUN_ON(network_thread_); + return num_pings_sent_; +} + void Connection::MaybeSetRemoteIceParametersAndGeneration( const IceParameters& ice_params, int generation) { @@ -1218,6 +1423,7 @@ void Connection::MaybeUpdatePeerReflexiveCandidate( } void Connection::OnMessage(rtc::Message* pmsg) { + RTC_DCHECK_RUN_ON(network_thread_); RTC_DCHECK(pmsg->message_id == MSG_DELETE); RTC_LOG(LS_INFO) << "Connection deleted with number of pings sent: " << num_pings_sent_; @@ -1226,11 +1432,18 @@ void Connection::OnMessage(rtc::Message* pmsg) { } int64_t Connection::last_received() const { + RTC_DCHECK_RUN_ON(network_thread_); return std::max(last_data_received_, std::max(last_ping_received_, last_ping_response_received_)); } +int64_t Connection::receiving_unchanged_since() const { + RTC_DCHECK_RUN_ON(network_thread_); + return receiving_unchanged_since_; +} + ConnectionInfo Connection::stats() { + RTC_DCHECK_RUN_ON(network_thread_); stats_.recv_bytes_second = round(recv_rate_tracker_.ComputeRate()); stats_.recv_total_bytes = recv_rate_tracker_.TotalSampleCount(); stats_.sent_bytes_second = round(send_rate_tracker_.ComputeRate()); @@ -1317,10 +1530,12 @@ void Connection::MaybeUpdateLocalCandidate(ConnectionRequest* request, } bool Connection::rtt_converged() const { + RTC_DCHECK_RUN_ON(network_thread_); return rtt_samples_ > (RTT_RATIO + 1); } bool Connection::missing_responses(int64_t now) const { + RTC_DCHECK_RUN_ON(network_thread_); if (pings_since_last_response_.empty()) { return false; } @@ -1331,6 +1546,7 @@ bool Connection::missing_responses(int64_t now) const { bool Connection::TooManyOutstandingPings( const absl::optional& max_outstanding_pings) const { + RTC_DCHECK_RUN_ON(network_thread_); if (!max_outstanding_pings.has_value()) { return false; } @@ -1341,6 +1557,7 @@ bool Connection::TooManyOutstandingPings( return true; } +// RTC_RUN_ON(network_thread_). bool Connection::ShouldSendGoogPing(const StunMessage* message) { if (remote_support_goog_ping_ == true && cached_stun_binding_ && cached_stun_binding_->EqualAttributes(message, [](int type) { @@ -1358,6 +1575,7 @@ bool Connection::ShouldSendGoogPing(const StunMessage* message) { } void Connection::ForgetLearnedState() { + RTC_DCHECK_RUN_ON(network_thread_); RTC_LOG(LS_INFO) << ToString() << ": Connection forget learned state"; requests_.Clear(); receiving_ = false; diff --git a/p2p/base/connection.h b/p2p/base/connection.h index 9fb1e79895..c102194498 100644 --- a/p2p/base/connection.h +++ b/p2p/base/connection.h @@ -111,34 +111,28 @@ class Connection : public CandidatePairInterface, STATE_WRITE_TIMEOUT = 3, // we have had a large number of ping failures }; - WriteState write_state() const { return write_state_; } - bool writable() const { return write_state_ == STATE_WRITABLE; } - bool receiving() const { return receiving_; } + WriteState write_state() const; + bool writable() const; + bool receiving() const; // Determines whether the connection has finished connecting. This can only // be false for TCP connections. - bool connected() const { return connected_; } - bool weak() const { return !(writable() && receiving() && connected()); } - bool active() const { return write_state_ != STATE_WRITE_TIMEOUT; } + bool connected() const; + bool weak() const; + bool active() const; // A connection is dead if it can be safely deleted. bool dead(int64_t now) const; // Estimate of the round-trip time over this connection. - int rtt() const { return rtt_; } + int rtt() const; int unwritable_timeout() const; - void set_unwritable_timeout(const absl::optional& value_ms) { - unwritable_timeout_ = value_ms; - } + void set_unwritable_timeout(const absl::optional& value_ms); int unwritable_min_checks() const; - void set_unwritable_min_checks(const absl::optional& value) { - unwritable_min_checks_ = value; - } + void set_unwritable_min_checks(const absl::optional& value); int inactive_timeout() const; - void set_inactive_timeout(const absl::optional& value) { - inactive_timeout_ = value; - } + void set_inactive_timeout(const absl::optional& value); // Gets the `ConnectionInfo` stats, where `best_connection` has not been // populated (default value false). @@ -174,15 +168,15 @@ class Connection : public CandidatePairInterface, // still keep it around in case the other side wants to use it. But we can // safely stop pinging on it and we can allow it to time out if the other // side stops using it as well. - bool pruned() const { return pruned_; } + bool pruned() const; void Prune(); - bool use_candidate_attr() const { return use_candidate_attr_; } + bool use_candidate_attr() const; void set_use_candidate_attr(bool enable); - void set_nomination(uint32_t value) { nomination_ = value; } + void set_nomination(uint32_t value); - uint32_t remote_nomination() const { return remote_nomination_; } + uint32_t remote_nomination() const; // One or several pairs may be nominated based on if Regular or Aggressive // Nomination is used. https://tools.ietf.org/html/rfc5245#section-8 // `nominated` is defined both for the controlling or controlled agent based @@ -190,12 +184,10 @@ class Connection : public CandidatePairInterface, // gets its `remote_nomination_` set when pinged by the controlling agent with // a nomination value. The controlling agent gets its `acked_nomination_` set // when receiving a response to a nominating ping. - bool nominated() const { return acked_nomination_ || remote_nomination_; } + bool nominated() const; int receiving_timeout() const; - void set_receiving_timeout(absl::optional receiving_timeout_ms) { - receiving_timeout_ = receiving_timeout_ms; - } + void set_receiving_timeout(absl::optional receiving_timeout_ms); // Makes the connection go away. void Destroy(); @@ -212,24 +204,23 @@ class Connection : public CandidatePairInterface, void UpdateState(int64_t now); // Called when this connection should try checking writability again. - int64_t last_ping_sent() const { return last_ping_sent_; } + int64_t last_ping_sent() const; void Ping(int64_t now); void ReceivedPingResponse( int rtt, const std::string& request_id, const absl::optional& nomination = absl::nullopt); - int64_t last_ping_response_received() const { - return last_ping_response_received_; - } - const absl::optional& last_ping_id_received() const { - return last_ping_id_received_; - } + + int64_t last_ping_response_received() const; + const absl::optional& last_ping_id_received() const; + // Used to check if any STUN ping response has been received. - int rtt_samples() const { return rtt_samples_; } + int rtt_samples() const; // Called whenever a valid ping is received on this connection. This is // public because the connection intercepts the first ping for us. - int64_t last_ping_received() const { return last_ping_received_; } + int64_t last_ping_received() const; + void ReceivedPing( const absl::optional& request_id = absl::nullopt); // Handles the binding request; sends a response if this is a valid request. @@ -239,8 +230,8 @@ class Connection : public CandidatePairInterface, // connectivity check from the peer. void HandlePiggybackCheckAcknowledgementIfAny(StunMessage* msg); // Timestamp when data was last sent (or attempted to be sent). - int64_t last_send_data() const { return last_send_data_; } - int64_t last_data_received() const { return last_data_received_; } + int64_t last_send_data() const; + int64_t last_data_received() const; // Debugging description of this connection std::string ToDebugId() const; @@ -248,19 +239,19 @@ class Connection : public CandidatePairInterface, std::string ToSensitiveString() const; // Structured description of this candidate pair. const webrtc::IceCandidatePairDescription& ToLogDescription(); - void set_ice_event_log(webrtc::IceEventLog* ice_event_log) { - ice_event_log_ = ice_event_log; - } + void set_ice_event_log(webrtc::IceEventLog* ice_event_log); + // Prints pings_since_last_response_ into a string. void PrintPingsSinceLastResponse(std::string* pings, size_t max); - bool reported() const { return reported_; } - void set_reported(bool reported) { reported_ = reported; } - // The following two methods are only used for logging in ToString above, and - // this flag is set true by P2PTransportChannel for its selected candidate - // pair. - bool selected() const { return selected_; } - void set_selected(bool selected) { selected_ = selected; } + bool reported() const; + void set_reported(bool reported); + + // `set_selected` is only used for logging in ToString above. The flag is + // set true by P2PTransportChannel for its selected candidate pair. + // TODO(tommi): Remove `selected()` once not referenced downstream. + bool selected() const; + void set_selected(bool selected); // This signal will be fired if this connection is nominated by the // controlling side. @@ -269,9 +260,9 @@ class Connection : public CandidatePairInterface, // Invoked when Connection receives STUN error response with 487 code. void HandleRoleConflictFromPeer(); - IceCandidatePairState state() const { return state_; } + IceCandidatePairState state() const; - int num_pings_sent() const { return num_pings_sent_; } + int num_pings_sent() const; uint32_t ComputeNetworkCost() const; @@ -290,9 +281,7 @@ class Connection : public CandidatePairInterface, // response in milliseconds int64_t last_received() const; // Returns the last time when the connection changed its receiving state. - int64_t receiving_unchanged_since() const { - return receiving_unchanged_since_; - } + int64_t receiving_unchanged_since() const; bool stable(int64_t now) const; @@ -327,12 +316,8 @@ class Connection : public CandidatePairInterface, const Port* PortForTest() const { return port_; } // Public for unit tests. - uint32_t acked_nomination() const { return acked_nomination_; } - - // Public for unit tests. - void set_remote_nomination(uint32_t remote_nomination) { - remote_nomination_ = remote_nomination; - } + uint32_t acked_nomination() const; + void set_remote_nomination(uint32_t remote_nomination); protected: enum { MSG_DELETE = 0, MSG_FIRST_AVAILABLE }; @@ -347,9 +332,12 @@ class Connection : public CandidatePairInterface, virtual void OnConnectionRequestResponse(ConnectionRequest* req, StunMessage* response); void OnConnectionRequestErrorResponse(ConnectionRequest* req, - StunMessage* response); - void OnConnectionRequestTimeout(ConnectionRequest* req); - void OnConnectionRequestSent(ConnectionRequest* req); + StunMessage* response) + RTC_RUN_ON(network_thread_); + void OnConnectionRequestTimeout(ConnectionRequest* req) + RTC_RUN_ON(network_thread_); + void OnConnectionRequestSent(ConnectionRequest* req) + RTC_RUN_ON(network_thread_); bool rtt_converged() const; @@ -363,8 +351,6 @@ class Connection : public CandidatePairInterface, void set_state(IceCandidatePairState state); void set_connected(bool value); - uint32_t nomination() const { return nomination_; } - void OnMessage(rtc::Message* pmsg) override; // The local port where this connection sends and receives packets. @@ -379,7 +365,7 @@ class Connection : public CandidatePairInterface, webrtc::TaskQueueBase* const network_thread_; const uint32_t id_; Port* const port_; - size_t local_candidate_index_; + size_t local_candidate_index_ RTC_GUARDED_BY(network_thread_); Candidate remote_candidate_; ConnectionInfo stats_; @@ -391,80 +377,93 @@ class Connection : public CandidatePairInterface, // Update the local candidate based on the mapped address attribute. // If the local candidate changed, fires SignalStateChange. void MaybeUpdateLocalCandidate(ConnectionRequest* request, - StunMessage* response); + StunMessage* response) + RTC_RUN_ON(network_thread_); - void LogCandidatePairConfig(webrtc::IceCandidatePairConfigType type); + void LogCandidatePairConfig(webrtc::IceCandidatePairConfigType type) + RTC_RUN_ON(network_thread_); void LogCandidatePairEvent(webrtc::IceCandidatePairEventType type, - uint32_t transaction_id); + uint32_t transaction_id) + RTC_RUN_ON(network_thread_); // Check if this IceMessage is identical // to last message ack:ed STUN_BINDING_REQUEST. - bool ShouldSendGoogPing(const StunMessage* message); + bool ShouldSendGoogPing(const StunMessage* message) + RTC_RUN_ON(network_thread_); - WriteState write_state_; - bool receiving_; - bool connected_; - bool pruned_; - bool selected_ = false; + WriteState write_state_ RTC_GUARDED_BY(network_thread_); + bool receiving_ RTC_GUARDED_BY(network_thread_); + bool connected_ RTC_GUARDED_BY(network_thread_); + bool pruned_ RTC_GUARDED_BY(network_thread_); + bool selected_ RTC_GUARDED_BY(network_thread_) = false; // By default `use_candidate_attr_` flag will be true, // as we will be using aggressive nomination. // But when peer is ice-lite, this flag "must" be initialized to false and // turn on when connection becomes "best connection". - bool use_candidate_attr_; + bool use_candidate_attr_ RTC_GUARDED_BY(network_thread_); // Used by the controlling side to indicate that this connection will be // selected for transmission if the peer supports ICE-renomination when this // value is positive. A larger-value indicates that a connection is nominated // later and should be selected by the controlled side with higher precedence. // A zero-value indicates not nominating this connection. - uint32_t nomination_ = 0; + uint32_t nomination_ RTC_GUARDED_BY(network_thread_) = 0; // The last nomination that has been acknowledged. - uint32_t acked_nomination_ = 0; + uint32_t acked_nomination_ RTC_GUARDED_BY(network_thread_) = 0; // Used by the controlled side to remember the nomination value received from // the controlling side. When the peer does not support ICE re-nomination, its // value will be 1 if the connection has been nominated. - uint32_t remote_nomination_ = 0; + uint32_t remote_nomination_ RTC_GUARDED_BY(network_thread_) = 0; - StunRequestManager requests_; - int rtt_; - int rtt_samples_ = 0; + StunRequestManager requests_ RTC_GUARDED_BY(network_thread_); + int rtt_ RTC_GUARDED_BY(network_thread_); + int rtt_samples_ RTC_GUARDED_BY(network_thread_) = 0; // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-totalroundtriptime - uint64_t total_round_trip_time_ms_ = 0; + uint64_t total_round_trip_time_ms_ RTC_GUARDED_BY(network_thread_) = 0; // https://w3c.github.io/webrtc-stats/#dom-rtcicecandidatepairstats-currentroundtriptime - absl::optional current_round_trip_time_ms_; - int64_t last_ping_sent_; // last time we sent a ping to the other side - int64_t last_ping_received_; // last time we received a ping from the other - // side - int64_t last_data_received_; - int64_t last_ping_response_received_; - int64_t receiving_unchanged_since_ = 0; - std::vector pings_since_last_response_; + absl::optional current_round_trip_time_ms_ + RTC_GUARDED_BY(network_thread_); + int64_t last_ping_sent_ RTC_GUARDED_BY( + network_thread_); // last time we sent a ping to the other side + int64_t last_ping_received_ + RTC_GUARDED_BY(network_thread_); // last time we received a ping from the + // other side + int64_t last_data_received_ RTC_GUARDED_BY(network_thread_); + int64_t last_ping_response_received_ RTC_GUARDED_BY(network_thread_); + int64_t receiving_unchanged_since_ RTC_GUARDED_BY(network_thread_) = 0; + std::vector pings_since_last_response_ + RTC_GUARDED_BY(network_thread_); // Transaction ID of the last connectivity check received. Null if having not // received a ping yet. - absl::optional last_ping_id_received_; + absl::optional last_ping_id_received_ + RTC_GUARDED_BY(network_thread_); - absl::optional unwritable_timeout_; - absl::optional unwritable_min_checks_; - absl::optional inactive_timeout_; + absl::optional unwritable_timeout_ RTC_GUARDED_BY(network_thread_); + absl::optional unwritable_min_checks_ RTC_GUARDED_BY(network_thread_); + absl::optional inactive_timeout_ RTC_GUARDED_BY(network_thread_); - bool reported_; - IceCandidatePairState state_; + bool reported_ RTC_GUARDED_BY(network_thread_); + IceCandidatePairState state_ RTC_GUARDED_BY(network_thread_); // Time duration to switch from receiving to not receiving. - absl::optional receiving_timeout_; - int64_t time_created_ms_; - int num_pings_sent_ = 0; + absl::optional receiving_timeout_ RTC_GUARDED_BY(network_thread_); + int64_t time_created_ms_ RTC_GUARDED_BY(network_thread_); + int num_pings_sent_ RTC_GUARDED_BY(network_thread_) = 0; - absl::optional log_description_; - webrtc::IceEventLog* ice_event_log_ = nullptr; + absl::optional log_description_ + RTC_GUARDED_BY(network_thread_); + webrtc::IceEventLog* ice_event_log_ RTC_GUARDED_BY(network_thread_) = nullptr; // GOOG_PING_REQUEST is sent in place of STUN_BINDING_REQUEST // if configured via field trial, the remote peer supports it (signaled // in STUN_BINDING) and if the last STUN BINDING is identical to the one // that is about to be sent. - absl::optional remote_support_goog_ping_; - std::unique_ptr cached_stun_binding_; + absl::optional remote_support_goog_ping_ + RTC_GUARDED_BY(network_thread_); + std::unique_ptr cached_stun_binding_ + RTC_GUARDED_BY(network_thread_); const IceFieldTrials* field_trials_; - rtc::EventBasedExponentialMovingAverage rtt_estimate_; + rtc::EventBasedExponentialMovingAverage rtt_estimate_ + RTC_GUARDED_BY(network_thread_); friend class Port; friend class ConnectionRequest;