Protect Session Map with a Recursive Mutex

(This is a merge of http://go/wvgerrit/72764)

Netflix has identified a calling pattern that causes this mutex to be
taken recursively. This is not guaranteed to be safe for Widevine's
old custom Lock implementation nor std::mutex. However, it is guaranteed
to be safe for std::recursive_mutex. This patch updates the mutex in use
accordingly.

In the long-term, this lock needs to be reconsidered, as already noted
by comments in the code. It would be great if the reconsidered locking
did not require a recursive-safe lock. The TODO for this has been spun
off into its own bug and the comment has been updated to point to this.

Bug: 120471929
Test: CE CDM Unit Tests
Test: Android Unit Tests
Change-Id: I34df64456de4b469b75caf25a33f0bc53a5da330

libwvdrmengine/cdm/core/include/cdm_engine.h

@@ -403,7 +403,7 @@ class CdmEngine {
   // Protect release_key_sets_ from non-thread-safe operations.
   std::mutex release_key_sets_lock_;
 
-  // TODO(rfrias): Replace with two sets of locks, one to protect
+  // TODO(b/124471172): Replace with two sets of locks, one to protect
   // the CdmSessionMap and a per-session lock to control access to
   // session usage/destruction.
   // Locks the session map |session_map_| and session usage/destruction
@@ -412,7 +412,9 @@ class CdmEngine {
   // The layer above the CDM implementation is expected to handle thread
   // synchronization to make sure other functions that access sessions do not
   // occur simultaneously with OpenSession or CloseSession.
-  std::mutex session_map_lock_;
+  // This mutex must be recursive because it is sometimes held while callbacks
+  // occur that may subsequently call back into CdmEngine.
+  std::recursive_mutex session_map_lock_;
 
   CORE_DISALLOW_COPY_AND_ASSIGN(CdmEngine);
 };

libwvdrmengine/cdm/core/src/cdm_engine.cpp

@@ -92,7 +92,7 @@ CdmEngine::CdmEngine(FileSystem* file_system,
 
 CdmEngine::~CdmEngine() {
   usage_session_.reset();
-  std::unique_lock<std::mutex> lock(session_map_lock_);
+  std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
   session_map_.Terminate();
 }
 
@@ -152,7 +152,7 @@ CdmResponseType CdmEngine::OpenSession(
   CdmSessionId id = new_session->session_id();
   LOGI("CdmEngine::OpenSession: %s", id.c_str());
 
-  std::unique_lock<std::mutex> lock(session_map_lock_);
+  std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
   session_map_.Add(id, new_session.release());
   if (session_id) *session_id = id;
   return NO_ERROR;
@@ -194,7 +194,7 @@ CdmResponseType CdmEngine::OpenKeySetSession(
 
 CdmResponseType CdmEngine::CloseSession(const CdmSessionId& session_id) {
   LOGI("CdmEngine::CloseSession: %s", session_id.c_str());
-  std::unique_lock<std::mutex> lock(session_map_lock_);
+  std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
   if (!session_map_.CloseSession(session_id)) {
     LOGE("CdmEngine::CloseSession: session not found = %s", session_id.c_str());
     return SESSION_NOT_FOUND_1;
@@ -229,7 +229,7 @@ CdmResponseType CdmEngine::CloseKeySetSession(const CdmKeySetId& key_set_id) {
 }
 
 bool CdmEngine::IsOpenSession(const CdmSessionId& session_id) {
-  std::unique_lock<std::mutex> lock(session_map_lock_);
+  std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
   return session_map_.Exists(session_id);
 }
 
@@ -1708,7 +1708,7 @@ CdmResponseType CdmEngine::Decrypt(const CdmSessionId& session_id,
     // else we must be level 1 direct and we don't need to return a buffer.
   }
 
-  std::unique_lock<std::mutex> lock(session_map_lock_);
+  std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
   std::shared_ptr<CdmSession> session;
   if (session_id.empty()) {
     CdmSessionList sessions;
@@ -1909,7 +1909,7 @@ bool CdmEngine::FindSessionForKey(const KeyId& key_id,
 
   uint32_t session_sharing_id = Properties::GetSessionSharingId(*session_id);
 
-  std::unique_lock<std::mutex> lock(session_map_lock_);
+  std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
   CdmSessionList sessions;
   session_map_.GetSessionList(sessions);
 
@@ -1966,7 +1966,7 @@ void CdmEngine::OnTimerEvent() {
   bool is_usage_update_needed = false;
 
   {
-    std::unique_lock<std::mutex> lock(session_map_lock_);
+    std::unique_lock<std::recursive_mutex> lock(session_map_lock_);
     CdmSessionList sessions;
     session_map_.GetSessionList(sessions);