diff --git a/libwvdrmengine/cdm/core/include/cdm_engine.h b/libwvdrmengine/cdm/core/include/cdm_engine.h index d8898635..d713e08b 100644 --- a/libwvdrmengine/cdm/core/include/cdm_engine.h +++ b/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); }; diff --git a/libwvdrmengine/cdm/core/include/cdm_session_map.h b/libwvdrmengine/cdm/core/include/cdm_session_map.h index 0e9fc779..7fa06b81 100644 --- a/libwvdrmengine/cdm/core/include/cdm_session_map.h +++ b/libwvdrmengine/cdm/core/include/cdm_session_map.h @@ -10,7 +10,6 @@ #include #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 > 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* session); - Lock lock_; CdmIdToSessionMap sessions_; CORE_DISALLOW_COPY_AND_ASSIGN(CdmSessionMap); diff --git a/libwvdrmengine/cdm/core/src/cdm_engine.cpp b/libwvdrmengine/cdm/core/src/cdm_engine.cpp index 81d8dffc..c7bc47d4 100644 --- a/libwvdrmengine/cdm/core/src/cdm_engine.cpp +++ b/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 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(); diff --git a/libwvdrmengine/cdm/core/src/cdm_session_map.cpp b/libwvdrmengine/cdm/core/src/cdm_session_map.cpp index 8f8b2bf0..e94a3173 100644 --- a/libwvdrmengine/cdm/core/src/cdm_session_map.cpp +++ b/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 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* 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()) {