Protect sessions from concurrent access.

Locks in earlier releases controlled access to sessions and the list
of sessions for each CdmEngine instance. This guarded against
concurrent access between session management (OpenSession,
CloseSession, etc), periodic timer calls and calls to Decrypt.

The list of sessions and locking was moved to a separate class
CdmSessionMap. This left open the possibility that a session
might be destructed, while being called to decrypt or invoked through the
timer. An attempt was made to add per-session locks in b/73781703
but this was found insufficient.

Per-session locks will be introduced in a future changelist, but for
now the coarser locks will be reintroduced.

Bug: 73781703
Bug: 79158083
Bug: 79262108
Bug: 79436509

Test: WV unit/integration tests, GTS GtsMediaTestCases tests and
      24 hours of continuous Netflix playback.

Change-Id: I30a3ede340192370dfe5c92c01b1c76df16b7123

libwvdrmengine/cdm/core/include/cdm_engine.h

@@ -330,6 +330,17 @@ class CdmEngine {
   // Protect release_key_sets_ from non-thread-safe operations.
   Lock release_key_sets_lock_;
 
+  // TODO(rfrias): 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
+  // between session management calls (OpenSession, CloseSession, etc),
+  // periodic timer calls (OnTimerEvent), and calls to Decrypt.
+  // 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.
+  Lock session_map_lock_;
+
   CORE_DISALLOW_COPY_AND_ASSIGN(CdmEngine);
 };
 

libwvdrmengine/cdm/core/include/cdm_session_map.h

@@ -10,7 +10,6 @@
 #include <string>
 
 #include "cdm_session.h"
-#include "lock.h"
 #include "shared_ptr.h"
 #include "wv_cdm_types.h"
 
