Fix handling of INSUFFICIENT_RESOURCES in LoadEntry
Bug: b/121214641 Merge of http://go/wvgerrit/69703 Test: Android + Linux unit tests LoadEntry attempts to handle INSUFFICIENT_RESOURCES by deleting an entry and retrying, but it's possible that the randomly-generated number of the entry to be deleted might match the entry we want to load. In this case, we have wasted a retry, since the code just continues on to the next iteration. This is changed to generate a number different from the entry to load. Furthermore, if the number of usage entries is 1, we break since there are no more entries to delete besides the one we want to load. The code is also changed to call srand in the creation of the usage_table_header, since without it, rand() would produce the same values, and similarly, our random generation is changed to use a simple mod. Tests are modified to reflect these changes. Change-Id: I95e125b8adbd85d0189f9d40ca15f3fe69e6d6b9
This commit is contained in:
@@ -79,6 +79,11 @@ class UsageTableHeader {
|
||||
// for the objects that DeleteEntry depends on.
|
||||
void DeleteEntryForTest(uint32_t usage_entry_number);
|
||||
|
||||
// TODO(rfrias): Move to utility class
|
||||
static int64_t GetRandomInRange(size_t upper_bound_exclusive);
|
||||
static int64_t GetRandomInRangeWithExclusion(size_t upper_bound_exclusive,
|
||||
size_t exclude);
|
||||
|
||||
private:
|
||||
CdmResponseType MoveEntry(uint32_t from /* usage entry number */,
|
||||
const CdmUsageEntry& from_usage_entry,
|
||||
@@ -127,9 +132,6 @@ class UsageTableHeader {
|
||||
|
||||
metrics::CryptoMetrics alternate_crypto_metrics_;
|
||||
|
||||
// TODO(rfrias): Move to utility class
|
||||
uint32_t GetRandomInRange(size_t upper_bound_inclusive);
|
||||
|
||||
// Test related declarations
|
||||
friend class UsageTableHeaderTest;
|
||||
|
||||
|
||||
@@ -152,8 +152,9 @@ CdmResponseType UsageTableHeader::AddEntry(
|
||||
retry_count < kMaxCryptoRetries &&
|
||||
status == INSUFFICIENT_CRYPTO_RESOURCES_3;
|
||||
++retry_count) {
|
||||
uint32_t entry_number_to_delete =
|
||||
int64_t entry_number_to_delete =
|
||||
GetRandomInRange(usage_entry_info_.size());
|
||||
if (entry_number_to_delete < 0) break;
|
||||
DeleteEntry(entry_number_to_delete, file_handle_.get(), metrics);
|
||||
|
||||
status = crypto_session->CreateUsageEntry(usage_entry_number);
|
||||
@@ -215,17 +216,18 @@ CdmResponseType UsageTableHeader::LoadEntry(CryptoSession* crypto_session,
|
||||
crypto_session->LoadUsageEntry(usage_entry_number, usage_entry);
|
||||
|
||||
// If loading a usage entry fails due to insufficient resources, release a
|
||||
// random entry and try again.
|
||||
// random entry different from |usage_entry_number| and try again. If there
|
||||
// are no more entries to release, we fail.
|
||||
for (uint32_t retry_count = 0;
|
||||
retry_count < kMaxCryptoRetries &&
|
||||
status == INSUFFICIENT_CRYPTO_RESOURCES_3;
|
||||
++retry_count) {
|
||||
uint32_t entry_number_to_delete =
|
||||
GetRandomInRange(usage_entry_info_.size());
|
||||
if (usage_entry_number != entry_number_to_delete) {
|
||||
DeleteEntry(entry_number_to_delete, file_handle_.get(), metrics);
|
||||
status = crypto_session->LoadUsageEntry(usage_entry_number, usage_entry);
|
||||
}
|
||||
// Get a random entry from the other entries.
|
||||
int64_t entry_number_to_delete = GetRandomInRangeWithExclusion(
|
||||
usage_entry_info_.size(), usage_entry_number);
|
||||
if (entry_number_to_delete < 0) break;
|
||||
DeleteEntry(entry_number_to_delete, file_handle_.get(), metrics);
|
||||
status = crypto_session->LoadUsageEntry(usage_entry_number, usage_entry);
|
||||
}
|
||||
|
||||
return status;
|
||||
@@ -707,9 +709,26 @@ bool UsageTableHeader::UpgradeUsageInfoFromUsageTable(
|
||||
return NO_ERROR;
|
||||
}
|
||||
|
||||
uint32_t UsageTableHeader::GetRandomInRange(size_t upper_bound_inclusive) {
|
||||
if (upper_bound_inclusive == 0) return 0;
|
||||
return rand() / (RAND_MAX / upper_bound_inclusive + 1);
|
||||
int64_t UsageTableHeader::GetRandomInRange(size_t upper_bound_exclusive) {
|
||||
if (upper_bound_exclusive == 0) {
|
||||
LOGE("upper_bound_exclusive must be > 0.");
|
||||
return -1;
|
||||
}
|
||||
return (int)((double)rand() / ((double)RAND_MAX + 1) * upper_bound_exclusive);
|
||||
}
|
||||
|
||||
int64_t UsageTableHeader::GetRandomInRangeWithExclusion(
|
||||
size_t upper_bound_exclusive, size_t exclude) {
|
||||
if ((upper_bound_exclusive <= 1) ||
|
||||
(exclude >= upper_bound_exclusive)) {
|
||||
LOGE(
|
||||
"upper_bound_exclusive must be > 1 and exclude must be < "
|
||||
"upper_bound_exclusive.");
|
||||
return -1;
|
||||
}
|
||||
uint32_t random = GetRandomInRange(upper_bound_exclusive - 1);
|
||||
if (random >= exclude) random++;
|
||||
return random;
|
||||
}
|
||||
|
||||
// TODO(fredgc): remove when b/65730828 is addressed
|
||||
|
||||
@@ -282,9 +282,12 @@ class MockUsageTableHeader : public UsageTableHeader {
|
||||
|
||||
// gmock methods
|
||||
using ::testing::_;
|
||||
using ::testing::AllOf;
|
||||
using ::testing::AtMost;
|
||||
using ::testing::ElementsAreArray;
|
||||
using ::testing::Ge;
|
||||
using ::testing::Invoke;
|
||||
using ::testing::Lt;
|
||||
using ::testing::NotNull;
|
||||
using ::testing::Return;
|
||||
using ::testing::SaveArg;
|
||||
@@ -977,14 +980,16 @@ TEST_F(UsageTableHeaderTest,
|
||||
MockUsageTableHeader* mock_usage_table_header = SetUpMock();
|
||||
Init(kSecurityLevelL1, kUsageTableHeader, k10UsageEntryInfoVector);
|
||||
|
||||
uint32_t usage_entry_number_to_load = rand() /
|
||||
(RAND_MAX / k10UsageEntryInfoVector.size() + 1);
|
||||
int64_t usage_entry_number_to_load =
|
||||
MockUsageTableHeader::GetRandomInRange(k10UsageEntryInfoVector.size());
|
||||
ASSERT_THAT(usage_entry_number_to_load,
|
||||
AllOf(Ge(0), Lt((int64_t)k10UsageEntryInfoVector.size())));
|
||||
CdmUsageEntry usage_entry_to_load = kUsageEntry;
|
||||
|
||||
// Setup expectations
|
||||
EXPECT_CALL(*mock_usage_table_header,
|
||||
DeleteEntry(_, device_files_, NotNull()))
|
||||
.Times(AtMost(1))
|
||||
.Times(1)
|
||||
.WillRepeatedly(
|
||||
DoAll(Invoke(this, &UsageTableHeaderTest::DeleteEntry),
|
||||
Return(NO_ERROR)));
|
||||
@@ -1008,14 +1013,16 @@ TEST_F(UsageTableHeaderTest,
|
||||
MockUsageTableHeader* mock_usage_table_header = SetUpMock();
|
||||
Init(kSecurityLevelL1, kUsageTableHeader, k10UsageEntryInfoVector);
|
||||
|
||||
uint32_t usage_entry_number_to_load = rand() /
|
||||
(RAND_MAX / k10UsageEntryInfoVector.size() + 1);
|
||||
int64_t usage_entry_number_to_load =
|
||||
MockUsageTableHeader::GetRandomInRange(k10UsageEntryInfoVector.size());
|
||||
ASSERT_THAT(usage_entry_number_to_load,
|
||||
AllOf(Ge(0), Lt((int64_t)k10UsageEntryInfoVector.size())));
|
||||
CdmUsageEntry usage_entry_to_load = kUsageEntry;
|
||||
|
||||
// Setup expectations
|
||||
EXPECT_CALL(*mock_usage_table_header,
|
||||
DeleteEntry(_, device_files_, NotNull()))
|
||||
.Times(AtMost(2))
|
||||
.Times(2)
|
||||
.WillRepeatedly(
|
||||
DoAll(Invoke(this, &UsageTableHeaderTest::DeleteEntry),
|
||||
Return(NO_ERROR)));
|
||||
@@ -1039,21 +1046,23 @@ TEST_F(UsageTableHeaderTest, LoadEntry_LoadUsageEntryFailsThrice) {
|
||||
MockUsageTableHeader* mock_usage_table_header = SetUpMock();
|
||||
Init(kSecurityLevelL1, kUsageTableHeader, k10UsageEntryInfoVector);
|
||||
|
||||
uint32_t usage_entry_number_to_load = rand() /
|
||||
(RAND_MAX / k10UsageEntryInfoVector.size() + 1);
|
||||
int64_t usage_entry_number_to_load =
|
||||
MockUsageTableHeader::GetRandomInRange(k10UsageEntryInfoVector.size());
|
||||
ASSERT_THAT(usage_entry_number_to_load,
|
||||
AllOf(Ge(0), Lt((int64_t)k10UsageEntryInfoVector.size())));
|
||||
CdmUsageEntry usage_entry_to_load = kUsageEntry;
|
||||
|
||||
// Setup expectations
|
||||
EXPECT_CALL(*mock_usage_table_header,
|
||||
DeleteEntry(_, device_files_, NotNull()))
|
||||
.Times(AtMost(3))
|
||||
.Times(3)
|
||||
.WillRepeatedly(
|
||||
DoAll(Invoke(this, &UsageTableHeaderTest::DeleteEntry),
|
||||
Return(NO_ERROR)));
|
||||
|
||||
EXPECT_CALL(*crypto_session_,
|
||||
LoadUsageEntry(usage_entry_number_to_load, usage_entry_to_load))
|
||||
.Times(AtMost(4))
|
||||
.Times(4)
|
||||
.WillRepeatedly(Return(INSUFFICIENT_CRYPTO_RESOURCES_3));
|
||||
|
||||
// Now invoke the method under test
|
||||
|
||||
Reference in New Issue
Block a user