SkOnce: add option to call another cleanup function once at exit.

Use this to clean up empty SkData and SkPathRef.

Current leaks:
  Leaked SkRefCntBase: 40
     Leaked SkFlattenable: 32
         Leaked SkPixelRef: 32
             Leaked SkMallocPixelRef: 32
     Leaked SkFontConfigInterface: 1
     Leaked SkWeakRefCnt: 1
         Leaked SkTypeface: 1
     Leaked SkFontMgr: 1
     Leaked SkDataTable: 3
     Leaked SkImage: 1
         Leaked ???: 1
     Leaked ???: 1

BUG=skia:
R=halcanary@google.com, reed@google.com

Author: mtklein@google.com

Review URL: https://codereview.chromium.org/132803005

git-svn-id: http://skia.googlecode.com/svn/trunk@13180 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/core/SkData.h b/include/core/SkData.h
index b5dc66f..bee0fcd 100644 (file)
--- a/include/core/SkData.h
+++ b/include/core/SkData.h
@@ -137,7 +137,7 @@ private:
     virtual ~SkData();
 
     // Called the first time someone calls NewEmpty to initialize the singleton.
-    static void NewEmptyImpl(SkData**);
+    static void NewEmptyImpl(int/*unused*/);
 
     typedef SkRefCnt INHERITED;
 };

diff --git a/include/core/SkOnce.h b/include/core/SkOnce.h
index 9c4ccd4..9663ef1 100644 (file)
--- a/include/core/SkOnce.h
+++ b/include/core/SkOnce.h
@@ -25,6 +25,8 @@
 // }
 //
 // OnceTest.cpp also should serve as a few other simple examples.
+//
+// You may optionally pass SkOnce a second function to be called at exit for cleanup.
 
 #include "SkThread.h"
 #include "SkTypes.h"
@@ -35,7 +37,7 @@
 struct SkOnceFlag;  // If manually created, initialize with SkOnceFlag once = SK_ONCE_INIT
 
 template <typename Func, typename Arg>
-inline void SkOnce(SkOnceFlag* once, Func f, Arg arg);
+inline void SkOnce(SkOnceFlag* once, Func f, Arg arg, void(*atExit)() = NULL);
 
 //  ----------------------  Implementation details below here. -----------------------------
 
