Merge "Finer-Grained OEMCrypto Locking"

libwvdrmengine/cdm/core/include/crypto_session.h

@@ -17,6 +17,7 @@
 #include "key_session.h"
 #include "metrics_collections.h"
 #include "oemcrypto_adapter.h"
+#include "rw_lock.h"
 #include "timer_metric.h"
 #include "wv_cdm_types.h"
 
@@ -256,7 +257,7 @@ class CryptoSession {
   // OEMCrypto to use a test keybox.
   // Ownership of the object is transfered to CryptoSession.
   static void SetCryptoSessionFactory(CryptoSessionFactory* factory) {
-    std::unique_lock<std::mutex> auto_lock(factory_lock_);
+    std::unique_lock<std::mutex> auto_lock(factory_mutex_);
     factory_.reset(factory);
   }
 
@@ -311,25 +312,59 @@ class CryptoSession {
       size_t max_chunk_size);
   static void IncrementIV(uint64_t increase_by, std::vector<uint8_t>* iv_out);
 
-  // These methods should be used to take the various CryptoSession locks in
-  // preference to taking the locks directly.
+  // These methods should be used to take the various CryptoSession mutexes in
+  // preference to taking the mutexes directly.
   //
-  // The Static Field Lock should be taken before accessing any of
-  // CryptoSession's non-atomic static fields. The OEMCrypto Lock should be
-  // taken before calling into OEMCrypto. Note that accessing |key_session_|
-  // often accesses OEMCrypto, so this lock should be held before calling into
-  // |key_session_| as well. If a function needs to take both locks
-  // simultaneously, it must *always* take the Static Field Lock first. In
-  // general, locks should only be held for the minimum time necessary.
-  // (e.g. |oem_crypto_lock_| should only be held for the duration of a single
-  // call into OEMCrypto, unless there is a compelling argument otherwise such
-  // as making two calls into OEMCrypto immediately after each other.)
+  // A lock should be taken on the Static Field Mutex before accessing any of
+  // CryptoSession's non-atomic static fields. It can be taken as a reader or as
+  // a writer, depending on how you will be accessing the static fields.
+  //
+  // Before calling into OEMCrypto, code must take locks on the OEMCrypto Mutex
+  // and/or the OEMCrypto Session Mutex. Which of them should be taken and how
+  // depends on the OEMCrypto function being called; consult the OEMCrypto
+  // specification's threading guarantees before making any calls. The OEMCrypto
+  // specification defines several classes of functions for the purposes of
+  // parallelism. The methods below lock the OEMCrypto Mutex and OEMCrypto
+  // Session Mutex in the correct order and manner to fulfill the guarantees in
+  // the specification.
+  //
+  // For this function class...    | ...use this locking method
+  // ------------------------------+---------------------------
+  // Initialization & Termination  |         WithOecWriteLock()
+  // Property                      |          WithOecReadLock()
+  // Session Initialization        |         WithOecWriteLock()
+  // Usage Table                   |         WithOecWriteLock()
+  // Session                       |       WithOecSessionLock()
+  //
+  // Note that accessing |key_session_| often accesses the OEMCrypto session, so
+  // WithOecSessionLock() should be used before accessing |key_session_| as
+  // well.
+  //
+  // If a function needs to take a lock on both the Static Field Mutex and some
+  // of the OEMCrypto mutexes simultaneously, it must *always* lock the Static
+  // Field Mutex before the OEMCrypto mutexes.
+  //
+  // In general, all locks should only be held for the minimum time necessary
+  // (e.g. a lock on the OEMCrypto mutexes should only be held for the duration
+  // of a single call into OEMCrypto) unless there is a compelling argument
+  // otherwise, such as making two calls into OEMCrypto immediately after each
+  // other.
   template <class Func>
-  static auto WithStaticFieldLock(const char* tag, Func body)
+  static auto WithStaticFieldWriteLock(const char* tag, Func body)
       -> decltype(body());
 
   template <class Func>
-  static auto WithOecLock(const char* tag, Func body) -> decltype(body());
+  static auto WithStaticFieldReadLock(const char* tag, Func body)
+      -> decltype(body());
+
+  template <class Func>
+  static auto WithOecWriteLock(const char* tag, Func body) -> decltype(body());
+
+  template <class Func>
+  static auto WithOecReadLock(const char* tag, Func body) -> decltype(body());
+
+  template <class Func>
+  auto WithOecSessionLock(const char* tag, Func body) -> decltype(body());
 
   static bool IsInitialized();
 
@@ -337,11 +372,12 @@ class CryptoSession {
   static const size_t kAes128BlockSize = 16;  // Block size for AES_CBC_128
   static const size_t kSignatureSize = 32;    // size for HMAC-SHA256 signature
 
-  // The |WithStaticFieldLock| and |WithOecLock| methods should be used in
-  // preference to taking these locks directly. When taken, the rules of
-  // ordering documented with those functions must still be upheld.
-  static std::mutex static_field_lock_;
-  static std::mutex oem_crypto_lock_;
+  // The locking methods above should be used in preference to taking these
+  // mutexes directly. If code takes these manually and needs to take more
+  // than one, it must *always* take them in the order they are defined here.
+  static shared_mutex static_field_mutex_;
+  static shared_mutex oem_crypto_mutex_;
+  std::mutex oem_crypto_session_mutex_;
 
   static bool initialized_;
   static int session_count_;
@@ -374,9 +410,9 @@ class CryptoSession {
   uint32_t api_version_;
 
   // In order to avoid creating a deadlock if instantiation needs to take any
-  // of the CryptoSession static locks, |factory_| is protected by its own lock
-  // that is only used in the two funtions that interact with it.
-  static std::mutex factory_lock_;
+  // of the CryptoSession static mutexes, |factory_| is protected by its own
+  // mutex that is only used in the two funtions that interact with it.
+  static std::mutex factory_mutex_;
   static std::unique_ptr<CryptoSessionFactory> factory_;
 
   CORE_DISALLOW_COPY_AND_ASSIGN(CryptoSession);