widevine-partner/android
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Avoid race condition on closing CDM session.

Browse Source 
[ Merge from go/wvgerrit/22920 ]

Automated tests reveal a race condition between closing
a session and the periodic policy timer event. If a close
session was in progress (WVDrmPlugin::closeSession) and
CdmEngine::CloseSession caused the CdmEngine::session_list_lock_
to be held, a call into CdmEngine::OnTimerEvent would pend on the
release of the lock.

The close session would continue to deallocate the session
and disable (stop) the policy timer leaving the CdmEngine::OnTimerEvent
call in an undefined state. This would result in an ANR.

This subtle race-condition was introduced when changes were made
to add in per-origin storage [ http://go/wvgerrit/17971 ]. This seems
to happen at a low frequency (~ < 0.5%).

To address a lock has been introduced to protect the map
WvContentDecryptionModule::cdms_.

Test: Unit tests + 200 aupt test iterations

b/33343891

Change-Id: I9788db8a7d1df84f0df82cdbadb9d0f0fbe21e4e
This commit is contained in:
Rahul Frias
2017-01-06 16:40:04 -08:00
parent 5f4397fdc4
commit 182f3c8058
2 changed files with 15 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
libwvdrmengine/cdm/include/wv_content_decryption_module.h
View File

@@ -148,6 +148,8 @@ class WvContentDecryptionModule : public android::RefBase, public TimerHandler {
// instance variables // instance variables
// This manages the lifetime of the CDM instances. // This manages the lifetime of the CDM instances.
std::map<std::string, CdmInfo> cdms_; std::map<std::string, CdmInfo> cdms_;
Lock cdms_lock_;
// This contains weak pointers to the CDM instances contained in |cdms_|. // This contains weak pointers to the CDM instances contained in |cdms_|.
std::map<std::string, CdmEngine*> cdm_by_session_id_; std::map<std::string, CdmEngine*> cdm_by_session_id_;

21
libwvdrmengine/cdm/src/wv_content_decryption_module.cpp
View File

@@ -265,6 +265,7 @@ WvContentDecryptionModule::CdmInfo::CdmInfo()
CdmEngine* WvContentDecryptionModule::EnsureCdmForOrigin( CdmEngine* WvContentDecryptionModule::EnsureCdmForOrigin(
const std::string& origin) { const std::string& origin) {
AutoLock auto_lock(cdms_lock_);
if (cdms_.find(origin) == cdms_.end()) { if (cdms_.find(origin) == cdms_.end()) {
// Will create a new instance using the default constructor. // Will create a new instance using the default constructor.
cdms_[origin].file_system.SetOrigin(origin); cdms_[origin].file_system.SetOrigin(origin);
@@ -288,23 +289,27 @@ void WvContentDecryptionModule::EnablePolicyTimer() {
} }
void WvContentDecryptionModule::DisablePolicyTimer(bool force) { void WvContentDecryptionModule::DisablePolicyTimer(bool force) {
AutoLock auto_lock(policy_timer_lock_);
bool has_sessions = false; bool has_sessions = false;
for (auto it = cdms_.begin(); it != cdms_.end();) { {
if (it->second.cdm_engine->SessionSize() != 0) { AutoLock auto_lock(cdms_lock_);
has_sessions = true; for (auto it = cdms_.begin(); it != cdms_.end();) {
++it; if (it->second.cdm_engine->SessionSize() != 0) {
} else { has_sessions = true;
// The CDM is no longer used for this origin, delete it. ++it;
it = cdms_.erase(it); } else {
// The CDM is no longer used for this origin, delete it.
it = cdms_.erase(it);
}
} }
} }
AutoLock auto_lock(policy_timer_lock_);
if ((!has_sessions || force) && policy_timer_.IsRunning()) if ((!has_sessions || force) && policy_timer_.IsRunning())
policy_timer_.Stop(); policy_timer_.Stop();
} }
void WvContentDecryptionModule::OnTimerEvent() { void WvContentDecryptionModule::OnTimerEvent() {
AutoLock auto_lock(cdms_lock_);
for (auto it = cdms_.begin(); it != cdms_.end(); ++it) { for (auto it = cdms_.begin(); it != cdms_.end(); ++it) {
it->second.cdm_engine->OnTimerEvent(); it->second.cdm_engine->OnTimerEvent();
} }