Require SK_DECLARE_STATIC_LAZY_PTR is used in global scope.
Function- or method- local scope isn't threadsafe; the pointer is generally
zero-initialized on first use in function scope (i.e. lazily... we have to go
deeper), but for globals we can be pretty sure the linker will do that for us.
BUG=skia:
No public API changes.
TBR=reed@google.com
Review URL: https://codereview.chromium.org/651723003
diff --git a/include/core/SkData.h b/include/core/SkData.h
index e25ef50..4f0c213 100644
--- a/include/core/SkData.h
+++ b/include/core/SkData.h
@@ -171,8 +171,7 @@
virtual void internal_dispose() const SK_OVERRIDE;
// Called the first time someone calls NewEmpty to initialize the singleton.
- static SkData* NewEmptyImpl();
- static void DeleteEmpty(SkData*);
+ friend SkData* sk_new_empty_data();
// shared internal factory
static SkData* PrivateNewWithCopy(const void* srcOrNull, size_t length);
diff --git a/include/core/SkPathRef.h b/include/core/SkPathRef.h
index 0b661b4..4b57fc8 100644
--- a/include/core/SkPathRef.h
+++ b/include/core/SkPathRef.h
@@ -421,7 +421,7 @@
/**
* Called the first time someone calls CreateEmpty to actually create the singleton.
*/
- static SkPathRef* CreateEmptyImpl();
+ friend SkPathRef* sk_create_empty_pathref();
void setIsOval(bool isOval) { fIsOval = isOval; }
diff --git a/include/ports/SkFontMgr.h b/include/ports/SkFontMgr.h
index bb8c7b7..181fe9f 100644
--- a/include/ports/SkFontMgr.h
+++ b/include/ports/SkFontMgr.h
@@ -131,7 +131,7 @@
unsigned styleBits) const = 0;
private:
static SkFontMgr* Factory(); // implemented by porting layer
- static SkFontMgr* CreateDefault();
+ friend SkFontMgr* sk_fontmgr_create_default();
typedef SkRefCnt INHERITED;
};
diff --git a/include/ports/SkRemotableFontMgr.h b/include/ports/SkRemotableFontMgr.h
index bd99497..bf24599 100644
--- a/include/ports/SkRemotableFontMgr.h
+++ b/include/ports/SkRemotableFontMgr.h
@@ -46,7 +46,8 @@
private:
SkRemotableFontIdentitySet() : fCount(0), fData() { }
- static SkRemotableFontIdentitySet* NewEmptyImpl();
+
+ friend SkRemotableFontIdentitySet* sk_remotable_font_identity_set_new();
int fCount;
SkAutoTMalloc<SkFontIdentity> fData;
diff --git a/src/core/SkData.cpp b/src/core/SkData.cpp
index 11cab7e..c5d1077 100644
--- a/src/core/SkData.cpp
+++ b/src/core/SkData.cpp
@@ -92,14 +92,13 @@
///////////////////////////////////////////////////////////////////////////////
-SkData* SkData::NewEmptyImpl() {
- return new SkData(NULL, 0, NULL, NULL);
-}
+// As a template argument these must have external linkage.
+SkData* sk_new_empty_data() { return new SkData(NULL, 0, NULL, NULL); }
+namespace { void sk_unref_data(SkData* ptr) { return SkSafeUnref(ptr); } }
-void SkData::DeleteEmpty(SkData* ptr) { SkDELETE(ptr); }
+SK_DECLARE_STATIC_LAZY_PTR(SkData, empty, sk_new_empty_data, sk_unref_data);
SkData* SkData::NewEmpty() {
- SK_DECLARE_STATIC_LAZY_PTR(SkData, empty, NewEmptyImpl, DeleteEmpty);
return SkRef(empty.get());
}
diff --git a/src/core/SkFontHost.cpp b/src/core/SkFontHost.cpp
index c582ba5..ce73491 100644
--- a/src/core/SkFontHost.cpp
+++ b/src/core/SkFontHost.cpp
@@ -198,12 +198,14 @@
return this->onLegacyCreateTypeface(familyName, styleBits);
}
-SkFontMgr* SkFontMgr::CreateDefault() {
+// As a template argument this must have external linkage.
+SkFontMgr* sk_fontmgr_create_default() {
SkFontMgr* fm = SkFontMgr::Factory();
return fm ? fm : SkNEW(SkEmptyFontMgr);
}
+SK_DECLARE_STATIC_LAZY_PTR(SkFontMgr, singleton, sk_fontmgr_create_default);
+
SkFontMgr* SkFontMgr::RefDefault() {
- SK_DECLARE_STATIC_LAZY_PTR(SkFontMgr, singleton, CreateDefault);
return SkRef(singleton.get());
}
diff --git a/src/core/SkGlyphCache.cpp b/src/core/SkGlyphCache.cpp
index ab816f9..17f4236 100755
--- a/src/core/SkGlyphCache.cpp
+++ b/src/core/SkGlyphCache.cpp
@@ -29,9 +29,10 @@
} // namespace
+SK_DECLARE_STATIC_LAZY_PTR(SkGlyphCache_Globals, globals, create_globals);
+
// Returns the shared globals
static SkGlyphCache_Globals& getSharedGlobals() {
- SK_DECLARE_STATIC_LAZY_PTR(SkGlyphCache_Globals, globals, create_globals);
return *globals.get();
}
diff --git a/src/core/SkImageFilter.cpp b/src/core/SkImageFilter.cpp
index 6f7762a..c431ece 100644
--- a/src/core/SkImageFilter.cpp
+++ b/src/core/SkImageFilter.cpp
@@ -97,7 +97,7 @@
if (!buffer.isValid() || !buffer.validate(SkIsValidRect(rect))) {
return false;
}
-
+
uint32_t flags = buffer.readUInt();
fCropRect = CropRect(rect, flags);
if (buffer.isVersionLT(SkReadBuffer::kImageFilterUniqueID_Version)) {
@@ -495,7 +495,8 @@
return SkNEW_ARGS(CacheImpl, (maxBytes));
}
+SK_DECLARE_STATIC_LAZY_PTR(SkImageFilter::Cache, cache, CreateCache);
+
SkImageFilter::Cache* SkImageFilter::Cache::Get() {
- SK_DECLARE_STATIC_LAZY_PTR(SkImageFilter::Cache, cache, CreateCache);
return cache.get();
}
diff --git a/src/core/SkLazyPtr.h b/src/core/SkLazyPtr.h
index 13218a7..0612327 100644
--- a/src/core/SkLazyPtr.h
+++ b/src/core/SkLazyPtr.h
@@ -47,15 +47,18 @@
* We may call Create more than once, but all threads will see the same pointer
* returned from get(). Any extra calls to Create will be cleaned up.
*
- * These macros must be used in a global or function scope, not as a class member.
+ * These macros must be used in a global scope, not in function scope or as a class member.
*/
#define SK_DECLARE_STATIC_LAZY_PTR(T, name, ...) \
- static Private::SkLazyPtr<T, ##__VA_ARGS__> name
+ namespace {} static Private::SkLazyPtr<T, ##__VA_ARGS__> name
#define SK_DECLARE_STATIC_LAZY_PTR_ARRAY(T, name, N, ...) \
- static Private::SkLazyPtrArray<T, N, ##__VA_ARGS__> name
+ namespace {} static Private::SkLazyPtrArray<T, N, ##__VA_ARGS__> name
+// namespace {} forces these macros to only be legal in global scopes. Chrome has thread-safety
+// problems with them in function-local statics because it uses -fno-threadsafe-statics, and even
+// in builds with threadsafe statics, those threadsafe statics are just unnecessary overhead.
// Everything below here is private implementation details. Don't touch, don't even look.
diff --git a/src/core/SkMessageBus.h b/src/core/SkMessageBus.h
index 1290ea9..a005c3b 100644
--- a/src/core/SkMessageBus.h
+++ b/src/core/SkMessageBus.h
@@ -38,19 +38,21 @@
private:
SkMessageBus();
static SkMessageBus* Get();
- static SkMessageBus* New();
+
+ // Allow SkLazyPtr to call SkMessageBus::SkMessageBus().
+ template <typename T> friend T* Private::sk_new();
SkTDArray<Inbox*> fInboxes;
SkMutex fInboxesMutex;
};
// This must go in a single .cpp file, not some .h, or we risk creating more than one global
-// SkMessageBus per type when using shared libraries.
-#define DECLARE_SKMESSAGEBUS_MESSAGE(Message) \
- template <> \
- SkMessageBus<Message>* SkMessageBus<Message>::Get() { \
- SK_DECLARE_STATIC_LAZY_PTR(SkMessageBus<Message>, bus, New); \
- return bus.get(); \
+// SkMessageBus per type when using shared libraries. NOTE: at most one per file will compile.
+#define DECLARE_SKMESSAGEBUS_MESSAGE(Message) \
+ SK_DECLARE_STATIC_LAZY_PTR(SkMessageBus<Message>, bus); \
+ template <> \
+ SkMessageBus<Message>* SkMessageBus<Message>::Get() { \
+ return bus.get(); \
}
// ----------------------- Implementation of SkMessageBus::Inbox -----------------------
@@ -97,11 +99,6 @@
SkMessageBus<Message>::SkMessageBus() {}
template <typename Message>
-/*static*/ SkMessageBus<Message>* SkMessageBus<Message>::New() {
- return SkNEW(SkMessageBus<Message>);
-}
-
-template <typename Message>
/*static*/ void SkMessageBus<Message>::Post(const Message& m) {
SkMessageBus<Message>* bus = SkMessageBus<Message>::Get();
SkAutoMutexAcquire lock(bus->fInboxesMutex);
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index 5707693..dffb6a3 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -29,14 +29,16 @@
//////////////////////////////////////////////////////////////////////////////
-SkPathRef* SkPathRef::CreateEmptyImpl() {
+// As a template argument, this must have external linkage.
+SkPathRef* sk_create_empty_pathref() {
SkPathRef* empty = SkNEW(SkPathRef);
empty->computeBounds(); // Avoids races later to be the first to do this.
return empty;
}
+SK_DECLARE_STATIC_LAZY_PTR(SkPathRef, empty, sk_create_empty_pathref);
+
SkPathRef* SkPathRef::CreateEmpty() {
- SK_DECLARE_STATIC_LAZY_PTR(SkPathRef, empty, CreateEmptyImpl);
return SkRef(empty.get());
}
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index f948787..81038bc 100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -79,8 +79,12 @@
}
};
+namespace {
+
SK_DECLARE_STATIC_MUTEX(gCreateDefaultMutex);
-SkTypeface* SkTypeface::CreateDefault(int style) {
+
+// As a template arguments, these must have external linkage.
+SkTypeface* sk_create_default_typeface(int style) {
// If backed by fontconfig, it's not safe to call SkFontHost::CreateTypeface concurrently.
// To be safe, we serialize here with a mutex so only one call to
// CreateTypeface is happening at any given time.
@@ -92,16 +96,14 @@
return t ? t : SkEmptyTypeface::Create();
}
-void SkTypeface::DeleteDefault(SkTypeface* t) {
- // The SkTypeface returned by SkFontHost::CreateTypeface may _itself_ be a
- // cleverly-shared singleton. This is less than ideal. This means we
- // cannot just assert our ownership and SkDELETE(t) like we'd want to.
- SkSafeUnref(t);
-}
+void sk_unref_typeface(SkTypeface* ptr) { SkSafeUnref(ptr); }
+
+} // namespace
+
+SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4,
+ sk_create_default_typeface, sk_unref_typeface);
SkTypeface* SkTypeface::GetDefaultTypeface(Style style) {
- SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkTypeface, defaults, 4, CreateDefault, DeleteDefault);
-
SkASSERT((int)style < 4);
return defaults[style];
}
diff --git a/src/core/SkXfermode.cpp b/src/core/SkXfermode.cpp
index 7fcdeb8..e115f6f 100644
--- a/src/core/SkXfermode.cpp
+++ b/src/core/SkXfermode.cpp
@@ -1211,7 +1211,7 @@
return fMode == s.fMode &&
fBackgroundAccess.getTexture() == s.fBackgroundAccess.getTexture();
}
-
+
virtual void onComputeInvariantOutput(InvariantOutput* inout) const SK_OVERRIDE {
inout->setToUnknown();
}
@@ -1708,6 +1708,7 @@
}
} // namespace
+SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkXfermode, cached, SkXfermode::kLastMode + 1, create_mode);
SkXfermode* SkXfermode::Create(Mode mode) {
SkASSERT(SK_ARRAY_COUNT(gProcCoeffs) == kModeCount);
@@ -1723,7 +1724,6 @@
return NULL;
}
- SK_DECLARE_STATIC_LAZY_PTR_ARRAY(SkXfermode, cached, kModeCount, create_mode);
return SkSafeRef(cached[mode]);
}
diff --git a/src/fonts/SkRemotableFontMgr.cpp b/src/fonts/SkRemotableFontMgr.cpp
index 633e914..3681f2c 100644
--- a/src/fonts/SkRemotableFontMgr.cpp
+++ b/src/fonts/SkRemotableFontMgr.cpp
@@ -16,11 +16,12 @@
*data = fData;
}
-SkRemotableFontIdentitySet* SkRemotableFontIdentitySet::NewEmptyImpl() {
+// As a template argument, this must have external linkage.
+SkRemotableFontIdentitySet* sk_remotable_font_identity_set_new() {
return SkNEW(SkRemotableFontIdentitySet);
}
+SK_DECLARE_STATIC_LAZY_PTR(SkRemotableFontIdentitySet, empty, sk_remotable_font_identity_set_new);
SkRemotableFontIdentitySet* SkRemotableFontIdentitySet::NewEmpty() {
- SK_DECLARE_STATIC_LAZY_PTR(SkRemotableFontIdentitySet, empty, NewEmptyImpl);
return SkRef(empty.get());
}
diff --git a/src/lazy/SkDiscardableMemoryPool.cpp b/src/lazy/SkDiscardableMemoryPool.cpp
index f8f44d0..96d6046 100644
--- a/src/lazy/SkDiscardableMemoryPool.cpp
+++ b/src/lazy/SkDiscardableMemoryPool.cpp
@@ -260,8 +260,9 @@
return SkNEW_ARGS(DiscardableMemoryPool, (size, mutex));
}
+SK_DECLARE_STATIC_LAZY_PTR(SkDiscardableMemoryPool, global, create_global_pool);
+
SkDiscardableMemoryPool* SkGetGlobalDiscardableMemoryPool() {
- SK_DECLARE_STATIC_LAZY_PTR(SkDiscardableMemoryPool, global, create_global_pool);
return global.get();
}