Make listener lists threadsafe with a mutex.
PS1 reverts commit 6c8d242b14355bf66c9137e9e4d6c7861d22168f.
PS2 uses an SkMutex for thread safety.
Change-Id: I9318f92cc028844b3dc5a99a00282c2762057895
Reviewed-on: https://skia-review.googlesource.com/155060
Reviewed-by: Mike Klein <mtklein@google.com>
diff --git a/include/core/SkPixelRef.h b/include/core/SkPixelRef.h
index 12f78ad..4c15afd 100644
--- a/include/core/SkPixelRef.h
+++ b/include/core/SkPixelRef.h
@@ -86,10 +86,9 @@
struct GenIDChangeListener {
virtual ~GenIDChangeListener() {}
virtual void onChange() = 0;
- GenIDChangeListener* next;
};
- // Takes ownership of listener.
+ // Takes ownership of listener. Threadsafe.
void addGenIDChangeListener(GenIDChangeListener* listener);
// Call when this pixelref is part of the key to a resourcecache entry. This allows the cache
@@ -117,7 +116,8 @@
const uint32_t fStableID;
#endif
- std::atomic<GenIDChangeListener*> fGenIDChangeListeners{nullptr}; // pointers are owned
+ SkMutex fGenIDChangeListenersMutex;
+ SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are owned
// Set true by caches when they cache content that's derived from the current pixels.
std::atomic<bool> fAddedToCache;
diff --git a/include/private/SkPathRef.h b/include/private/SkPathRef.h
index e997e56..92568d8 100644
--- a/include/private/SkPathRef.h
+++ b/include/private/SkPathRef.h
@@ -8,10 +8,9 @@
#ifndef SkPathRef_DEFINED
#define SkPathRef_DEFINED
-#include <atomic>
-
#include "SkAtomics.h"
#include "SkMatrix.h"
+#include "SkMutex.h"
#include "SkPoint.h"
#include "SkRRect.h"
#include "SkRect.h"
@@ -313,10 +312,9 @@
struct GenIDChangeListener : SkRefCnt {
virtual ~GenIDChangeListener() {}
virtual void onChange() = 0;
- GenIDChangeListener* next;
};
- void addGenIDChangeListener(sk_sp<GenIDChangeListener>);
+ void addGenIDChangeListener(sk_sp<GenIDChangeListener>); // Threadsafe.
bool isValid() const;
SkDEBUGCODE(void validate() const { SkASSERT(this->isValid()); } )
@@ -547,7 +545,8 @@
mutable uint32_t fGenerationID;
SkDEBUGCODE(int32_t fEditorsAttached;) // assert that only one editor in use at any time.
- std::atomic<GenIDChangeListener*> fGenIDChangeListeners{nullptr}; // pointers are reffed
+ SkMutex fGenIDChangeListenersMutex;
+ SkTDArray<GenIDChangeListener*> fGenIDChangeListeners; // pointers are reffed
mutable uint8_t fBoundsIsDirty;
mutable bool fIsFinite; // only meaningful if bounds are valid
diff --git a/modules/pathkit/compile.sh b/modules/pathkit/compile.sh
index 3d1f33a..25bc3d5 100755
--- a/modules/pathkit/compile.sh
+++ b/modules/pathkit/compile.sh
@@ -121,12 +121,14 @@
src/core/SkPoint.cpp \
src/core/SkRRect.cpp \
src/core/SkRect.cpp \
+src/core/SkSemaphore.cpp \
src/core/SkStream.cpp \
src/core/SkString.cpp \
src/core/SkStringUtils.cpp \
src/core/SkStroke.cpp \
src/core/SkStrokeRec.cpp \
src/core/SkStrokerPriv.cpp \
+src/core/SkThreadID.cpp \
src/core/SkUtils.cpp \
src/effects/SkDashPathEffect.cpp \
src/effects/SkTrimPathEffect.cpp \
diff --git a/src/core/SkPathRef.cpp b/src/core/SkPathRef.cpp
index fa0ddba..495f29b 100644
--- a/src/core/SkPathRef.cpp
+++ b/src/core/SkPathRef.cpp
@@ -700,28 +700,22 @@
}
void SkPathRef::addGenIDChangeListener(sk_sp<GenIDChangeListener> listener) {
-
if (nullptr == listener || this == gEmpty) {
return;
}
-
- SkPathRef::GenIDChangeListener* ptr = listener.release();
- ptr->next = fGenIDChangeListeners;
- while (!fGenIDChangeListeners.compare_exchange_weak(ptr->next, ptr)) {}
+ SkAutoMutexAcquire lock(fGenIDChangeListenersMutex);
+ *fGenIDChangeListeners.append() = listener.release();
}
// we need to be called *before* the genID gets changed or zerod
void SkPathRef::callGenIDChangeListeners() {
-
- SkPathRef::GenIDChangeListener* ptr = fGenIDChangeListeners.exchange(nullptr);
-
- while (ptr != nullptr) {
- ptr->onChange();
- auto next = ptr->next;
- // Listeners get at most one shot, so whether these triggered or not, blow them away.
- ptr->unref();
- ptr = next;
+ SkAutoMutexAcquire lock(fGenIDChangeListenersMutex);
+ for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
+ fGenIDChangeListeners[i]->onChange();
}
+
+ // Listeners get at most one shot, so whether these triggered or not, blow them away.
+ fGenIDChangeListeners.unrefAll();
}
SkRRect SkPathRef::getRRect() const {
diff --git a/src/core/SkPixelRef.cpp b/src/core/SkPixelRef.cpp
index c0cc42b..ac9e773 100644
--- a/src/core/SkPixelRef.cpp
+++ b/src/core/SkPixelRef.cpp
@@ -92,19 +92,17 @@
delete listener;
return;
}
-
- listener->next = fGenIDChangeListeners;
- while (!fGenIDChangeListeners.compare_exchange_weak(listener->next, listener)) {}
+ SkAutoMutexAcquire lock(fGenIDChangeListenersMutex);
+ *fGenIDChangeListeners.append() = listener;
}
// we need to be called *before* the genID gets changed or zerod
void SkPixelRef::callGenIDChangeListeners() {
+ SkAutoMutexAcquire lock(fGenIDChangeListenersMutex);
// We don't invalidate ourselves if we think another SkPixelRef is sharing our genID.
- GenIDChangeListener* head = fGenIDChangeListeners.exchange(nullptr);
-
if (this->genIDIsUnique()) {
- for (auto cursor = head; cursor != nullptr; cursor = cursor->next) {
- cursor->onChange();
+ for (int i = 0; i < fGenIDChangeListeners.count(); i++) {
+ fGenIDChangeListeners[i]->onChange();
}
// TODO: SkAtomic could add "old_value = atomic.xchg(new_value)" to make this clearer.
@@ -114,11 +112,7 @@
}
}
// Listeners get at most one shot, so whether these triggered or not, blow them away.
- for (auto cursor = head; cursor != nullptr;) {
- auto next = cursor->next;
- delete cursor;
- cursor = next;
- }
+ fGenIDChangeListeners.deleteAll();
}
void SkPixelRef::notifyPixelsChanged() {