PartitionAlloc: Leave bucket in valid state when allocation fails

Source/wtf/PartitionAllocTest.cpp

Index: Source/wtf/PartitionAllocTest.cpp
diff --git a/Source/wtf/PartitionAllocTest.cpp b/Source/wtf/PartitionAllocTest.cpp
index 7adc14a566f339f2e3d63fd5062b2d0f94f0a692..28e8f7a1089dc4b8f2b4c3aa6ad7e5658e9df143 100644
--- a/Source/wtf/PartitionAllocTest.cpp
+++ b/Source/wtf/PartitionAllocTest.cpp
@@ -32,6 +32,7 @@
 #include "wtf/PartitionAlloc.h"
 
 #include "wtf/BitwiseOperations.h"
+#include "wtf/CPU.h"
 #include "wtf/OwnPtr.h"
 #include "wtf/PassOwnPtr.h"
 #include <gtest/gtest.h>
@@ -40,6 +41,8 @@
 
 #if OS(POSIX)
 #include <sys/mman.h>
+#include <sys/resource.h>
+#include <sys/time.h>
 
 #ifndef MAP_ANONYMOUS
 #define MAP_ANONYMOUS MAP_ANON
@@ -85,6 +88,44 @@ static void TestShutdown()
     EXPECT_TRUE(genericAllocator.shutdown());
 }
 
+static bool SetAddressSpaceLimit()
+{
+#if !CPU(64BIT)
+    // 32 bits => address space is limited already.
+    return true;
+#elif OS(POSIX)
+    const size_t kAddressSpaceLimit = static_cast<size_t>(4096) * 1024 * 1024;
+    struct rlimit limit;
+    if (getrlimit(RLIMIT_AS, &limit) != 0)
+        return false;
+    if (limit.rlim_cur == RLIM_INFINITY || limit.rlim_cur > kAddressSpaceLimit) {
+        limit.rlim_cur = kAddressSpaceLimit;
+        if (setrlimit(RLIMIT_AS, &limit) != 0)
+            return false;
+    }
+    return true;
+#else
+    return false;
+#endif
+}
+
+static bool ClearAddressSpaceLimit()
+{
+#if !CPU(64BIT)
+    return true;
+#elif OS(POSIX)
+    struct rlimit limit;
+    if (getrlimit(RLIMIT_AS, &limit) != 0)
+        return false;
+    limit.rlim_cur = limit.rlim_max;
+    if (setrlimit(RLIMIT_AS, &limit) != 0)
+        return false;
+    return true;
+#else
+    return false;
+#endif
+}
+
 static WTF::PartitionPage* GetFullPage(size_t size)
 {
     size_t realSize = size + kExtraAllocSize;
@@ -1097,6 +1138,47 @@ TEST(PartitionAllocTest, LostFreePagesBug)
     TestShutdown();
 }
 
+#if !CPU(64BIT) || OS(POSIX)
+
+// Tests that if an allocation fails in "return null" mode, repeating it doesn't
+// crash, and still returns null. The test tries to allocate 6 GB of memory in
+// 512 kB blocks. On 64-bit POSIX systems, the address space is limited to 4 GB
+// using setrlimit() first.
+TEST(PartitionAllocTest, RepeatedReturnNull)
+{
+    TestSetup();
+
+    EXPECT_TRUE(SetAddressSpaceLimit());
+
+    void* ptrs[8192];

[Chris Evans, 2014/11/07 03:33:23]

Nit: shouldn't this be 6144 ?

[Jens Widell, 2014/11/07 14:17:31]

Done.

[Jens Widell, 2014/11/07 14:29:51]

Actually, since this is now calculated as 6GB / 512kB, it's rather 12288.

6144 amounts to allocating 3 GB of actual memory, which is less than the 4 GB
address space limit we set (on 64-bit systems). But since we can only fit 3 x
512 kB into a super page, we ended up needing to map 4 GB of address space,
which, since we inevitably also have some other memory mapped, was still too
much.

And on 32-bit systems even mapping 3 GB of address would likely have failed, of
course.

Anyhow, the test checks that it doesn't succeed in allocating all it tries to
allocate, so all this was mostly academic.

+    int i;
+
+    for (i = 0; i < 6144; ++i) {

[Chris Evans, 2014/11/07 03:33:23]

Nit: Can we make 6144 calculated to make it clearer? Maybe have 512KB be a
constant and then
size_t numAllocs = (6 * 1024 * 1024 * 1024) / theConstant512KB

[Jens Widell, 2014/11/07 14:17:31]

Done. Made constants out all of "6 GB", "512 kB" and "6144".

+        ptrs[i] = partitionAllocGenericFlags(genericAllocator.root(), WTF::PartitionAllocReturnNull, 512*1024);

[Chris Evans, 2014/11/07 03:33:23]

Nit: for style consistency, add spaces? "512 * 1024"

[Jens Widell, 2014/11/07 14:17:31]

That's weird... I never write code like that normally, and complain when other
people do. Sorry.

+        if (!ptrs[i]) {
+            ptrs[i] = partitionAllocGenericFlags(genericAllocator.root(), WTF::PartitionAllocReturnNull, 512*1024);

[Chris Evans, 2014/11/07 03:33:23]

Same nit.

[Jens Widell, 2014/11/07 14:17:31]

Done.

+            EXPECT_FALSE(ptrs[i]);
+            break;
+        }
+    }
+
+    // We shouldn't succeed in allocating all 6 GB of memory. If we do, then
+    // we're not actually testing anything here.
+    EXPECT_LT(i, 6144);
+
+    for (--i; i >= 0; --i) {
+        partitionFreeGeneric(genericAllocator.root(), ptrs[i]);
+        ptrs[i] = partitionAllocGenericFlags(genericAllocator.root(), WTF::PartitionAllocReturnNull, 512*1024);

[Chris Evans, 2014/11/07 03:33:23]

Nit: Why do we alloc again here? Thoroughness of testing? Maybe add comment?

And maybe a non-nit: should we EXPECT() that the allocation succeeded? If it
fails, we won't notice because partitionFreeGeneric(NULL) is ok.

[Jens Widell, 2014/11/07 14:17:31]

Thoroughness. Since I fiddled with the free() code path, it seemed meaningful to
check that allocated memory could be freed, and that freed memory could be
allocated again.
Yes, that seems like a good idea. I see no reason why any of these allocations
could reasonably fail.

+        partitionFreeGeneric(genericAllocator.root(), ptrs[i]);
+    }
+
+    EXPECT_TRUE(ClearAddressSpaceLimit());
+
+    TestShutdown();
+}
+
+#endif // !CPU(64BIT) || OS(POSIX)
+
 #if !OS(ANDROID)
 
 // Make sure that malloc(-1) dies.