From: commit-bot@chromium.org Date: Fri, 17 Jan 2014 17:55:02 +0000 (+0000) Subject: Make leak counters thread-safe and turn them on by default for Debug X-Git-Tag: accepted/tizen/5.0/unified/20181102.025319~9337 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2ab1ba055536825552d6b49f0210c4e1531f02f0;p=platform%2Fupstream%2FlibSkiaSharp.git Make leak counters thread-safe and turn them on by default for Debug Make leak counters implemented with SK_DECLARE_INST_COUNT thread-safe. Enable the leak counting for Debug builds when Skia is built as a static library. Having SK_DECLARE_INST_COUNT without SK_DEFINE_INST_COUNT relies on static variables in member functions declared in the header files. These might be duplicated in the clients of the library when Skia is built as a dynamic library, producing incorrect operation. Protect the instance counter initialization step (initStep) by using SkOnce. Makes SkOnce.h part of the public API, since SkInstCnt is public. Protect the per-class child list shared variable with a per-class mutex. Changes the behavior in the way that if the child list has been "cleaned up", it will still try to create subsequent child lists. BUG=skia:1219 R=robertphillips@google.com, mtklein@google.com, bsalomon@google.com, bungeman@google.com, djsollen@google.com Author: kkinnunen@nvidia.com Review URL: https://codereview.chromium.org/99483003 git-svn-id: http://skia.googlecode.com/svn/trunk@13120 2bbb7eff-a529-9590-31e7-b0007b416f81 --- diff --git a/gm/colormatrix.cpp b/gm/colormatrix.cpp index 9ab6563..2f98693 100644 --- a/gm/colormatrix.cpp +++ b/gm/colormatrix.cpp @@ -12,9 +12,9 @@ #define WIDTH 500 #define HEIGHT 500 -class SkOnce { +class SkDoOnce { public: - SkOnce() : fOnce(false) {}; + SkDoOnce() : fOnce(false) {}; bool once() const { if (fOnce) { @@ -39,7 +39,7 @@ static void setArray(SkPaint* paint, const SkScalar array[]) { namespace skiagm { class ColorMatrixGM : public GM { - SkOnce fOnce; + SkDoOnce fOnce; void init() { if (fOnce.once()) { fSolidBitmap = this->createSolidBitmap(64, 64); diff --git a/gm/convexpaths.cpp b/gm/convexpaths.cpp index 8eb4cba..91afbfe 100644 --- a/gm/convexpaths.cpp +++ b/gm/convexpaths.cpp @@ -9,9 +9,9 @@ #include "SkRandom.h" #include "SkTArray.h" -class SkOnce : SkNoncopyable { +class SkDoOnce : SkNoncopyable { public: - SkOnce() { fDidOnce = false; } + SkDoOnce() { fDidOnce = false; } bool needToDo() const { return !fDidOnce; } bool alreadyDone() const { return fDidOnce; } @@ -27,7 +27,7 @@ private: namespace skiagm { class ConvexPathsGM : public GM { - SkOnce fOnce; + SkDoOnce fOnce; public: ConvexPathsGM() { this->setBGColor(0xFF000000); diff --git a/gyp/core.gypi b/gyp/core.gypi index 56ea42a..55447fe 100644 --- a/gyp/core.gypi +++ b/gyp/core.gypi @@ -114,7 +114,6 @@ '<(skia_src_path)/core/SkMessageBus.h', '<(skia_src_path)/core/SkMetaData.cpp', '<(skia_src_path)/core/SkMipMap.cpp', - '<(skia_src_path)/core/SkOnce.h', '<(skia_src_path)/core/SkOrderedReadBuffer.cpp', '<(skia_src_path)/core/SkOrderedWriteBuffer.cpp', '<(skia_src_path)/core/SkPackBits.cpp', @@ -256,6 +255,7 @@ '<(skia_include_path)/core/SkMath.h', '<(skia_include_path)/core/SkMatrix.h', '<(skia_include_path)/core/SkMetaData.h', + '<(skia_include_path)/core/SkOnce.h', '<(skia_include_path)/core/SkOSFile.h', '<(skia_include_path)/core/SkPackBits.h', '<(skia_include_path)/core/SkPaint.h', diff --git a/include/core/SkInstCnt.h b/include/core/SkInstCnt.h index 89bbfa1..7e5b454 100644 --- a/include/core/SkInstCnt.h +++ b/include/core/SkInstCnt.h @@ -20,9 +20,16 @@ #include "SkTypes.h" #if SK_ENABLE_INST_COUNT +// Static variables inside member functions below may be defined multiple times +// if Skia is being used as a dynamic library. Instance counting should be on +// only for static builds. See bug skia:2058. +#if defined(SKIA_DLL) +#error Instance counting works only when Skia is built as a static library. +#endif + +#include "SkOnce.h" #include "SkTArray.h" #include "SkThread.h" - extern bool gPrintInstCount; // The non-root classes just register themselves with their parent @@ -38,17 +45,16 @@ extern bool gPrintInstCount; #define SK_DECLARE_INST_COUNT_INTERNAL(className, initStep) \ class SkInstanceCountHelper { \ public: \ - typedef int (*PFCheckInstCnt)(int level, bool cleanUp); \ SkInstanceCountHelper() { \ - static bool gInited; \ - if (!gInited) { \ - initStep \ - GetChildren() = new SkTArray; \ - gInited = true; \ - } \ + SK_DECLARE_STATIC_ONCE(once); \ + SkOnce(&once, init, 0); \ sk_atomic_inc(GetInstanceCountPtr()); \ } \ \ + static void init(int) { \ + initStep \ + } \ + \ SkInstanceCountHelper(const SkInstanceCountHelper&) { \ sk_atomic_inc(GetInstanceCountPtr()); \ } \ @@ -62,11 +68,16 @@ extern bool gPrintInstCount; return &gInstanceCount; \ } \ \ - static SkTArray*& GetChildren() { \ - static SkTArray* gChildren; \ + static SkTArray*& GetChildren() { \ + static SkTArray* gChildren; \ return gChildren; \ } \ \ + static SkBaseMutex& GetChildrenMutex() { \ + SK_DECLARE_STATIC_MUTEX(childrenMutex); \ + return childrenMutex; \ + } \ + \ } fInstanceCountHelper; \ \ static int32_t GetInstanceCount() { \ @@ -86,7 +97,7 @@ extern bool gPrintInstCount; if (NULL == SkInstanceCountHelper::GetChildren()) { \ return GetInstanceCount(); \ } \ - SkTArray* children = \ + SkTArray* children = \ SkInstanceCountHelper::GetChildren(); \ int childCount = children->count(); \ int count = GetInstanceCount(); \ @@ -105,8 +116,12 @@ extern bool gPrintInstCount; } \ \ static void AddInstChild(int (*childCheckInstCnt)(int, bool)) { \ - if (CheckInstanceCount != childCheckInstCnt && \ - NULL != SkInstanceCountHelper::GetChildren()) { \ + if (CheckInstanceCount != childCheckInstCnt) { \ + SkAutoMutexAcquire ama(SkInstanceCountHelper::GetChildrenMutex()); \ + if (NULL == SkInstanceCountHelper::GetChildren()) { \ + SkInstanceCountHelper::GetChildren() = \ + new SkTArray; \ + } \ SkInstanceCountHelper::GetChildren()->push_back(childCheckInstCnt); \ } \ } diff --git a/src/core/SkOnce.h b/include/core/SkOnce.h similarity index 100% rename from src/core/SkOnce.h rename to include/core/SkOnce.h diff --git a/include/core/SkPostConfig.h b/include/core/SkPostConfig.h index 2156519..dc76522 100644 --- a/include/core/SkPostConfig.h +++ b/include/core/SkPostConfig.h @@ -120,12 +120,12 @@ * SK_ENABLE_INST_COUNT controlls printing how many reference counted objects * are still held on exit. * Defaults to 1 in DEBUG and 0 in RELEASE. - * FIXME: currently always 0, since it fails if multiple threads run at once - * (see skbug.com/1219 ). */ #ifndef SK_ENABLE_INST_COUNT # ifdef SK_DEBUG -# define SK_ENABLE_INST_COUNT 0 +// Only enabled for static builds, because instance counting relies on static +// variables in functions defined in header files. +# define SK_ENABLE_INST_COUNT !defined(SKIA_DLL) # else # define SK_ENABLE_INST_COUNT 0 # endif