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 "SkAtomics.h"
 #include "SkRWBuffer.h"
 #include "SkStream.h"
@@ skipping 24 matching lines @@
     //
     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 32 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
     }
 };
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 // The reader can only access block.fCapacity (which never changes), and cannot access
 // block.fUsed, which may be updated by the writer.
 //
-SkROBuffer::SkROBuffer(const SkBufferHead* head, size_t available)
-    : fHead(head), fAvailable(available)
+SkROBuffer::SkROBuffer(const SkBufferHead* head, size_t available, const SkBufferBlock* tail)
+    : fHead(head), fAvailable(available), fTail(tail)
 {
     if (head) {
         fHead->ref();
         SkASSERT(available > 0);
-        head->validate(available);
+        head->validate(available, tail);
     } else {
         SkASSERT(0 == available);
+        SkASSERT(!tail);
     }
 }
 
 SkROBuffer::~SkROBuffer() {
     if (fHead) {
-        fHead->validate(fAvailable);
         fHead->unref();
     }
 }
 
 SkROBuffer::Iter::Iter(const SkROBuffer* buffer) {
     this->reset(buffer);
 }
 
 void SkROBuffer::Iter::reset(const SkROBuffer* buffer) {
-    if (buffer) {
+    fBuffer = buffer;
+    if (buffer && buffer->fHead) {
         fBlock = &buffer->fHead->fBlock;
         fRemaining = buffer->fAvailable;
     } 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->fCapacity, fRemaining);
 }
 
 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(0 == fRemaining);
+            fBlock = nullptr;
+        } else {
+            fBlock = fBlock->fNext;
+        }
     }
     return fRemaining != 0;
 }
 
 ///////////////////////////////////////////////////////////////////////////////////////////////////
 
 SkRWBuffer::SkRWBuffer(size_t initialCapacity) : fHead(nullptr), fTail(nullptr), fTotalUsed(0) {}
 
 SkRWBuffer::~SkRWBuffer() {
     this->validate();
@@ skipping 38 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);
 }