Set/clear the data channel pointers on the network thread

Bug: webrtc:9987
Change-Id: I8fa1b675a54729a26ee55926c6f27bb59981d379
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/213665
Commit-Queue: Tommi <tommi@webrtc.org>
Reviewed-by: Mirko Bonadei <mbonadei@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33640}
This commit is contained in:
Tomas Gunnarsson 2021-04-07 15:14:43 +02:00 committed by Commit Bot
parent 90fab63b98
commit d69e0709c8
5 changed files with 70 additions and 37 deletions

View File

@ -505,6 +505,19 @@ void DataChannelController::set_data_channel_type(
data_channel_type_ = type;
}
cricket::RtpDataChannel* DataChannelController::rtp_data_channel() const {
// TODO(bugs.webrtc.org/9987): Only allow this accessor to be called on the
// network thread.
// RTC_DCHECK_RUN_ON(network_thread());
return rtp_data_channel_;
}
void DataChannelController::set_rtp_data_channel(
cricket::RtpDataChannel* channel) {
RTC_DCHECK_RUN_ON(network_thread());
rtp_data_channel_ = channel;
}
DataChannelTransportInterface* DataChannelController::data_channel_transport()
const {
// TODO(bugs.webrtc.org/11547): Only allow this accessor to be called on the

View File

@ -115,12 +115,8 @@ class DataChannelController : public RtpDataChannelProviderInterface,
// Accessors
cricket::DataChannelType data_channel_type() const;
void set_data_channel_type(cricket::DataChannelType type);
cricket::RtpDataChannel* rtp_data_channel() const {
return rtp_data_channel_;
}
void set_rtp_data_channel(cricket::RtpDataChannel* channel) {
rtp_data_channel_ = channel;
}
cricket::RtpDataChannel* rtp_data_channel() const;
void set_rtp_data_channel(cricket::RtpDataChannel* channel);
DataChannelTransportInterface* data_channel_transport() const;
void set_data_channel_transport(DataChannelTransportInterface* transport);
const std::map<std::string, rtc::scoped_refptr<RtpDataChannel>>*
@ -203,9 +199,9 @@ class DataChannelController : public RtpDataChannelProviderInterface,
// |rtp_data_channel_| is used if in RTP data channel mode,
// |data_channel_transport_| when using SCTP.
// TODO(bugs.webrtc.org/9987): Accessed on both signaling and network
// thread.
cricket::RtpDataChannel* rtp_data_channel_ = nullptr;
// TODO(bugs.webrtc.org/9987): Accessed on both
// signaling and some other thread.
SctpSidAllocator sid_allocator_ /* RTC_GUARDED_BY(signaling_thread()) */;
std::vector<rtc::scoped_refptr<SctpDataChannel>> sctp_data_channels_

View File

@ -522,11 +522,13 @@ PeerConnection::~PeerConnection() {
// should be destroyed there.
network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(network_thread());
TeardownDataChannelTransport_n();
transport_controller_.reset();
port_allocator_.reset();
if (network_thread_safety_)
network_thread_safety_->SetNotAlive();
});
// call_ and event_log_ must be destroyed on the worker thread.
worker_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(worker_thread());
@ -1737,6 +1739,12 @@ void PeerConnection::Close() {
rtp_manager_->Close();
network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
// Data channels will already have been unset via the DestroyAllChannels()
// call above, which triggers a call to TeardownDataChannelTransport_n().
// TODO(tommi): ^^ That's not exactly optimal since this is yet another
// blocking hop to the network thread during Close(). Further still, the
// voice/video/data channels will be cleared on the worker thread.
RTC_DCHECK(!data_channel_controller_.rtp_data_channel());
transport_controller_.reset();
port_allocator_->DiscardCandidatePool();
if (network_thread_safety_) {
@ -2410,16 +2418,30 @@ bool PeerConnection::SetupDataChannelTransport_n(const std::string& mid) {
return true;
}
void PeerConnection::TeardownDataChannelTransport_n() {
if (!sctp_mid_n_ && !data_channel_controller_.data_channel_transport()) {
void PeerConnection::SetupRtpDataChannelTransport_n(
cricket::RtpDataChannel* data_channel) {
data_channel_controller_.set_rtp_data_channel(data_channel);
if (!data_channel)
return;
}
RTC_LOG(LS_INFO) << "Tearing down data channel transport for mid="
<< *sctp_mid_n_;
// |sctp_mid_| may still be active through an SCTP transport. If not, unset
// it.
sctp_mid_n_.reset();
// TODO(bugs.webrtc.org/9987): OnSentPacket_w needs to be changed to
// OnSentPacket_n (and be called on the network thread).
data_channel->SignalSentPacket().connect(this,
&PeerConnection::OnSentPacket_w);
}
void PeerConnection::TeardownDataChannelTransport_n() {
// Clear the RTP data channel if any.
data_channel_controller_.set_rtp_data_channel(nullptr);
if (sctp_mid_n_) {
// |sctp_mid_| may still be active through an SCTP transport. If not, unset
// it.
RTC_LOG(LS_INFO) << "Tearing down data channel transport for mid="
<< *sctp_mid_n_;
sctp_mid_n_.reset();
}
data_channel_controller_.TeardownDataChannelTransport_n();
}

View File

@ -442,6 +442,8 @@ class PeerConnection : public PeerConnectionInternal,
bool SetupDataChannelTransport_n(const std::string& mid)
RTC_RUN_ON(network_thread());
void SetupRtpDataChannelTransport_n(cricket::RtpDataChannel* data_channel)
RTC_RUN_ON(network_thread());
void TeardownDataChannelTransport_n() RTC_RUN_ON(network_thread());
cricket::ChannelInterface* GetChannel(const std::string& content_name);

View File

@ -4671,19 +4671,18 @@ bool SdpOfferAnswerHandler::CreateDataChannel(const std::string& mid) {
case cricket::DCT_RTP:
default:
RtpTransportInternal* rtp_transport = pc_->GetRtpTransport(mid);
// TODO(bugs.webrtc.org/9987): set_rtp_data_channel() should be called on
// the network thread like set_data_channel_transport is.
data_channel_controller()->set_rtp_data_channel(
cricket::RtpDataChannel* data_channel =
channel_manager()->CreateRtpDataChannel(
pc_->configuration()->media_config, rtp_transport,
signaling_thread(), mid, pc_->SrtpRequired(),
pc_->GetCryptoOptions(), &ssrc_generator_));
if (!data_channel_controller()->rtp_data_channel()) {
pc_->GetCryptoOptions(), &ssrc_generator_);
if (!data_channel)
return false;
}
data_channel_controller()->rtp_data_channel()->SignalSentPacket().connect(
pc_, &PeerConnection::OnSentPacket_w);
pc_->network_thread()->Invoke<void>(RTC_FROM_HERE, [this, data_channel] {
RTC_DCHECK_RUN_ON(pc_->network_thread());
pc_->SetupRtpDataChannelTransport_n(data_channel);
});
have_pending_rtp_data_channel_ = true;
return true;
}
@ -4722,21 +4721,22 @@ void SdpOfferAnswerHandler::DestroyTransceiverChannel(
void SdpOfferAnswerHandler::DestroyDataChannelTransport() {
RTC_DCHECK_RUN_ON(signaling_thread());
if (data_channel_controller()->rtp_data_channel()) {
data_channel_controller()->OnTransportChannelClosed();
DestroyChannelInterface(data_channel_controller()->rtp_data_channel());
data_channel_controller()->set_rtp_data_channel(nullptr);
}
const bool has_sctp = pc_->sctp_mid().has_value();
auto* rtp_data_channel = data_channel_controller()->rtp_data_channel();
if (pc_->sctp_mid()) {
RTC_DCHECK_RUN_ON(pc_->signaling_thread());
if (has_sctp || rtp_data_channel)
data_channel_controller()->OnTransportChannelClosed();
pc_->network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(pc_->network_thread());
pc_->TeardownDataChannelTransport_n();
});
pc_->network_thread()->Invoke<void>(RTC_FROM_HERE, [this] {
RTC_DCHECK_RUN_ON(pc_->network_thread());
pc_->TeardownDataChannelTransport_n();
});
if (has_sctp)
pc_->ResetSctpDataMid();
}
if (rtp_data_channel)
DestroyChannelInterface(rtp_data_channel);
}
void SdpOfferAnswerHandler::DestroyChannelInterface(