Add sk_calloc and sk_calloc_throw to SkMemory_new_handler.cpp.

skia/ext/SkMemory_new_handler.cpp

Index: skia/ext/SkMemory_new_handler.cpp
diff --git a/skia/ext/SkMemory_new_handler.cpp b/skia/ext/SkMemory_new_handler.cpp
index dbbc4944512a2904eeb18ed3643e854d86b77205..45b8cf95442fb3597d5516048da84b3d74bc2c6b 100644
--- a/skia/ext/SkMemory_new_handler.cpp
+++ b/skia/ext/SkMemory_new_handler.cpp
@@ -11,13 +11,21 @@
 #include "third_party/skia/include/core/SkTypes.h"
 #include "third_party/skia/include/core/SkThread.h"
 
-// This implementation of sk_malloc_flags() and friends is identical
-// to SkMemory_malloc.c, except that it disables the CRT's new_handler
-// during malloc(), when SK_MALLOC_THROW is not set (ie., when
-// sk_malloc_flags() would not abort on NULL).
+// This implementation of sk_malloc_flags() and friends is identical to
+// SkMemory_malloc.cpp, except that it disables the CRT's new_handler during
+// malloc() and calloc() when SK_MALLOC_THROW is not set (because our normal
+// new_handler itself will crash on failure when using tcmalloc).

[Stephen White, 2013/09/24 22:35:02]

(Hysterical note: this is actually not tcmalloc-specific: malloc() calling the
new_handler actually came from MSC's malloc implementation; tcmalloc just
inherited it from MSC to make our lives easier when we switched to tcmalloc).

 
 SK_DECLARE_STATIC_MUTEX(gSkNewHandlerMutex);
 
+static inline void* throwOnFailure(size_t size, void* p) {

[darin (slow to review), 2013/10/05 06:12:00]

style nit: throwOnFailure -> ThrowOnFailure ?

Actually, I'm not sure what style guide applies to this file, so my suggestion
might be off base. The indentation is 4 chars instead of 2. Lines are not
wrapped to 80. Is this following Skia conventions?

[mtklein, 2013/10/07 17:32:36]

Ah, good eye.  The intent was to (continue to) follow Skia style here.  I took a
look and weirdly we were both wrong: Skia wants this as throw_on_failure. 
Uploaded a new version.

+    if (size > 0 && p == NULL) {
+        // If we've got a NULL here, the only reason we should have failed is running out of RAM.
+        sk_out_of_memory();
+    }
+    return p;
+}
+
 void sk_throw() {
     SkASSERT(!"sk_throw");
     abort();
@@ -28,19 +36,8 @@ void sk_out_of_memory(void) {
     abort();
 }
 
-void* sk_malloc_throw(size_t size) {
-    return sk_malloc_flags(size, SK_MALLOC_THROW);
-}
-
 void* sk_realloc_throw(void* addr, size_t size) {
-    void* p = realloc(addr, size);
-    if (size == 0) {
-        return p;
-    }
-    if (p == NULL) {
-        sk_throw();
-    }
-    return p;
+    return throwOnFailure(size, realloc(addr, size));
 }
 
 void sk_free(void* p) {
@@ -49,29 +46,50 @@ void sk_free(void* p) {
     }
 }
 
-void* sk_malloc_flags(size_t size, unsigned flags) {
-    void* p;
+void* sk_malloc_throw(size_t size) {
+    return throwOnFailure(size, malloc(size));
+}
+
+// Platform specific ways to try really hard to get a malloc that won't crash on failure.
+static void* sk_malloc_nothrow(size_t size) {
 #if defined(ANDROID)
-    // Android doesn't have std::set_new_handler.
-    p = malloc(size);
+    // Android doesn't have std::set_new_handler, so we just call malloc.
+    return malloc(size);
+#elif defined(OS_MACOSX) && !defined(OS_IOS)
+    return base::UncheckedMalloc(size);
 #else
-    if (!(flags & SK_MALLOC_THROW)) {
-#if defined(OS_MACOSX) && !defined(OS_IOS)
-      p = base::UncheckedMalloc(size);
-#else
-      SkAutoMutexAcquire lock(gSkNewHandlerMutex);
-      std::new_handler old_handler = std::set_new_handler(NULL);
-      p = malloc(size);
-      std::set_new_handler(old_handler);
-#endif
-    } else {
-      p = malloc(size);
-    }
+    // This is not really thread safe.  It only won't collide with itself, but we're totally
+    // unprotected from races with other code that calls set_new_handler.
+    SkAutoMutexAcquire lock(gSkNewHandlerMutex);
+    std::new_handler old_handler = std::set_new_handler(NULL);
+    void* p = malloc(size);
+    std::set_new_handler(old_handler);
+    return p;
 #endif
-    if (p == NULL) {
-        if (flags & SK_MALLOC_THROW) {
-            sk_throw();
-        }
+}
+
+void* sk_malloc_flags(size_t size, unsigned flags) {
+    if (flags & SK_MALLOC_THROW) {
+        return sk_malloc_throw(size);
     }
+    return sk_malloc_nothrow(size);
+}
+
+void* sk_calloc_throw(size_t size) {

[Stephen White, 2013/09/24 22:35:02]

Nit: shouldn't this have the same signature as calloc() itself?

[mtklein, 2013/09/26 14:44:16]

Personally, I'm with you.  But that's not what the header says.

+    return throwOnFailure(size, calloc(size, 1));
+}
+
+// Jump through the same hoops as sk_malloc_nothrow to avoid a crash, but for calloc.
+void* sk_calloc(size_t size) {
+#if defined(ANDROID)
+    return calloc(size, 1);
+#elif defined(OS_MACOSX) && !defined(OS_IOS)
+    return base::UncheckedCalloc(size, 1);
+#else
+    SkAutoMutexAcquire lock(gSkNewHandlerMutex);
+    std::new_handler old_handler = std::set_new_handler(NULL);
+    void* p = calloc(size, 1);
+    std::set_new_handler(old_handler);
     return p;
+#endif
 }