am 74167954: am 802fe3b3: Lock session list in CdmEngine OnTimerEvent

* commit '7416795401c6c06b69fcfdf11fae6c195941d383':
  Lock session list in CdmEngine OnTimerEvent

libwvdrmengine/cdm/core/include/cdm_engine.h

@@ -9,6 +9,7 @@
 #include "certificate_provisioning.h"
 #include "crypto_session.h"
 #include "initialization_data.h"
+#include "lock.h"
 #include "oemcrypto_adapter.h"
 #include "scoped_ptr.h"
 #include "wv_cdm_types.h"
@@ -154,6 +155,12 @@ class CdmEngine {
   scoped_ptr<UsagePropertySet> usage_property_set_;
   int64_t last_usage_information_update_time_;
 
+  Lock session_list_lock_;
+  // If the timer is currently active -- do not delete any sessions.
+  bool timer_event_active_;
+  // Sessions to be deleted after the timer has finished.
+  std::vector<CdmSession*> dead_sessions_;
+
   CORE_DISALLOW_COPY_AND_ASSIGN(CdmEngine);
 };
 

libwvdrmengine/cdm/core/src/cdm_engine.cpp

@@ -58,7 +58,8 @@ CdmEngine::CdmEngine()
     : cert_provisioning_(NULL),
       cert_provisioning_requested_security_level_(kLevelDefault),
       usage_session_(NULL),
-      last_usage_information_update_time_(0) {
+      last_usage_information_update_time_(0),
+      timer_event_active_(false) {
   Properties::Init();
   if (!seeded_) {
     Clock clock;
@@ -68,6 +69,7 @@ CdmEngine::CdmEngine()
 }
 
 CdmEngine::~CdmEngine() {
+  AutoLock lock(session_list_lock_);
   CdmSessionMap::iterator i(sessions_.begin());
   for (; i != sessions_.end(); ++i) {
     delete i->second;
@@ -108,6 +110,7 @@ CdmResponseType CdmEngine::OpenSession(
     return sts;
   }
   *session_id = new_session->session_id();
+  AutoLock lock(session_list_lock_);
   sessions_[*session_id] = new_session.release();
   return NO_ERROR;
 }
@@ -134,7 +137,7 @@ CdmResponseType CdmEngine::OpenKeySetSession(
 
 CdmResponseType CdmEngine::CloseSession(const CdmSessionId& session_id) {
   LOGI("CdmEngine::CloseSession");
-
+  AutoLock lock(session_list_lock_);
   CdmSessionMap::iterator iter = sessions_.find(session_id);
   if (iter == sessions_.end()) {
     LOGE("CdmEngine::CloseSession: session not found = %s", session_id.c_str());
@@ -143,7 +146,9 @@ CdmResponseType CdmEngine::CloseSession(const CdmSessionId& session_id) {
 
   CdmSession* session = iter->second;
   sessions_.erase(session_id);
-  delete session;
+  // If the event timer is active, it will delete this session when it's done.
+  if (timer_event_active_) dead_sessions_.push_back(session);
+  else delete session;
   return NO_ERROR;
 }
 
@@ -905,26 +910,38 @@ void CdmEngine::OnTimerEvent() {
   bool is_initial_usage_update = false;
   bool is_usage_update_needed = false;
 
+  // Make a copy of all currently open sessions.  While this event handler is
+  // active, after we release session_list_lock_, CloseSession and OpenSession
+  // might modify sessions_, but they will not delete sessions.
+  session_list_lock_.Acquire();
+  const size_t session_count = sessions_.size();
+  CdmSession* event_sessions[session_count];
+  timer_event_active_ = true;
+  size_t index = 0;
   for (CdmSessionMap::iterator iter = sessions_.begin();
-      iter != sessions_.end(); ++iter) {
+       iter != sessions_.end() && index < session_count; ++iter) {
     is_initial_usage_update = is_initial_usage_update ||
         iter->second->is_initial_usage_update();
     is_usage_update_needed = is_usage_update_needed ||
         iter->second->is_usage_update_needed();
+    event_sessions[index++] = iter->second;
+  }
+  session_list_lock_.Release();
 
-    iter->second->OnTimerEvent(usage_update_period_expired);
+
+  for (size_t i=0; i < session_count; i++) {
+    event_sessions[i]->OnTimerEvent(usage_update_period_expired);
   }
 
   if (is_usage_update_needed &&
       (usage_update_period_expired || is_initial_usage_update)) {
     bool has_usage_been_updated = false;
-    for (CdmSessionMap::iterator iter = sessions_.begin();
-        iter != sessions_.end(); ++iter) {
-      iter->second->reset_usage_flags();
+    for (size_t i=0; i < session_count; i++) {
+      event_sessions[i]->reset_usage_flags();
       if (!has_usage_been_updated) {
         // usage is updated for all sessions so this needs to be
         // called only once per update usage information period
-        CdmResponseType status = iter->second->UpdateUsageInformation();
+        CdmResponseType status = event_sessions[i]->UpdateUsageInformation();
         if (NO_ERROR != status) {
           LOGW("Update usage information failed: %d", status);
         } else {
@@ -933,6 +950,16 @@ void CdmEngine::OnTimerEvent() {
       }
     }
   }
+  {
+    // We are done with the event handler.  Now we look for any sessions that
+    // were closed.
+    AutoLock lock(session_list_lock_);
+    for (size_t i=0; i < dead_sessions_.size(); i++) {
+      delete dead_sessions_[i];
+    }
+    dead_sessions_.clear();
+    timer_event_active_ = false;
+  }
 }
 
 void CdmEngine::OnKeyReleaseEvent(const CdmKeySetId& key_set_id) {