Lock session list in CdmEngine OnTimerEvent (master)

Cherry pick of
https://widevine-internal-review.googlesource.com/#/c/12935/

Change-Id: I029d36b2b6d092ae938fca2a7f6d893814c25a8a
This commit is contained in:
Fred Gylys-Colwell
2015-03-06 13:29:21 -08:00
parent 2c809b62ca
commit 6444332cd7
3 changed files with 26 additions and 41 deletions

View File

@@ -138,9 +138,16 @@ class CdmEngine {
virtual bool IsKeyLoaded(const KeyId& key_id);
virtual bool FindSessionForKey(const KeyId& key_id, CdmSessionId* sessionId);
// Event listener related methods
// Event listener related methods.
// We assume that the WvCdmEventListener is asynchronous -- i.e. an event
// should be dispatched to another thread which actually does the work. In
// particular, if a syncrhonous listener calls OpenSession or CloseSession,
// the thread will dead lock.
// Returns false if listener already attached.
virtual bool AttachEventListener(const CdmSessionId& session_id,
WvCdmEventListener* listener);
// Returns true if listener was detached.
virtual bool DetachEventListener(const CdmSessionId& session_id,
WvCdmEventListener* listener);
@@ -148,7 +155,8 @@ class CdmEngine {
virtual void NotifyResolution(const CdmSessionId& session_id, uint32_t width,
uint32_t height);
// Timer expiration method
// Timer expiration method. This method is not re-entrant -- there can be
// only one timer.
virtual void OnTimerEvent();
private:
@@ -175,11 +183,13 @@ class CdmEngine {
scoped_ptr<UsagePropertySet> usage_property_set_;
int64_t last_usage_information_update_time_;
// Locks the list of sessions, |sessions_|, for the event timer. It will be
// locked in OpenSession, CloseSession. It is also locked in OnTimerEvent and
// OnKeyReleaseEvent while the list of event listeners is being generated.
// 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_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);
};

View File

@@ -74,6 +74,7 @@ class CdmSession {
virtual bool IsKeyLoaded(const KeyId& key_id);
// See comments for CdmEngine::AttachEventListener/DetachEventListener.
virtual bool AttachEventListener(WvCdmEventListener* listener);
virtual bool DetachEventListener(WvCdmEventListener* listener);

View File

@@ -140,15 +140,9 @@ CdmResponseType CdmEngine::CloseSession(const CdmSessionId& session_id) {
LOGE("CdmEngine::CloseSession: session not found = %s", session_id.c_str());
return KEY_ERROR;
}
CdmSession* session = iter->second;
sessions_.erase(session_id);
// 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;
}
@@ -894,38 +888,27 @@ 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;
AutoLock lock(session_list_lock_);
for (CdmSessionMap::iterator iter = sessions_.begin();
iter != sessions_.end() && index < session_count; ++iter) {
iter != sessions_.end(); ++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();
for (size_t i=0; i < session_count; i++) {
event_sessions[i]->OnTimerEvent(usage_update_period_expired);
iter->second->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 (size_t i=0; i < session_count; i++) {
event_sessions[i]->reset_usage_flags();
for (CdmSessionMap::iterator iter = sessions_.begin();
iter != sessions_.end(); ++iter) {
iter->second->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 = event_sessions[i]->UpdateUsageInformation();
CdmResponseType status = iter->second->UpdateUsageInformation();
if (NO_ERROR != status) {
LOGW("Update usage information failed: %d", status);
} else {
@@ -934,19 +917,10 @@ 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) {
AutoLock lock(session_list_lock_);
for (CdmSessionMap::iterator iter = sessions_.begin();
iter != sessions_.end(); ++iter) {
iter->second->OnKeyReleaseEvent(key_set_id);