admin/webrtc_m130
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix misunderstanding: OnTransportChanged is called on network thread

Browse Source 
Earlier CLs assumed that the object pointed to by call_ had to be
accessed on the worker thread. While this is generally the case,
Call::MediaTransportChange is explicitly thread safe, so
PeerConnection::OnTransportChanged doesn't have to run on the worker
thread for that reason.

Which is fortunate, because it actually runs on the network thread.
The RTC_RUN_ON(worker_thread()) annotation on the method declaration
was ineffective because this method is being called via a base class
pointer; replacing it with a call to
RTC_DCHECK_RUN_ON(worker_thread()) in the function body immediately
triggered assertions in the unit tests.

Bug: webrtc:9987
Change-Id: I08cf558a74f4ca2b2eff8ef4810ebbd1287a9726
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/129442
Commit-Queue: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27287}
This commit is contained in:
Karl Wiberg 2019-03-26 13:08:37 +01:00 committed by Commit Bot
parent 89cc7d43e6
commit ac025898e1
2 changed files with 7 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
pc/peer_connection.cc
View File

@ -7063,6 +7063,7 @@ bool PeerConnection::OnTransportChanged(
RtpTransportInternal* rtp_transport,
rtc::scoped_refptr<DtlsTransport> dtls_transport,
MediaTransportInterface* media_transport) {
RTC_DCHECK_RUN_ON(network_thread());
RTC_DCHECK_RUNS_SERIALIZED(&use_media_transport_race_checker_);
bool ret = true;
auto base_channel = GetChannel(mid);
@ -7076,7 +7077,7 @@ bool PeerConnection::OnTransportChanged(
if (use_media_transport_) {
// Only pass media transport to call object if media transport is used
// for media (and not data channel).
call_->MediaTransportChange(media_transport);
call_ptr_->MediaTransportChange(media_transport);
}
return ret;

9
pc/peer_connection.h
View File

@ -1030,8 +1030,7 @@ class PeerConnection : public PeerConnectionInternal,
bool OnTransportChanged(const std::string& mid,
RtpTransportInternal* rtp_transport,
rtc::scoped_refptr<DtlsTransport> dtls_transport,
MediaTransportInterface* media_transport) override
RTC_RUN_ON(worker_thread());
MediaTransportInterface* media_transport) override;
// Returns the observer. Will crash on CHECK if the observer is removed.
PeerConnectionObserver* Observer() const RTC_RUN_ON(signaling_thread());
@ -1151,11 +1150,13 @@ class PeerConnection : public PeerConnectionInternal,
bool remote_peer_supports_msid_ RTC_GUARDED_BY(signaling_thread()) = false;
// The unique_ptr belongs to the worker thread, but the Call object manages
// its own thread safety.
std::unique_ptr<Call> call_ RTC_GUARDED_BY(worker_thread());
// Points to the same thing as `call_`. Since it's const, we may read the
// pointer (but not touch the object) from any thread.
Call* const call_ptr_ RTC_PT_GUARDED_BY(worker_thread());
// pointer from any thread.
Call* const call_ptr_;
std::unique_ptr<StatsCollector> stats_
RTC_GUARDED_BY(signaling_thread()); // A pointer is passed to senders_