Fixes for SkRWBuffer

src/core/SkRWBuffer.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkRWBuffer.h"
 #include "SkStream.h"
 
@@ skipping 22 matching lines @@
     // Return number of bytes actually appended
     size_t append(const void* src, size_t length) {
         this->validate();
         size_t amount = SkTMin(this->avail(), length);
         memcpy(this->availData(), src, amount);
         fUsed += amount;
         this->validate();
         return amount;
     }
 
+    // Do not call in the reader thread, since the writer may be updating fUsed.
+    // (The assertion is still true, but TSAN still may complain about its raciness.)
     void validate() const {
 #ifdef SK_DEBUG
         SkASSERT(fCapacity > 0);
         SkASSERT(fUsed <= fCapacity);
 #endif
     }
 
 private:
     static size_t LengthToCapacity(size_t length) {
         const size_t minSize = kMinAllocSize - sizeof(SkBufferBlock);
@@ skipping 34 matching lines @@
             SkBufferBlock* block = fBlock.fNext;
             sk_free((void*)this);
             while (block) {
                 SkBufferBlock* next = block->fNext;
                 sk_free(block);
                 block = next;
             }
         }
     }
 
-    void validate(size_t minUsed, SkBufferBlock* tail = nullptr) const {
+    void validate(size_t minUsed, const SkBufferBlock* tail = nullptr) const {
 #ifdef SK_DEBUG
         SkASSERT(fRefCnt > 0);
         size_t totalUsed = 0;
         const SkBufferBlock* block = &fBlock;
         const SkBufferBlock* lastBlock = block;
         while (block) {
             block->validate();
             totalUsed += block->fUsed;
             lastBlock = block;
             block = block->fNext;
         }
         SkASSERT(minUsed <= totalUsed);
         if (tail) {
             SkASSERT(tail == lastBlock);
         }
 #endif
     }
 };
 
-SkROBuffer::SkROBuffer(const SkBufferHead* head, size_t used) : fHead(head), fUsed(used) {
+SkROBuffer::SkROBuffer(const SkBufferHead* head, size_t used, const SkBufferBlock* tail)
+    : fHead(head)
+    , fUsed(used)
+    , fTail(tail)
+    , fTailUsed(tail ? tail->fUsed : 0)
+{
     if (head) {
         fHead->ref();
         SkASSERT(used > 0);
-        head->validate(used);
+        SkASSERT(tail);
+        head->validate(used, tail);
     } else {
         SkASSERT(0 == used);
+        SkASSERT(!tail);
+        SkASSERT(0 == fTailUsed);
     }
 }
 
 SkROBuffer::~SkROBuffer() {
     if (fHead) {
-        fHead->validate(fUsed);
         fHead->unref();
     }
 }
 
 SkROBuffer::Iter::Iter(const SkROBuffer* buffer) {
     this->reset(buffer);
 }
 
 void SkROBuffer::Iter::reset(const SkROBuffer* buffer) {
+    if (!buffer->fUsed) {
+        SkASSERT(!buffer->fHead);
+        buffer = nullptr;
+    }
+    fBuffer = buffer;
     if (buffer) {
+        SkASSERT(buffer->fHead);
         fBlock = &buffer->fHead->fBlock;
         fRemaining = buffer->fUsed;
     } else {
         fBlock = nullptr;
         fRemaining = 0;
     }
 }
 
 const void* SkROBuffer::Iter::data() const {
     return fRemaining ? fBlock->startData() : nullptr;
 }
 
 size_t SkROBuffer::Iter::size() const {
     if (!fBlock) {
         return 0;
     }
-    return SkTMin(fBlock->fUsed, fRemaining);
+
+    if (fBuffer->fTail == fBlock) {
+        return fBuffer->fTailUsed;
+    }
+
+    // There is another block that this object knows about, with data that is
+    // accounted for in fRemaining.
+    // Further, since there are more blocks, it is safe to read fUsed, which will
+    // not be updated anymore.
+    SkASSERT(fRemaining > fBlock->fUsed);
+    return fBlock->fUsed;
 }
 
 bool SkROBuffer::Iter::next() {
     if (fRemaining) {
         fRemaining -= this->size();
-        fBlock = fBlock->fNext;
+        if (fBuffer->fTail == fBlock) {
+            // There are more blocks, but fBuffer does not know about them.
+            SkASSERT(fRemaining == 0);
+            fBlock = nullptr;
+        } else {
+            fBlock = fBlock->fNext;
+        }
     }
     return fRemaining != 0;
 }
 
 SkRWBuffer::SkRWBuffer(size_t initialCapacity) : fHead(nullptr), fTail(nullptr), fTotalUsed(0) {}
 
 SkRWBuffer::~SkRWBuffer() {
     this->validate();
     if (fHead) {
         fHead->unref();
@@ skipping 54 matching lines @@
 void SkRWBuffer::validate() const {
     if (fHead) {
         fHead->validate(fTotalUsed, fTail);
     } else {
         SkASSERT(nullptr == fTail);
         SkASSERT(0 == fTotalUsed);
     }
 }
 #endif
 
-SkROBuffer* SkRWBuffer::newRBufferSnapshot() const { return new SkROBuffer(fHead, fTotalUsed); }
+SkROBuffer* SkRWBuffer::newRBufferSnapshot() const {
+    return new SkROBuffer(fHead, fTotalUsed, fTail);
+}
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 class SkROBufferStreamAsset : public SkStreamAsset {
     void validate() const {
 #ifdef SK_DEBUG
         SkASSERT(fGlobalOffset <= fBuffer->size());
         SkASSERT(fLocalOffset <= fIter.size());
         SkASSERT(fLocalOffset <= fGlobalOffset);
 #endif
@@ skipping 97 matching lines @@
     const SkROBuffer*   fBuffer;
     SkROBuffer::Iter    fIter;
     size_t              fLocalOffset;
     size_t              fGlobalOffset;
 };
 
 SkStreamAsset* SkRWBuffer::newStreamSnapshot() const {
     SkAutoTUnref<SkROBuffer> buffer(this->newRBufferSnapshot());
     return new SkROBufferStreamAsset(buffer);
 }