From d374b17b7cf66e3300430650d285191caa02828e Mon Sep 17 00:00:00 2001 From: Rahul Frias Date: Tue, 23 Oct 2018 00:12:45 -0700 Subject: [PATCH] Fix invalid iterator in CloseCdm [ Merge of http://go/ag/5334065 and http://go/wvgerrit/65122 ] Sessions were not being correctly released when CloseCdm() was called. Broadcom noticed this issue and proposed the fix. Bug: 117876077 Test: WV unit/integration tests, GtsMediaTestCases and playback tests Change-Id: I8800744f2396f0955c76d5f3e187a69fe04330f6 --- .../cdm/src/wv_content_decryption_module.cpp | 9 ++-- .../cdm/test/request_license_test.cpp | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp index de6ed70b..edf4a148 100644 --- a/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp +++ b/libwvdrmengine/cdm/src/wv_content_decryption_module.cpp @@ -434,9 +434,12 @@ CdmResponseType WvContentDecryptionModule::CloseCdm( return UNKNOWN_ERROR; } // Remove any sessions that point to this engine. - for (auto session_it : cdm_by_session_id_) { - if (session_it.second == it->second.cdm_engine.get()) { - cdm_by_session_id_.erase(session_it.first); + for (auto session_it = cdm_by_session_id_.begin(); + session_it != cdm_by_session_id_.end();) { + if (session_it->second == it->second.cdm_engine.get()) { + session_it = cdm_by_session_id_.erase(session_it); + } else { + ++session_it; } } cdms_.erase(it); diff --git a/libwvdrmengine/cdm/test/request_license_test.cpp b/libwvdrmengine/cdm/test/request_license_test.cpp index 570283b5..203bbe0a 100644 --- a/libwvdrmengine/cdm/test/request_license_test.cpp +++ b/libwvdrmengine/cdm/test/request_license_test.cpp @@ -55,6 +55,13 @@ const wvcdm::CdmIdentifier kExampleIdentifier = { 7 }; +const wvcdm::CdmIdentifier kAlternateCdmIdentifier1 = { + "alternate_spoid_1", + "alternate_origin_1", + "com.alternate1.url", + 8 +}; + const std::string kEmptyServiceCertificate; // Protobuf generated classes @@ -5041,4 +5048,48 @@ INSTANTIATE_TEST_CASE_P( ::testing::Range(&kCenc30SwitchCipherData[0], &kCenc30SwitchCipherData[8])); +TEST_F(WvCdmRequestLicenseTest, CloseCdmReleaseResourcesTest) { + Provision(kAlternateCdmIdentifier1, kLevelDefault); + + // Retrieve a streaming license + EXPECT_EQ(NO_ERROR, decryptor_.OpenSession(config_.key_system(), NULL, + kDefaultCdmIdentifier, NULL, + &session_id_)); + GenerateKeyRequest(binary_key_id(), kLicenseTypeStreaming); + VerifyKeyRequestResponse(config_.license_server(), config_.client_auth()); + + // Open a few more sessions + CdmSessionId session_id1; + CdmSessionId session_id2; + CdmSessionId session_id3; + CdmSessionId session_id4; + CdmSessionId alternate_session_id; + EXPECT_EQ(NO_ERROR, decryptor_.OpenSession(config_.key_system(), NULL, + kDefaultCdmIdentifier, NULL, + &session_id1)); + EXPECT_EQ(NO_ERROR, decryptor_.OpenSession(config_.key_system(), NULL, + kDefaultCdmIdentifier, NULL, + &session_id2)); + EXPECT_EQ(NO_ERROR, decryptor_.OpenSession(config_.key_system(), NULL, + kDefaultCdmIdentifier, NULL, + &session_id3)); + EXPECT_EQ(NO_ERROR, decryptor_.OpenSession(config_.key_system(), NULL, + kDefaultCdmIdentifier, NULL, + &session_id4)); + EXPECT_EQ(NO_ERROR, decryptor_.OpenSession(config_.key_system(), NULL, + kAlternateCdmIdentifier1, NULL, + &alternate_session_id)); + + + // Close all open sessions and disable the timer running for |session_id_| + decryptor_.CloseCdm(kDefaultCdmIdentifier); + + EXPECT_FALSE(decryptor_.IsOpenSession(session_id_)); + EXPECT_FALSE(decryptor_.IsOpenSession(session_id1)); + EXPECT_FALSE(decryptor_.IsOpenSession(session_id2)); + EXPECT_FALSE(decryptor_.IsOpenSession(session_id3)); + EXPECT_FALSE(decryptor_.IsOpenSession(session_id4)); + EXPECT_TRUE(decryptor_.IsOpenSession(alternate_session_id)); +} + } // namespace wvcdm