@@ -18,11 +17,18 @@ namespace wvcdm {
 
 typedef std::list<shared_ptr<CdmSession> > CdmSessionList;
 
+// TODO(rfrias): Concurrency protection for this class has moved to CdmEngine.
+// Add it back when locks to control access to session usage and destruction
+// are introduced.
 class CdmSessionMap {
  public:
   CdmSessionMap() {}
   virtual ~CdmSessionMap();
 
+  // Use |Terminate| rather than relying on the destructor to release
+  // resources, as it can be protected by locks.
+  void Terminate();
+
   void Add(const std::string& id, CdmSession* session);
 
   bool CloseSession(const std::string& id);
@@ -43,7 +49,6 @@ class CdmSessionMap {
   bool FindSessionNoLock(const CdmSessionId& session_id,
                          shared_ptr<CdmSession>* session);
 
-  Lock lock_;
   CdmIdToSessionMap sessions_;
 
   CORE_DISALLOW_COPY_AND_ASSIGN(CdmSessionMap);

libwvdrmengine/cdm/core/src/cdm_engine.cpp

@@ -85,7 +85,10 @@ CdmEngine::CdmEngine(FileSystem* file_system, const std::string& spoid)
   }
 }
 
-CdmEngine::~CdmEngine() {}
+CdmEngine::~CdmEngine() {
+  AutoLock lock(session_map_lock_);
+  session_map_.Terminate();
+}
 
 CdmResponseType CdmEngine::OpenSession(
     const CdmKeySystem& key_system, CdmClientPropertySet* property_set,
@@ -143,6 +146,7 @@ CdmResponseType CdmEngine::OpenSession(
   CdmSessionId id = new_session->session_id();
   LOGI("CdmEngine::OpenSession: %s", id.c_str());
 
+  AutoLock lock(session_map_lock_);
   session_map_.Add(id, new_session.release());
   if (session_id) *session_id = id;
   return NO_ERROR;
@@ -184,6 +188,7 @@ CdmResponseType CdmEngine::OpenKeySetSession(
 
 CdmResponseType CdmEngine::CloseSession(const CdmSessionId& session_id) {
   LOGI("CdmEngine::CloseSession: %s", session_id.c_str());
+  AutoLock 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;
@@ -217,6 +222,7 @@ CdmResponseType CdmEngine::CloseKeySetSession(const CdmKeySetId& key_set_id) {
 }
 
 bool CdmEngine::IsOpenSession(const CdmSessionId& session_id) {
+  AutoLock lock(session_map_lock_);
   return session_map_.Exists(session_id);
 }
 
@@ -1472,6 +1478,7 @@ CdmResponseType CdmEngine::Decrypt(const CdmSessionId& session_id,
     // else we must be level 1 direct and we don't need to return a buffer.
   }
 
+  AutoLock lock(session_map_lock_);
   shared_ptr<CdmSession> session;
   if (session_id.empty()) {
     CdmSessionList sessions;
@@ -1592,6 +1599,7 @@ bool CdmEngine::FindSessionForKey(const KeyId& key_id,
 
   uint32_t session_sharing_id = Properties::GetSessionSharingId(*session_id);
 
+  AutoLock lock(session_map_lock_);
   CdmSessionList sessions;
   session_map_.GetSessionList(sessions);
 
@@ -1637,9 +1645,6 @@ void CdmEngine::OnTimerEvent() {
   Clock clock;
   uint64_t current_time = clock.GetCurrentTime();
 
-  CdmSessionList sessions;
-  session_map_.GetSessionList(sessions);
-
   bool usage_update_period_expired = false;
   if (current_time - last_usage_information_update_time_ >
       kUpdateUsageInformationPeriod) {
@@ -1650,6 +1655,10 @@ void CdmEngine::OnTimerEvent() {
   bool is_initial_usage_update = false;
   bool is_usage_update_needed = false;
 
+  AutoLock lock(session_map_lock_);
+  CdmSessionList sessions;
+  session_map_.GetSessionList(sessions);
+
   while (!sessions.empty()) {
     is_initial_usage_update =
         is_initial_usage_update || sessions.front()->is_initial_usage_update();

libwvdrmengine/cdm/core/src/cdm_session_map.cpp

@@ -12,7 +12,10 @@
 namespace wvcdm {
 
 CdmSessionMap::~CdmSessionMap() {
-  AutoLock lock(lock_);
+  Terminate();
+}
+
+void CdmSessionMap::Terminate() {
   for (CdmIdToSessionMap::iterator i = sessions_.begin();
        i != sessions_.end(); ++i) {
     i->second->Close();
@@ -22,12 +25,10 @@ CdmSessionMap::~CdmSessionMap() {
 }
 
 void CdmSessionMap::Add(const std::string& id, CdmSession* session) {
-  AutoLock lock(lock_);
   sessions_[id].reset(session);
 }
 
 bool CdmSessionMap::CloseSession(const std::string& id) {
-  AutoLock lock(lock_);
   shared_ptr<CdmSession> session;
   if (!FindSessionNoLock(id, &session)) {
     return false;
@@ -38,13 +39,11 @@ bool CdmSessionMap::CloseSession(const std::string& id) {
 }
 
 bool CdmSessionMap::Exists(const std::string& id) {
-  AutoLock lock(lock_);
   return sessions_.find(id) != sessions_.end();
 }
 
 bool CdmSessionMap::FindSession(const CdmSessionId& id,
                                 shared_ptr<CdmSession>* session) {
-  AutoLock lock(lock_);
   return FindSessionNoLock(id, session);
 }
 
@@ -61,7 +60,6 @@ bool CdmSessionMap::FindSessionNoLock(const CdmSessionId& session_id,
 
 void CdmSessionMap::GetSessionList(CdmSessionList& sessions) {
   sessions.clear();
-  AutoLock lock(lock_);
   for (CdmIdToSessionMap::iterator iter = sessions_.begin();
        iter != sessions_.end(); ++iter) {
     if (!(iter->second)->IsClosed()) {