@@ -92,10 +94,13 @@ inline static void acquire_barrier() {
 // This should be rarely called, so we separate it from SkOnce and don't mark it as inline.
 // (We don't mind if this is an actual function call, but odds are it'll be inlined anyway.)
 template <typename Func, typename Arg>
-static void sk_once_slow(SkOnceFlag* once, Func f, Arg arg) {
+static void sk_once_slow(SkOnceFlag* once, Func f, Arg arg, void (*atExit)()) {
     const SkAutoSpinlock lock(&once->lock);
     if (!once->done) {
         f(arg);
+        if (atExit != NULL) {
+            atexit(atExit);
+        }
         // Also known as a store-store/load-store barrier, this makes sure that the writes
         // done before here---in particular, those done by calling f(arg)---are observable
         // before the writes after the line, *done = true.
@@ -127,10 +132,10 @@ void AnnotateBenignRace(const char* file, int line, const volatile void* mem, co
 
 // This is our fast path, called all the time.  We do really want it to be inlined.
 template <typename Func, typename Arg>
-inline void SkOnce(SkOnceFlag* once, Func f, Arg arg) {
+inline void SkOnce(SkOnceFlag* once, Func f, Arg arg, void(*atExit)()) {
     SK_ANNOTATE_BENIGN_RACE(&(once->done), "Don't worry TSAN, we're sure this is safe.");
     if (!once->done) {
-        sk_once_slow(once, f, arg);
+        sk_once_slow(once, f, arg, atExit);
     }
     // Also known as a load-load/load-store barrier, this acquire barrier makes
     // sure that anything we read from memory---in particular, memory written by

diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h
index e10a06f..8802714 100644 (file)
--- a/include/core/SkPathRef.h
+++ b/include/core/SkPathRef.h
@@ -418,7 +418,7 @@ private:
     /**
      * Called the first time someone calls CreateEmpty to actually create the singleton.
      */
-    static void CreateEmptyImpl(SkPathRef** empty);
+    static void CreateEmptyImpl(int/*unused*/);
 
     void setIsOval(bool isOval) { fIsOval = isOval; }
 

diff --git a/src/core/SkData.cpp b/src/core/SkData.cpp
index fd963a9..176e93c 100644 (file)
--- a/src/core/SkData.cpp
+++ b/src/core/SkData.cpp
@@ -48,16 +48,18 @@ size_t SkData::copyRange(size_t offset, size_t length, void* buffer) const {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-void SkData::NewEmptyImpl(SkData** empty) {
-    *empty = new SkData(NULL, 0, NULL, NULL);
+static SkData* gEmptyDataRef = NULL;
+static void cleanup_gEmptyDataRef() { gEmptyDataRef->unref(); }
+
+void SkData::NewEmptyImpl(int) {
+    gEmptyDataRef = new SkData(NULL, 0, NULL, NULL);
 }
 
 SkData* SkData::NewEmpty() {
-    static SkData* gEmptyRef;
     SK_DECLARE_STATIC_ONCE(once);
-    SkOnce(&once, SkData::NewEmptyImpl, &gEmptyRef);
-    gEmptyRef->ref();
-    return gEmptyRef;
+    SkOnce(&once, SkData::NewEmptyImpl, 0, cleanup_gEmptyDataRef);
+    gEmptyDataRef->ref();
+    return gEmptyDataRef;
 }
 
 // assumes fPtr was allocated via sk_malloc

diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index b83a451..161eb80 100644 (file)
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -28,15 +28,17 @@ SkPathRef::Editor::Editor(SkAutoTUnref<SkPathRef>* pathRef,
 }
 
 //////////////////////////////////////////////////////////////////////////////
-void SkPathRef::CreateEmptyImpl(SkPathRef** empty) {
-    *empty = SkNEW(SkPathRef);
-    (*empty)->computeBounds();  // Preemptively avoid a race to clear fBoundsIsDirty.
+static SkPathRef* gEmptyPathRef = NULL;
+static void cleanup_gEmptyPathRef() { gEmptyPathRef->unref(); }
+
+void SkPathRef::CreateEmptyImpl(int) {
+    gEmptyPathRef = SkNEW(SkPathRef);
+    gEmptyPathRef->computeBounds();  // Preemptively avoid a race to clear fBoundsIsDirty.
 }
 
 SkPathRef* SkPathRef::CreateEmpty() {
-    static SkPathRef* gEmptyPathRef;
     SK_DECLARE_STATIC_ONCE(once);
-    SkOnce(&once, SkPathRef::CreateEmptyImpl, &gEmptyPathRef);
+    SkOnce(&once, SkPathRef::CreateEmptyImpl, 0, cleanup_gEmptyPathRef);
     return SkRef(gEmptyPathRef);
 }
 

diff --git a/src/core/SkScaledImageCache.cpp b/src/core/SkScaledImageCache.cpp
index 2bc692a..6a1264e 100644 (file)
--- a/src/core/SkScaledImageCache.cpp
+++ b/src/core/SkScaledImageCache.cpp
@@ -683,21 +683,22 @@ void SkScaledImageCache::dump() const {
 #include "SkThread.h"
 
 SK_DECLARE_STATIC_MUTEX(gMutex);
+static SkScaledImageCache* gScaledImageCache = NULL;
+static void cleanup_gScaledImageCache() { SkDELETE(gScaledImageCache); }
 
-static void create_cache(SkScaledImageCache** cache) {
+static void create_cache(int) {
 #ifdef SK_USE_DISCARDABLE_SCALEDIMAGECACHE
-    *cache = SkNEW_ARGS(SkScaledImageCache, (SkDiscardableMemory::Create));
+    gScaledImageCache = SkNEW_ARGS(SkScaledImageCache, (SkDiscardableMemory::Create));
 #else
-    *cache = SkNEW_ARGS(SkScaledImageCache, (SK_DEFAULT_IMAGE_CACHE_LIMIT));
+    gScaledImageCache = SkNEW_ARGS(SkScaledImageCache, (SK_DEFAULT_IMAGE_CACHE_LIMIT));
 #endif
 }
 
 static SkScaledImageCache* get_cache() {
-    static SkScaledImageCache* gCache(NULL);
-    SK_DECLARE_STATIC_ONCE(create_cache_once);
-    SkOnce(&create_cache_once, create_cache, &gCache);
-    SkASSERT(NULL != gCache);
-    return gCache;
+    SK_DECLARE_STATIC_ONCE(once);
+    SkOnce(&once, create_cache, 0, cleanup_gScaledImageCache);
+    SkASSERT(NULL != gScaledImageCache);
+    return gScaledImageCache;
 }
 
 

diff --git a/tests/CachedDecodingPixelRefTest.cpp b/tests/CachedDecodingPixelRefTest.cpp
index a04ebe6..f984123 100644 (file)
--- a/tests/CachedDecodingPixelRefTest.cpp
+++ b/tests/CachedDecodingPixelRefTest.cpp
@@ -142,12 +142,6 @@ static void test_three_encodings(skiatest::Reporter* reporter,
     }
 }
 
-static void purge_global_scaled_image_cache() {
-    size_t byteLimit = SkScaledImageCache::GetByteLimit();
-    SkScaledImageCache::SetByteLimit(0);
-    SkScaledImageCache::SetByteLimit(byteLimit);
-}
-
 ////////////////////////////////////////////////////////////////////////////////
 static bool install_skCachingPixelRef(SkData* encoded, SkBitmap* dst) {
     return SkCachingPixelRef::Install(
@@ -169,7 +163,6 @@ static bool install_skDiscardablePixelRef(SkData* encoded, SkBitmap* dst) {
  */
 DEF_TEST(DecodingImageGenerator, reporter) {
     test_three_encodings(reporter, install_skCachingPixelRef);
-    purge_global_scaled_image_cache();
     test_three_encodings(reporter, install_skDiscardablePixelRef);
 }
 
@@ -302,8 +295,6 @@ DEF_TEST(DiscardableAndCachingPixelRef, reporter) {
     check_pixelref(TestImageGenerator::kSucceedGetPixels_TestType,
                    reporter, kSkCaching_PixelRefType, NULL);
 
-    purge_global_scaled_image_cache();
-
     check_pixelref(TestImageGenerator::kFailGetInfo_TestType,
                    reporter, kSkDiscardable_PixelRefType, NULL);
     check_pixelref(TestImageGenerator::kFailGetPixels_TestType,

diff --git a/tests/OnceTest.cpp b/tests/OnceTest.cpp
index f9ab427..3a99c39 100644 (file)
--- a/tests/OnceTest.cpp
+++ b/tests/OnceTest.cpp
@@ -77,3 +77,15 @@ DEF_TEST(SkOnce_Multithreaded, r) {
     // Only one should have done the +=.
     REPORTER_ASSERT(r, 6 == x);
 }
+
+// Test that the atExit option works.
+static int gToDecrement = 1;
+static void noop(int) {}
+static void decrement() { gToDecrement--; }
+static void checkDecremented() { SkASSERT(gToDecrement == 0); }
+
+DEF_TEST(SkOnce_atExit, r) {
+    atexit(checkDecremented);
+    SK_DECLARE_STATIC_ONCE(once);
+    SkOnce(&once, noop, 0, decrement);
+}