Make SkString reference counting thread/TSAN safe.
BUG=skia:7107
Change-Id: I8ead98f7694faaed8e6f6f29b1fcd88501d36b66
Reviewed-on: https://skia-review.googlesource.com/54400
Reviewed-by: Mike Klein <mtklein@google.com>
Commit-Queue: Ben Wagner <bungeman@google.com>
diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index 8eee127..d0e3c12 100644
--- a/src/core/SkString.cpp
+++ b/src/core/SkString.cpp
@@ -197,7 +197,7 @@
///////////////////////////////////////////////////////////////////////////////
// the 3 values are [length] [refcnt] [terminating zero data]
-const SkString::Rec SkString::gEmptyRec = { 0, 0, 0 };
+const SkString::Rec SkString::gEmptyRec = { 0, {0}, 0 };
#define SizeOfRec() (gEmptyRec.data() - (const char*)&gEmptyRec)
@@ -231,7 +231,7 @@
// add 1 for terminating 0, then align4 so we can have some slop when growing the string
rec = (Rec*)sk_malloc_throw(SizeOfRec() + SkAlign4(len + 1));
rec->fLength = SkToU32(len);
- rec->fRefCnt = 1;
+ rec->fRefCnt.store(+1, std::memory_order_relaxed);
if (text) {
memcpy(rec->data(), text, len);
}
@@ -242,7 +242,8 @@
SkString::Rec* SkString::RefRec(Rec* src) {
if (src != &gEmptyRec) {
- sk_atomic_inc(&src->fRefCnt);
+ // No barrier required.
+ (void)src->fRefCnt.fetch_add(+1, std::memory_order_relaxed);
}
return src;
}
@@ -251,12 +252,12 @@
void SkString::validate() const {
// make sure know one has written over our global
SkASSERT(0 == gEmptyRec.fLength);
- SkASSERT(0 == gEmptyRec.fRefCnt);
+ SkASSERT(0 == gEmptyRec.fRefCnt.load(std::memory_order_relaxed));
SkASSERT(0 == gEmptyRec.data()[0]);
if (fRec != &gEmptyRec) {
SkASSERT(fRec->fLength > 0);
- SkASSERT(fRec->fRefCnt > 0);
+ SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
SkASSERT(0 == fRec->data()[fRec->fLength]);
}
}
@@ -298,8 +299,8 @@
this->validate();
if (fRec->fLength) {
- SkASSERT(fRec->fRefCnt > 0);
- if (sk_atomic_dec(&fRec->fRefCnt) == 1) {
+ SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
+ if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
sk_free(fRec);
}
}
@@ -351,8 +352,8 @@
this->validate();
if (fRec->fLength) {
- SkASSERT(fRec->fRefCnt > 0);
- if (sk_atomic_dec(&fRec->fRefCnt) == 1) {
+ SkASSERT(fRec->fRefCnt.load(std::memory_order_relaxed) > 0);
+ if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
sk_free(fRec);
}
}
@@ -364,9 +365,9 @@
this->validate();
if (fRec->fLength) {
- if (fRec->fRefCnt > 1) {
+ if (!fRec->unique()) {
Rec* rec = AllocRec(fRec->data(), fRec->fLength);
- if (sk_atomic_dec(&fRec->fRefCnt) == 1) {
+ if (1 == fRec->fRefCnt.fetch_add(-1, std::memory_order_acq_rel)) {
// In this case after our check of fRecCnt > 1, we suddenly
// did become the only owner, so now we have two copies of the
// data (fRec and rec), so we need to delete one of them.
@@ -384,10 +385,10 @@
void SkString::set(const char text[], size_t len) {
len = trim_size_t_to_u32(len);
-
+ bool unique = fRec->unique();
if (0 == len) {
this->reset();
- } else if (1 == fRec->fRefCnt && len <= fRec->fLength) {
+ } else if (unique && len <= fRec->fLength) {
// should we resize if len <<<< fLength, to save RAM? (e.g. len < (fLength>>1))?
// just use less of the buffer without allocating a smaller one
char* p = this->writable_str();
@@ -396,7 +397,7 @@
}
p[len] = 0;
fRec->fLength = SkToU32(len);
- } else if (1 == fRec->fRefCnt && (fRec->fLength >> 2) == (len >> 2)) {
+ } else if (unique && (fRec->fLength >> 2) == (len >> 2)) {
// we have spare room in the current allocation, so don't alloc a larger one
char* p = this->writable_str();
if (text) {
@@ -472,7 +473,7 @@
which is equivalent for testing to (length + 1 + 3) >> 2 == (length + 1 + 3 + len) >> 2
and we can then eliminate the +1+3 since that doesn't affec the answer
*/
- if (1 == fRec->fRefCnt && (length >> 2) == ((length + len) >> 2)) {
+ if (fRec->unique() && (length >> 2) == ((length + len) >> 2)) {
char* dst = this->writable_str();
if (offset < length) {