diff --git a/media/sctp/sctptransport.cc b/media/sctp/sctptransport.cc index 42780f0e69..41d4b7a0c5 100644 --- a/media/sctp/sctptransport.cc +++ b/media/sctp/sctptransport.cc @@ -46,8 +46,8 @@ namespace { // take off 80 bytes for DTLS/TURN/TCP/IP overhead. static constexpr size_t kSctpMtu = 1200; -// The size of the SCTP association send buffer. 256kB, the usrsctp default. -static constexpr int kSendBufferSize = 262144; +// The size of the SCTP association send buffer. 256kB, the usrsctp default. +static constexpr int kSendBufferSize = 256 * 1024; // Set the initial value of the static SCTP Data Engines reference count. int g_usrsctp_usage_count = 0; diff --git a/pc/channelmanager.cc b/pc/channelmanager.cc index 522f19736c..23abdefb59 100644 --- a/pc/channelmanager.cc +++ b/pc/channelmanager.cc @@ -58,12 +58,6 @@ void ChannelManager::Construct(std::unique_ptr me, ChannelManager::~ChannelManager() { if (initialized_) { Terminate(); - // If srtp is initialized (done by the Channel) then we must call - // srtp_shutdown to free all crypto kernel lists. But we need to make sure - // shutdown always called at the end, after channels are destroyed. - // ChannelManager d'tor is always called last, it's safe place to call - // shutdown. - ShutdownSrtp(); } // The media engine needs to be deleted on the worker thread for thread safe // destruction, diff --git a/pc/srtpfilter.cc b/pc/srtpfilter.cc index fbea08fdc5..5f581f3826 100644 --- a/pc/srtpfilter.cc +++ b/pc/srtpfilter.cc @@ -25,13 +25,6 @@ namespace cricket { -// NOTE: This is called from ChannelManager D'tor. -void ShutdownSrtp() { - // If srtp_dealloc is not executed then this will clear all existing sessions. - // This should be called when application is shutting down. - SrtpSession::Terminate(); -} - SrtpFilter::SrtpFilter() { } diff --git a/pc/srtpfilter.h b/pc/srtpfilter.h index ffdf637ae9..556d89a6b6 100644 --- a/pc/srtpfilter.h +++ b/pc/srtpfilter.h @@ -33,8 +33,6 @@ struct srtp_ctx_t_; namespace cricket { -void ShutdownSrtp(); - // A helper class used to negotiate SDES crypto params. // TODO(zhihuang): Find a better name for this class, like "SdesNegotiator". class SrtpFilter { diff --git a/pc/srtpsession.cc b/pc/srtpsession.cc index ba49b166ae..4ba3619d9c 100644 --- a/pc/srtpsession.cc +++ b/pc/srtpsession.cc @@ -10,20 +10,16 @@ #include "pc/srtpsession.h" -#include "third_party/libsrtp/include/srtp.h" -#include "third_party/libsrtp/include/srtp_priv.h" #include "media/base/rtputils.h" #include "pc/externalhmac.h" +#include "rtc_base/criticalsection.h" #include "rtc_base/logging.h" #include "rtc_base/sslstreamadapter.h" +#include "third_party/libsrtp/include/srtp.h" +#include "third_party/libsrtp/include/srtp_priv.h" namespace cricket { -bool SrtpSession::inited_ = false; - -// This lock protects SrtpSession::inited_. -rtc::GlobalLockPod SrtpSession::lock_; - SrtpSession::SrtpSession() {} SrtpSession::~SrtpSession() { @@ -31,6 +27,9 @@ SrtpSession::~SrtpSession() { srtp_set_user_data(session_, nullptr); srtp_dealloc(session_); } + if (inited_) { + DecrementLibsrtpUsageCountAndMaybeDeinit(); + } } bool SrtpSession::SetSend(int cs, const uint8_t* key, size_t len) { @@ -300,7 +299,11 @@ bool SrtpSession::SetKey(int type, int cs, const uint8_t* key, size_t len) { return false; } - if (!Init()) { + // This is the first time we need to actually interact with libsrtp, so + // initialize it if needed. + if (IncrementLibsrtpUsageCountAndMaybeInit()) { + inited_ = true; + } else { return false; } @@ -323,10 +326,15 @@ void SrtpSession::SetEncryptedHeaderExtensionIds( encrypted_header_extension_ids_ = encrypted_header_extension_ids; } -bool SrtpSession::Init() { - rtc::GlobalLockScope ls(&lock_); +int g_libsrtp_usage_count = 0; +rtc::GlobalLockPod g_libsrtp_lock; - if (!inited_) { +// static +bool SrtpSession::IncrementLibsrtpUsageCountAndMaybeInit() { + rtc::GlobalLockScope ls(&g_libsrtp_lock); + + RTC_DCHECK_GE(g_libsrtp_usage_count, 0); + if (g_libsrtp_usage_count == 0) { int err; err = srtp_init(); if (err != srtp_err_status_ok) { @@ -345,22 +353,21 @@ bool SrtpSession::Init() { LOG(LS_ERROR) << "Failed to initialize fake auth, err=" << err; return false; } - inited_ = true; } - + ++g_libsrtp_usage_count; return true; } -void SrtpSession::Terminate() { - rtc::GlobalLockScope ls(&lock_); +// static +void SrtpSession::DecrementLibsrtpUsageCountAndMaybeDeinit() { + rtc::GlobalLockScope ls(&g_libsrtp_lock); - if (inited_) { + RTC_DCHECK_GE(g_libsrtp_usage_count, 1); + if (--g_libsrtp_usage_count == 0) { int err = srtp_shutdown(); if (err) { LOG(LS_ERROR) << "srtp_shutdown failed. err=" << err; - return; } - inited_ = false; } } diff --git a/pc/srtpsession.h b/pc/srtpsession.h index 2d4dd6f121..94702da130 100644 --- a/pc/srtpsession.h +++ b/pc/srtpsession.h @@ -74,9 +74,6 @@ class SrtpSession { // been set. bool IsExternalAuthActive() const; - // Calls srtp_shutdown if it's initialized. - static void Terminate(); - private: bool DoSetKey(int type, int cs, const uint8_t* key, size_t len); bool SetKey(int type, int cs, const uint8_t* key, size_t len); @@ -87,7 +84,14 @@ class SrtpSession { // Returns send stream current packet index from srtp db. bool GetSendStreamPacketIndex(void* data, int in_len, int64_t* index); - static bool Init(); + // These methods are responsible for initializing libsrtp (if the usage count + // is incremented from 0 to 1) or deinitializing it (when decremented from 1 + // to 0). + // + // Returns true if successful (will always be successful if already inited). + static bool IncrementLibsrtpUsageCountAndMaybeInit(); + static void DecrementLibsrtpUsageCountAndMaybeDeinit(); + void HandleEvent(const srtp_event_data_t* ev); static void HandleEventThunk(srtp_event_data_t* ev); @@ -95,7 +99,7 @@ class SrtpSession { srtp_ctx_t_* session_ = nullptr; int rtp_auth_tag_len_ = 0; int rtcp_auth_tag_len_ = 0; - static bool inited_; + bool inited_ = false; static rtc::GlobalLockPod lock_; int last_send_seq_num_ = -1; bool external_auth_active_ = false;