From 4244b5f6b4381bf983379778ed9a7afc69a70cdc Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Thu, 15 Oct 2020 12:57:05 +0000 Subject: [PATCH] Tidying stuff in PC resources class - Declare as non-copyable and non-movable - Return const pointers from functions marked const, and double up accessors where both const and non-const are needed - Add helper in order to const sctp_factory_ - Use non-const reference args where appropriate Bug: webrtc:11967 Change-Id: I84f0d1a1b4a5c6c1eb89972345d774667acc8823 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/188584 Reviewed-by: Niels Moller Reviewed-by: Tommi Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/master@{#32415} --- pc/connection_context.cc | 53 +++++++++++++++++++++++----------------- pc/connection_context.h | 51 ++++++++++++++++++++++---------------- 2 files changed, 60 insertions(+), 44 deletions(-) diff --git a/pc/connection_context.cc b/pc/connection_context.cc index 846a53ef23..ea9fb72f53 100644 --- a/pc/connection_context.cc +++ b/pc/connection_context.cc @@ -22,23 +22,23 @@ namespace { rtc::Thread* MaybeStartThread(rtc::Thread* old_thread, const std::string& thread_name, bool with_socket_server, - std::unique_ptr* thread_holder) { + std::unique_ptr& thread_holder) { if (old_thread) { return old_thread; } if (with_socket_server) { - *thread_holder = rtc::Thread::CreateWithSocketServer(); + thread_holder = rtc::Thread::CreateWithSocketServer(); } else { - *thread_holder = rtc::Thread::Create(); + thread_holder = rtc::Thread::Create(); } - (*thread_holder)->SetName(thread_name, nullptr); - (*thread_holder)->Start(); - return (*thread_holder).get(); + thread_holder->SetName(thread_name, nullptr); + thread_holder->Start(); + return thread_holder.get(); } rtc::Thread* MaybeWrapThread(rtc::Thread* signaling_thread, - bool* wraps_current_thread) { - *wraps_current_thread = false; + bool& wraps_current_thread) { + wraps_current_thread = false; if (signaling_thread) { return signaling_thread; } @@ -47,11 +47,24 @@ rtc::Thread* MaybeWrapThread(rtc::Thread* signaling_thread, // If this thread isn't already wrapped by an rtc::Thread, create a // wrapper and own it in this class. this_thread = rtc::ThreadManager::Instance()->WrapCurrentThread(); - *wraps_current_thread = true; + wraps_current_thread = true; } return this_thread; } +std::unique_ptr MaybeCreateSctpFactory( + std::unique_ptr factory, + rtc::Thread* network_thread) { + if (factory) { + return factory; + } +#ifdef HAVE_SCTP + return std::make_unique(network_thread); +#else + return nullptr; +#endif +} + } // namespace ConnectionContext::ConnectionContext( @@ -59,34 +72,28 @@ ConnectionContext::ConnectionContext( : network_thread_(MaybeStartThread(dependencies.network_thread, "pc_network_thread", true, - &owned_network_thread_)), + owned_network_thread_)), worker_thread_(MaybeStartThread(dependencies.worker_thread, "pc_worker_thread", false, - &owned_worker_thread_)), + owned_worker_thread_)), signaling_thread_(MaybeWrapThread(dependencies.signaling_thread, - &wraps_current_thread_)), + wraps_current_thread_)), network_monitor_factory_(std::move(dependencies.network_monitor_factory)), call_factory_(std::move(dependencies.call_factory)), media_engine_(std::move(dependencies.media_engine)), - sctp_factory_(std::move(dependencies.sctp_factory)), + sctp_factory_(MaybeCreateSctpFactory(std::move(dependencies.sctp_factory), + network_thread())), trials_(dependencies.trials ? std::move(dependencies.trials) : std::make_unique()) { signaling_thread_->AllowInvokesToThread(worker_thread_); signaling_thread_->AllowInvokesToThread(network_thread_); worker_thread_->AllowInvokesToThread(network_thread_); network_thread_->DisallowAllInvokes(); - -#ifdef HAVE_SCTP - if (!sctp_factory_) { - sctp_factory_ = - std::make_unique(network_thread()); - } -#endif } ConnectionContext::~ConnectionContext() { - RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_DCHECK_RUN_ON(signaling_thread_); channel_manager_.reset(nullptr); // Make sure |worker_thread()| and |signaling_thread()| outlive @@ -100,12 +107,12 @@ ConnectionContext::~ConnectionContext() { void ConnectionContext::SetOptions( const PeerConnectionFactoryInterface::Options& options) { - RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_DCHECK_RUN_ON(signaling_thread_); options_ = options; } bool ConnectionContext::Initialize() { - RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_DCHECK_RUN_ON(signaling_thread_); rtc::InitRandom(rtc::Time32()); // If network_monitor_factory_ is non-null, it will be used to create a diff --git a/pc/connection_context.h b/pc/connection_context.h index 6dc91a1f9e..502ba5a88c 100644 --- a/pc/connection_context.h +++ b/pc/connection_context.h @@ -38,6 +38,10 @@ class RtcEventLog; // interferes with the operation of other PeerConnections. class ConnectionContext : public rtc::RefCountInterface { public: + // This class is not copyable or movable. + ConnectionContext(const ConnectionContext&) = delete; + ConnectionContext& operator=(const ConnectionContext&) = delete; + // Functions called from PeerConnectionFactory void SetOptions(const PeerConnectionFactoryInterface::Options& options); @@ -45,15 +49,18 @@ class ConnectionContext : public rtc::RefCountInterface { // Functions called from PeerConnection and friends SctpTransportFactoryInterface* sctp_transport_factory() const { - RTC_DCHECK_RUN_ON(signaling_thread()); + RTC_DCHECK_RUN_ON(signaling_thread_); return sctp_factory_.get(); } cricket::ChannelManager* channel_manager() const; - rtc::Thread* signaling_thread() const { return signaling_thread_; } - rtc::Thread* worker_thread() const { return worker_thread_; } - rtc::Thread* network_thread() const { return network_thread_; } + rtc::Thread* signaling_thread() { return signaling_thread_; } + const rtc::Thread* signaling_thread() const { return signaling_thread_; } + rtc::Thread* worker_thread() { return worker_thread_; } + const rtc::Thread* worker_thread() const { return worker_thread_; } + rtc::Thread* network_thread() { return network_thread_; } + const rtc::Thread* network_thread() const { return network_thread_; } const PeerConnectionFactoryInterface::Options& options() const { return options_; @@ -62,16 +69,16 @@ class ConnectionContext : public rtc::RefCountInterface { const WebRtcKeyValueConfig& trials() const { return *trials_.get(); } // Accessors only used from the PeerConnectionFactory class - rtc::BasicNetworkManager* default_network_manager() const { - RTC_DCHECK_RUN_ON(signaling_thread()); + rtc::BasicNetworkManager* default_network_manager() { + RTC_DCHECK_RUN_ON(signaling_thread_); return default_network_manager_.get(); } - rtc::BasicPacketSocketFactory* default_socket_factory() const { - RTC_DCHECK_RUN_ON(signaling_thread()); + rtc::BasicPacketSocketFactory* default_socket_factory() { + RTC_DCHECK_RUN_ON(signaling_thread_); return default_socket_factory_.get(); } - CallFactoryInterface* call_factory() const { - RTC_DCHECK_RUN_ON(worker_thread()); + CallFactoryInterface* call_factory() { + RTC_DCHECK_RUN_ON(worker_thread_); return call_factory_.get(); } @@ -83,34 +90,36 @@ class ConnectionContext : public rtc::RefCountInterface { virtual ~ConnectionContext(); private: + // The following three variables are used to communicate between the + // constructor and the destructor, and are never exposed externally. bool wraps_current_thread_; // Note: Since owned_network_thread_ and owned_worker_thread_ are used // in the initialization of network_thread_ and worker_thread_, they // must be declared before them, so that they are initialized first. std::unique_ptr owned_network_thread_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(signaling_thread_); std::unique_ptr owned_worker_thread_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(signaling_thread_); rtc::Thread* const network_thread_; rtc::Thread* const worker_thread_; rtc::Thread* const signaling_thread_; PeerConnectionFactoryInterface::Options options_ - RTC_GUARDED_BY(signaling_thread()); - // Accessed both on signaling thread and worker thread. + RTC_GUARDED_BY(signaling_thread_); + // channel_manager is accessed both on signaling thread and worker thread. std::unique_ptr channel_manager_; std::unique_ptr const network_monitor_factory_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(signaling_thread_); std::unique_ptr default_network_manager_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(signaling_thread_); std::unique_ptr const call_factory_ - RTC_GUARDED_BY(worker_thread()); + RTC_GUARDED_BY(worker_thread_); std::unique_ptr default_socket_factory_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(signaling_thread_); std::unique_ptr media_engine_ - RTC_GUARDED_BY(signaling_thread()); - std::unique_ptr sctp_factory_ - RTC_GUARDED_BY(signaling_thread()); + RTC_GUARDED_BY(signaling_thread_); + std::unique_ptr const sctp_factory_ + RTC_GUARDED_BY(signaling_thread_); // Accessed both on signaling thread and worker thread. std::unique_ptr const trials_; };