Keep count of libsrtp clients, and only deinitialize when it goes to 0.

This is the same thing we're doing for usrsctp. Before this CL, the
first SrtpSession to call SetKey would initialize libsrtp, and
ChannelManager's destructor would deinitialize it. This works for an
application that only uses one instance of ChannelManager simultaneously
(or one instance of PeerConnectionFactory), but not one that uses
multiple.

Now, libsrtp is effectively reference-counted, with the first
SrtpSession to take a reference initializing it, and the last to remove
its reference deinitializing it.

This issue was discovered recently due to a change that resulted in
using srtp_update. Without using that method, the issue went unnoticed;
maybe srtp_protect/srtp_unprotect don't require initialization?

Bug: webrtc:8388
Change-Id: If1329360f0b469e454810e62e9b5acfbd4cba100
Reviewed-on: https://webrtc-review.googlesource.com/9000
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#20262}

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;

pc/channelmanager.cc

@@ -58,12 +58,6 @@ void ChannelManager::Construct(std::unique_ptr<MediaEngineInterface> 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,

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() {
 }
 

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 {

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;
   }
 }
 

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;