Skip to content

Commit

Permalink
Convert users of DoReturnNullTest to be death tests
Browse files Browse the repository at this point in the history
This worker function is designed to test the behaviour of the
partition allocator by restricting the amount of memory available and
then intentionally exhausting it. Doing this leaves some of the global
state associated with the allocator in a bad state. Thus these tests
are non-hermetic. This Cl wraps up the call to the work function in a
EXPECT_DEATH to force the test to be run its own process, which
prevents the tests from poluting other tests.

This removes the need to have them in their own class and eliminates
the flakiness being seen, since run order no longer affects the
viability of future test cases.

BUG=chromium:851148

Change-Id: I6d63b769d73c0e725a4e0c811d816b62ad15d57b
Reviewed-on: https://chromium-review.googlesource.com/1106602
Reviewed-by: Tom Sepez <tsepez@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Commit-Queue: Ryan Harrison <rharrison@chromium.org>
Cr-Commit-Position: refs/heads/master@{#570041}
  • Loading branch information
zoddicus authored and Commit Bot committed Jun 25, 2018
1 parent acd5eda commit 6141b62
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 33 deletions.
61 changes: 31 additions & 30 deletions base/allocator/partition_allocator/partition_alloc_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ class PartitionAllocTest : public testing::Test {
if (!IsLargeMemoryDevice()) {
LOG(WARNING)
<< "Skipping test on this device because of crbug.com/678782";
return;
LOG(FATAL) << "DoReturnNullTest";
}

ASSERT_TRUE(SetAddressSpaceLimit());
Expand Down Expand Up @@ -242,6 +242,7 @@ class PartitionAllocTest : public testing::Test {
generic_allocator.root()->Free(ptrs);

EXPECT_TRUE(ClearAddressSpaceLimit());
LOG(FATAL) << "DoReturnNullTest";
}

SizeSpecificPartitionAllocator<kTestMaxAllocation> allocator;
Expand Down Expand Up @@ -1282,6 +1283,9 @@ TEST_F(PartitionAllocTest, LostFreePagesBug) {
EXPECT_TRUE(bucket->decommitted_pages_head);
}

// Death tests misbehave on Android, http://crbug.com/643760.
#if defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)

// Unit tests that check if an allocation fails in "return null" mode,
// repeating it doesn't crash, and still returns null. The tests need to
// stress memory subsystem limits to do so, hence they try to allocate
Expand All @@ -1295,47 +1299,44 @@ TEST_F(PartitionAllocTest, LostFreePagesBug) {
// they tend to get OOM-killed rather than pass.
// TODO(https://crbug.com/779645): Fuchsia currently sets OS_POSIX, but does
// not provide a working setrlimit().
#if !defined(ARCH_CPU_64_BITS) || \
(defined(OS_POSIX) && \
!(defined(OS_FUCHSIA) || defined(OS_MACOSX) || defined(OS_ANDROID)))

// This is defined as a separate test class because RepeatedReturnNull
// test exhausts the process memory, and breaks any test in the same
// class that runs after it.
class PartitionAllocReturnNullTest : public PartitionAllocTest {};

// Test "return null" for larger, direct-mapped allocations first. As a
// direct-mapped allocation's pages are unmapped and freed on release, this
// test is performd first for these "return null" tests in order to leave
// sufficient unreserved virtual memory around for the later one(s).
TEST_F(PartitionAllocReturnNullTest, RepeatedReturnNullDirect) {
//
// Disable these test on Windows, since they run slower, so tend to timout and
// cause flake.
#if !defined(OS_WIN) && \
(!defined(ARCH_CPU_64_BITS) || \
(defined(OS_POSIX) && \
!(defined(OS_FUCHSIA) || defined(OS_MACOSX) || defined(OS_ANDROID))))

// The following four tests wrap a called function in an expect death statement
// to perform their test, because they are non-hermetic. Specifically they are
// going to attempt to exhaust the allocatable memory, which leaves the
// allocator in a bad global state.
// Performing them as death tests causes them to be forked into their own
// process, so they won't pollute other tests.
TEST_F(PartitionAllocDeathTest, RepeatedAllocReturnNullDirect) {
// A direct-mapped allocation size.
DoReturnNullTest(32 * 1024 * 1024, false);
EXPECT_DEATH(DoReturnNullTest(32 * 1024 * 1024, false), "DoReturnNullTest");
}

// Test "return null" with a 512 kB block size.
TEST_F(PartitionAllocReturnNullTest, RepeatedReturnNull) {
// A single-slot but non-direct-mapped allocation size.
DoReturnNullTest(512 * 1024, false);
// Repeating above test with Realloc
TEST_F(PartitionAllocDeathTest, RepeatedReallocReturnNullDirect) {
EXPECT_DEATH(DoReturnNullTest(32 * 1024 * 1024, true), "DoReturnNullTest");
}

// Repeating the above tests using Realloc instead of Alloc.
class PartitionReallocReturnNullTest : public PartitionAllocTest {};

TEST_F(PartitionReallocReturnNullTest, RepeatedReturnNullDirect) {
DoReturnNullTest(32 * 1024 * 1024, true);
// Test "return null" with a 512 kB block size.
TEST_F(PartitionAllocDeathTest, RepeatedAllocReturnNull) {
// A single-slot but non-direct-mapped allocation size.
EXPECT_DEATH(DoReturnNullTest(512 * 1024, false), "DoReturnNullTest");
}

TEST_F(PartitionReallocReturnNullTest, RepeatedReturnNull) {
DoReturnNullTest(512 * 1024, true);
// Repeating above test with Realloc.
TEST_F(PartitionAllocDeathTest, RepeatedReallocReturnNull) {
EXPECT_DEATH(DoReturnNullTest(512 * 1024, true), "DoReturnNullTest");
}

#endif // !defined(ARCH_CPU_64_BITS) || (defined(OS_POSIX) &&
// !(defined(OS_FUCHSIA) || defined(OS_MACOSX) || defined(OS_ANDROID)))

// Death tests misbehave on Android, http://crbug.com/643760.
#if defined(GTEST_HAS_DEATH_TEST) && !defined(OS_ANDROID)

// Make sure that malloc(-1) dies.
// In the past, we had an integer overflow that would alias malloc(-1) to
// malloc(0), which is not good.
Expand Down
6 changes: 3 additions & 3 deletions chromecast/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -200,10 +200,10 @@ cast_test_group("cast_tests") {
gtest_excludes = []
if (target_os == "linux") {
if (is_cast_desktop_build) {
# Disable PartitionAllocReturnNullTest.RepeatedReturnNullDirect (b/67975693)
# Disable PartitionAllocDeathTest.Repeated*ReturnNullDirect (b/67975693)
gtest_excludes += [
"PartitionAllocReturnNullTest.RepeatedReturnNullDirect",
"PartitionReallocReturnNullTest.RepeatedReturnNullDirect",
"PartitionAllocDeathTest.RepeatedAllocReturnNullDirect",
"PartitionAllocDeathTest.RepeatedReallocReturnNullDirect",
]
} else {
# Disable ProcessMetricsTest.GetNumberOfThreads (b/15610509)
Expand Down

0 comments on commit 6141b62

Please sign in to comment.