make string's reference counting thread-safe

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

diff --git a/include/core/SkString.h b/include/core/SkString.h
index 9da6a3c..3fa367b 100644 (file)
--- a/include/core/SkString.h
+++ b/include/core/SkString.h
@@ -66,7 +66,7 @@ public:
                 SkString(const SkString&);
                 ~SkString();
 
-    bool        isEmpty() const { return fRec->fLength == 0; }
+    bool        isEmpty() const { return 0 == fRec->fLength; }
     size_t      size() const { return (size_t) fRec->fLength; }
     const char* c_str() const { return fRec->data(); }
     char operator[](size_t n) const { return this->c_str()[n]; }
@@ -151,8 +151,8 @@ public:
 private:
     struct Rec {
     public:
-        uint16_t    fLength;
-        uint16_t    fRefCnt;
+        size_t      fLength;
+        int32_t     fRefCnt;
         char        fBeginningOfData;
 
         char* data() { return &fBeginningOfData; }
@@ -168,7 +168,7 @@ private:
 #endif
 
     static const Rec gEmptyRec;
-    static Rec* AllocRec(const char text[], U16CPU len);
+    static Rec* AllocRec(const char text[], size_t len);
     static Rec* RefRec(Rec*);
 };
 

diff --git a/src/core/SkString.cpp b/src/core/SkString.cpp
index 4490423..4658ff8 100644 (file)
--- a/src/core/SkString.cpp
+++ b/src/core/SkString.cpp
@@ -9,6 +9,7 @@
 
 #include "SkString.h"
 #include "SkFixed.h"
+#include "SkThread.h"
 #include "SkUtils.h"
 #include <stdarg.h>
 #include <stdio.h>
@@ -181,22 +182,20 @@ char* SkStrAppendFixed(char string[], SkFixed x) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
-#define kMaxRefCnt_SkString     SK_MaxU16
-
 // the 3 values are [length] [refcnt] [terminating zero data]
 const SkString::Rec SkString::gEmptyRec = { 0, 0, 0 };
 
 #define SizeOfRec()     (gEmptyRec.data() - (const char*)&gEmptyRec)
 
-SkString::Rec* SkString::AllocRec(const char text[], U16CPU len) {
+SkString::Rec* SkString::AllocRec(const char text[], size_t len) {
     Rec* rec;
 
-    if (len == 0) {
+    if (0 == len) {
         rec = const_cast<Rec*>(&gEmptyRec);
     } else {
         // 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 = SkToU16(len);
+        rec->fLength = len;
         rec->fRefCnt = 1;
         if (text) {
             memcpy(rec->data(), text, len);
@@ -208,11 +207,7 @@ SkString::Rec* SkString::AllocRec(const char text[], U16CPU len) {
 
 SkString::Rec* SkString::RefRec(Rec* src) {
     if (src != &gEmptyRec) {
-        if (src->fRefCnt == kMaxRefCnt_SkString) {
-            src = AllocRec(src->data(), src->fLength);
-        } else {
-            src->fRefCnt += 1;
-        }
+        sk_atomic_inc(&src->fRefCnt);
     }
     return src;
 }
@@ -220,14 +215,14 @@ SkString::Rec* SkString::RefRec(Rec* src) {
 #ifdef SK_DEBUG
 void SkString::validate() const {
     // make sure know one has written over our global
-    SkASSERT(gEmptyRec.fLength == 0);
-    SkASSERT(gEmptyRec.fRefCnt == 0);
-    SkASSERT(gEmptyRec.data()[0] == 0);
+    SkASSERT(0 == gEmptyRec.fLength);
+    SkASSERT(0 == gEmptyRec.fRefCnt);
+    SkASSERT(0 == gEmptyRec.data()[0]);
 
     if (fRec != &gEmptyRec) {
         SkASSERT(fRec->fLength > 0);
         SkASSERT(fRec->fRefCnt > 0);
-        SkASSERT(fRec->data()[fRec->fLength] == 0);
+        SkASSERT(0 == fRec->data()[fRec->fLength]);
     }
     SkASSERT(fStr == c_str());
 }
@@ -280,7 +275,7 @@ SkString::~SkString() {
 
     if (fRec->fLength) {
         SkASSERT(fRec->fRefCnt > 0);
-        if (--fRec->fRefCnt == 0) {
+        if (sk_atomic_dec(&fRec->fRefCnt) == 1) {
             sk_free(fRec);
         }
     }
@@ -324,7 +319,7 @@ void SkString::reset() {
 
     if (fRec->fLength) {
         SkASSERT(fRec->fRefCnt > 0);
-        if (--fRec->fRefCnt == 0) {
+        if (sk_atomic_dec(&fRec->fRefCnt) == 1) {
             sk_free(fRec);
         }
     }
@@ -340,8 +335,14 @@ char* SkString::writable_str() {
 
     if (fRec->fLength) {
         if (fRec->fRefCnt > 1) {
-            fRec->fRefCnt -= 1;
-            fRec = AllocRec(fRec->data(), fRec->fLength);
+            Rec* rec = AllocRec(fRec->data(), fRec->fLength);
+            if (sk_atomic_dec(&fRec->fRefCnt) == 1) {
+                // 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.
+                sk_free(fRec);
+            }
+            fRec = rec;
         #ifdef SK_DEBUG
             fStr = fRec->data();
         #endif
@@ -355,9 +356,9 @@ void SkString::set(const char text[]) {
 }
 
 void SkString::set(const char text[], size_t len) {
-    if (len == 0) {
+    if (0 == len) {
         this->reset();
-    } else if (fRec->fRefCnt == 1 && len <= fRec->fLength) {
+    } else if (1 == fRec->fRefCnt && 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();
@@ -365,15 +366,15 @@ void SkString::set(const char text[], size_t len) {
             memcpy(p, text, len);
         }
         p[len] = 0;
-        fRec->fLength = SkToU16(len);
-    } else if (fRec->fRefCnt == 1 && ((unsigned)fRec->fLength >> 2) == (len >> 2)) {
+        fRec->fLength = len;
+    } else if (1 == fRec->fRefCnt && (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) {
             memcpy(p, text, len);
         }
         p[len] = 0;
-        fRec->fLength = SkToU16(len);
+        fRec->fLength = len;
     } else {
         SkString tmp(text, len);
         this->swap(tmp);
@@ -390,7 +391,7 @@ void SkString::setUTF16(const uint16_t src[]) {
 }
 
 void SkString::setUTF16(const uint16_t src[], size_t count) {
-    if (count == 0) {
+    if (0 == count) {
         this->reset();
     } else if (count <= fRec->fLength) {
         // should we resize if len <<<< fLength, to save RAM? (e.g. len < (fLength>>1))
@@ -434,7 +435,7 @@ void SkString::insert(size_t offset, const char text[], size_t len) {
             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 (fRec->fRefCnt == 1 && (length >> 2) == ((length + len) >> 2)) {
+        if (1 == fRec->fRefCnt && (length >> 2) == ((length + len) >> 2)) {
             char* dst = this->writable_str();
 
             if (offset < length) {
@@ -443,7 +444,7 @@ void SkString::insert(size_t offset, const char text[], size_t len) {
             memcpy(dst + offset, text, len);
 
             dst[length + len] = 0;
-            fRec->fLength = SkToU16(length + len);
+            fRec->fLength = length + len;
         } else {
             /*  Seems we should use realloc here, since that is safe if it fails
                 (we have the original data), and might be faster than alloc/copy/free.