teach TSAN about SkSpinlock, SkRefCnt, and SkOnce

include/core/SkRefCnt.h

 
 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 
 #ifndef SkRefCnt_DEFINED
@@ skipping 19 matching lines @@
     SK_DECLARE_INST_COUNT_ROOT(SkRefCntBase)
 
     /** Default construct, initializing the reference count to 1.
     */
     SkRefCntBase() : fRefCnt(1) {}
 
     /** Destruct, asserting that the reference count is 1.
     */
     virtual ~SkRefCntBase() {
 #ifdef SK_DEBUG
-        SkASSERT(fRefCnt == 1);
+        SkASSERT(this->unique());
         fRefCnt = 0;    // illegal value, to catch us if we reuse after delete
 #endif
     }
 
     /** Return the reference count. Use only for debugging. */
     int32_t getRefCnt() const { return fRefCnt; }
 
     /** May return true if the caller is the only owner.
      *  Ensures that all previous owner's actions are complete.
      */
     bool unique() const {
         // We believe we're reading fRefCnt in a safe way here, so we stifle the TSAN warning about
         // an unproctected read.  Generally, don't read fRefCnt, and don't stifle this warning.
         bool const unique = (1 == SK_ANNOTATE_UNPROTECTED_READ(fRefCnt));
         if (unique) {
+            SK_ANNOTATE_HAPPENS_AFTER(this);
             // Acquire barrier (L/SL), if not provided by load of fRefCnt.
             // Prevents user's 'unique' code from happening before decrements.
             //TODO: issue the barrier.

[bungeman-skia, 2014/04/23 22:37:54]

We do need to actually issue a barrier here on ARM. Should do that at some
point.

[mtklein, 2014/04/23 23:41:47]

Ooh, yes, or as you noted, provide a way to load fRefCnt with that barrier.

[bungeman-skia, 2014/04/24 14:06:56]

Eh, there's no real reason to provide any barrier to reading fRefCnt. If it
isn't 1 then you can't do anything anyway (and it may already have changed); if
you were testing against 1, then use unique.

         }
         return unique;
     }
 
     /** Increment the reference count. Must be balanced by a call to unref().
     */
     void ref() const {
-        SkASSERT(fRefCnt > 0);
+        SkASSERT(this->unsafeGetRefCnt() > 0);
         sk_atomic_inc(&fRefCnt);  // No barrier required.
     }
 
     /** Decrement the reference count. If the reference count is 1 before the
         decrement, then delete the object. Note that if this is the case, then
         the object needs to have been allocated via new, and not on the stack.
     */
     void unref() const {
-        SkASSERT(fRefCnt > 0);
+        SkASSERT(this->unsafeGetRefCnt() > 0);
+        SK_ANNOTATE_HAPPENS_BEFORE(this);
         // Release barrier (SL/S), if not provided below.
         if (sk_atomic_dec(&fRefCnt) == 1) {
+            SK_ANNOTATE_HAPPENS_AFTER(this);
             // Acquire barrier (L/SL), if not provided above.
             // Prevents code in dispose from happening before the decrement.
             sk_membar_acquire__after_atomic_dec();
             internal_dispose();
         }
     }
 
 #ifdef SK_DEBUG
     void validate() const {
-        SkASSERT(fRefCnt > 0);
+        SkASSERT(this->unsafeGetRefCnt() > 0);
     }
 #endif
 
 protected:
     /**
      *  Allow subclasses to call this if they've overridden internal_dispose
      *  so they can reset fRefCnt before the destructor is called. Should only
      *  be called right before calling through to inherited internal_dispose()
      *  or before calling the destructor.
      */
     void internal_dispose_restore_refcnt_to_1() const {
 #ifdef SK_DEBUG
         SkASSERT(0 == fRefCnt);
         fRefCnt = 1;
 #endif
     }
 
 private:
+    // OK for use in asserts, but not much else.
+    int32_t unsafeGetRefCnt() { return SK_ANNOTATE_UNPROTECTED_READ(fRefCnt); }
+
     /**
      *  Called when the ref count goes to 0.
      */
     virtual void internal_dispose() const {
         this->internal_dispose_restore_refcnt_to_1();
         SkDELETE(this);
     }
 
     // The following friends are those which override internal_dispose()
     // and conditionally call SkRefCnt::internal_dispose().
@@ skipping 167 matching lines @@
     typedef T* SkRefPtr::*unspecified_bool_type;
     operator unspecified_bool_type() const {
         return fObj ? &SkRefPtr::fObj : NULL;
     }
 
 private:
     T* fObj;
 };
 
 #endif