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

Segfault When Running Jenkins Tests... Sometimes

Browse Source 
(This is a merge of http://go/wvgerrit/71330)

The Service Certificate unit tests actually relied on the ability to
call Properties::Init() multiple times to clear previous mutable state.
Unfortunately, they didn't check the return code that could have told
them their mutable state wasn't being cleared and instead proceeded to
use a pointer which — depending on compiler — could be totally valid and
allow the test to pass or could be invalid and cause a segfault. You can
read the bug for a fuller explanation of the mechanics.

The fix is twofold. First, the tests will now assert out if insertion
into the property set map fails, preventing segfaults. Second, a helper
has been added to Properties that allows tests interested in
re-initializing Properties to do so. The default behavior for most tests
remains the same: Properties can only be initialized once and subsequent
calls to Properties::Init() are ignored.

This patch also fixed a few formatting issues I noticed.

Bug: 123099779
Test: Jenkins Unit Tests w/ GCC
Test: CE CDM Unit Tests w/ GCC & Clang
Change-Id: Ifd29f3ddf5cff934933cf47b92ecd12ab0a4a938
This commit is contained in:
John W. Bruce
2019-01-25 14:48:00 -08:00
parent 85e2c6a026
commit dfc5909d0c
2 changed files with 36 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

16
libwvdrmengine/cdm/core/include/properties.h
View File

@@ -85,6 +85,21 @@ class Properties {
CdmClientPropertySet* property_set);
static bool RemoveSessionPropertySet(const CdmSessionId& session_id);
protected:
// This function always runs the code in |Init()| (and subsequently
// |InitOnce()|) even if Properties have already been initialized. This is
// needed by certain tests that are dependent on controlling the mutable state
// of Properties. Should not be used in general, as most tests rely on
// Properties' normal guarantee about |Init()| being safe to call multiple
// times without destroying mutable state.
static void ForceReinit() {
{
std::unique_lock<std::mutex> lock(init_mutex_);
is_initialized_ = false;
}
Init();
}
private:
static CdmClientPropertySet* GetCdmClientPropertySet(
const CdmSessionId& session_id);
@@ -111,7 +126,6 @@ class Properties {
FRIEND_TEST(CdmLicenseTest, PrepareKeyRequestValidation);
#endif
private:
// Called at least once before any properties are used.
static void InitOnce();

33
libwvdrmengine/cdm/core/test/service_certificate_unittest.cpp
View File

@@ -41,6 +41,11 @@ const std::string kTestSignedCertificate = a2bs_hex(
"D2E8D5986104AACC4DD475FD96EE9CE4E326F21B83C7058577B38732CDDABC6A6BED13FB0D"
"49D38A45EB87A5F4");
class PropertiesTestPeer : public Properties {
public:
using Properties::ForceReinit;
};
} // unnamed namespace
class ServiceCertificateTest : public ::testing::Test {
@@ -75,7 +80,9 @@ class StubCdmClientPropertySet : public CdmClientPropertySet {
uint32_t session_sharing_id() const override { return session_sharing_id_; }
void set_session_sharing_id(uint32_t id) override { session_sharing_id_ = id; }
void set_session_sharing_id(uint32_t id) override {
session_sharing_id_ = id;
}
const std::string& app_id() const override { return app_id_; }
@@ -100,8 +107,9 @@ TEST_F(ServiceCertificateTest, InitPrivacyModeRequired) {
property_set.enable_privacy_mode();
Properties::Init();
Properties::AddSessionPropertySet(kTestSessionId1, &property_set);
PropertiesTestPeer::ForceReinit();
ASSERT_TRUE(PropertiesTestPeer::AddSessionPropertySet(kTestSessionId1,
&property_set));
service_certificate_.Init(kTestSessionId1);
EXPECT_FALSE(service_certificate_.has_certificate());
@@ -113,14 +121,14 @@ TEST_F(ServiceCertificateTest, InitServiceCertificatePresent) {
property_set.enable_privacy_mode();
property_set.set_service_certificate(kTestSignedCertificate);
Properties::Init();
Properties::AddSessionPropertySet(kTestSessionId1, &property_set);
PropertiesTestPeer::ForceReinit();
ASSERT_TRUE(PropertiesTestPeer::AddSessionPropertySet(kTestSessionId1,
&property_set));
std::string raw_service_certificate;
EXPECT_TRUE(Properties::GetServiceCertificate(kTestSessionId1,
&raw_service_certificate));
EXPECT_EQ(NO_ERROR,
service_certificate_.Init(raw_service_certificate));
EXPECT_TRUE(PropertiesTestPeer::GetServiceCertificate(
kTestSessionId1, &raw_service_certificate));
EXPECT_EQ(NO_ERROR, service_certificate_.Init(raw_service_certificate));
EXPECT_TRUE(service_certificate_.has_certificate());
}
@@ -129,11 +137,12 @@ TEST_F(ServiceCertificateTest, SetServiceCertificate) {
property_set.enable_privacy_mode();
Properties::Init();
Properties::AddSessionPropertySet(kTestSessionId1, &property_set);
PropertiesTestPeer::ForceReinit();
ASSERT_TRUE(PropertiesTestPeer::AddSessionPropertySet(kTestSessionId1,
&property_set));
EXPECT_EQ(NO_ERROR, service_certificate_.Init(kTestSignedCertificate));
EXPECT_TRUE(service_certificate_.has_certificate());
}
}
} // namespace wvcdm