Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(290)

Side by Side Diff: base/memory/ref_counted.h

Issue 2666423002: Assert sequence validity on non-thread-safe RefCount manipulations (2) (Closed)
Patch Set: remove DisableSequenceConsistencyAssertions Created 3 years, 8 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View unified diff | Download patch
« no previous file with comments | « base/at_exit.cc ('k') | base/memory/ref_counted.cc » ('j') | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 // Copyright (c) 2012 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
2 // Use of this source code is governed by a BSD-style license that can be 2 // Use of this source code is governed by a BSD-style license that can be
3 // found in the LICENSE file. 3 // found in the LICENSE file.
4 4
5 #ifndef BASE_MEMORY_REF_COUNTED_H_ 5 #ifndef BASE_MEMORY_REF_COUNTED_H_
6 #define BASE_MEMORY_REF_COUNTED_H_ 6 #define BASE_MEMORY_REF_COUNTED_H_
7 7
8 #include <stddef.h> 8 #include <stddef.h>
9 9
10 #include <cassert> 10 #include <cassert>
11 #include <iosfwd> 11 #include <iosfwd>
12 #include <type_traits> 12 #include <type_traits>
13 13
14 #include "base/atomic_ref_count.h" 14 #include "base/atomic_ref_count.h"
15 #include "base/base_export.h" 15 #include "base/base_export.h"
16 #include "base/compiler_specific.h" 16 #include "base/compiler_specific.h"
17 #include "base/logging.h" 17 #include "base/logging.h"
18 #include "base/macros.h" 18 #include "base/macros.h"
19 #include "base/sequence_checker.h"
19 #include "base/threading/thread_collision_warner.h" 20 #include "base/threading/thread_collision_warner.h"
20 #include "build/build_config.h" 21 #include "build/build_config.h"
21 22
22 namespace base { 23 namespace base {
23 24
24 namespace subtle { 25 namespace subtle {
25 26
26 class BASE_EXPORT RefCountedBase { 27 class BASE_EXPORT RefCountedBase {
27 public: 28 public:
28 bool HasOneRef() const { return ref_count_ == 1; } 29 bool HasOneRef() const { return ref_count_ == 1; }
29 30
30 protected: 31 protected:
31 RefCountedBase() {} 32 RefCountedBase() {
33 #if DCHECK_IS_ON()
34 sequence_checker_.DetachFromSequence();
35 #endif
36 }
32 37
33 ~RefCountedBase() { 38 ~RefCountedBase() {
34 #if DCHECK_IS_ON() 39 #if DCHECK_IS_ON()
35 DCHECK(in_dtor_) << "RefCounted object deleted without calling Release()"; 40 DCHECK(in_dtor_) << "RefCounted object deleted without calling Release()";
36 #endif 41 #endif
37 } 42 }
38 43
39 void AddRef() const { 44 void AddRef() const {
40 // TODO(maruel): Add back once it doesn't assert 500 times/sec. 45 // TODO(maruel): Add back once it doesn't assert 500 times/sec.
41 // Current thread books the critical section "AddRelease" 46 // Current thread books the critical section "AddRelease"
42 // without release it. 47 // without release it.
43 // DFAKE_SCOPED_LOCK_THREAD_LOCKED(add_release_); 48 // DFAKE_SCOPED_LOCK_THREAD_LOCKED(add_release_);
44 #if DCHECK_IS_ON() 49 #if DCHECK_IS_ON()
45 DCHECK(!in_dtor_); 50 DCHECK(!in_dtor_);
51 if (ref_count_ >= 1) {
52 DCHECK(CalledOnValidSequence());
53 }
46 #endif 54 #endif
47 55
48 ++ref_count_; 56 ++ref_count_;
49 } 57 }
50 58
51 // Returns true if the object should self-delete. 59 // Returns true if the object should self-delete.
52 bool Release() const { 60 bool Release() const {
53 --ref_count_; 61 --ref_count_;
54 62
55 // TODO(maruel): Add back once it doesn't assert 500 times/sec. 63 // TODO(maruel): Add back once it doesn't assert 500 times/sec.
56 // Current thread books the critical section "AddRelease" 64 // Current thread books the critical section "AddRelease"
57 // without release it. 65 // without release it.
58 // DFAKE_SCOPED_LOCK_THREAD_LOCKED(add_release_); 66 // DFAKE_SCOPED_LOCK_THREAD_LOCKED(add_release_);
59 67
60 #if DCHECK_IS_ON() 68 #if DCHECK_IS_ON()
61 DCHECK(!in_dtor_); 69 DCHECK(!in_dtor_);
62 if (ref_count_ == 0) 70 if (ref_count_ == 0)
63 in_dtor_ = true; 71 in_dtor_ = true;
72
73 if (ref_count_ >= 1)
74 DCHECK(CalledOnValidSequence());
75 if (ref_count_ == 1)
76 sequence_checker_.DetachFromSequence();
64 #endif 77 #endif
65 78
66 return ref_count_ == 0; 79 return ref_count_ == 0;
67 } 80 }
68 81
69 private: 82 private:
83 #if DCHECK_IS_ON()
84 bool CalledOnValidSequence() const;
85 #endif
86
70 mutable size_t ref_count_ = 0; 87 mutable size_t ref_count_ = 0;
88
71 #if DCHECK_IS_ON() 89 #if DCHECK_IS_ON()
72 mutable bool in_dtor_ = false; 90 mutable bool in_dtor_ = false;
91 mutable SequenceChecker sequence_checker_;
73 #endif 92 #endif
74 93
75 DFAKE_MUTEX(add_release_); 94 DFAKE_MUTEX(add_release_);
76 95
77 DISALLOW_COPY_AND_ASSIGN(RefCountedBase); 96 DISALLOW_COPY_AND_ASSIGN(RefCountedBase);
78 }; 97 };
79 98
80 class BASE_EXPORT RefCountedThreadSafeBase { 99 class BASE_EXPORT RefCountedThreadSafeBase {
81 public: 100 public:
82 bool HasOneRef() const; 101 bool HasOneRef() const;
(...skipping 11 matching lines...) Expand all
94 mutable AtomicRefCount ref_count_ = 0; 113 mutable AtomicRefCount ref_count_ = 0;
95 #if DCHECK_IS_ON() 114 #if DCHECK_IS_ON()
96 mutable bool in_dtor_ = false; 115 mutable bool in_dtor_ = false;
97 #endif 116 #endif
98 117
99 DISALLOW_COPY_AND_ASSIGN(RefCountedThreadSafeBase); 118 DISALLOW_COPY_AND_ASSIGN(RefCountedThreadSafeBase);
100 }; 119 };
101 120
102 } // namespace subtle 121 } // namespace subtle
103 122
123 // ScopedAllowCrossThreadRefCountAccess disables the check documented on
124 // RefCounted below for rare pre-existing use cases where thread-safety was
125 // guaranteed through other means (e.g. explicit sequencing of calls across
126 // execution sequences when bouncing between threads in order). New callers
127 // should refrain from using this (callsites handling thread-safety through
128 // locks should use RefCountedThreadSafe per the overhead of its atomics being
129 // negligible compared to locks anyways and callsites doing explicit sequencing
130 // should properly std::move() the ref to avoid hitting this check).
131 // TODO(tzik): Cleanup existing use cases and remove
132 // ScopedAllowCrossThreadRefCountAccess.
133 class BASE_EXPORT ScopedAllowCrossThreadRefCountAccess final {
134 public:
135 #if DCHECK_IS_ON()
136 ScopedAllowCrossThreadRefCountAccess();
137 ~ScopedAllowCrossThreadRefCountAccess();
138 #else
139 ScopedAllowCrossThreadRefCountAccess() {}
140 ~ScopedAllowCrossThreadRefCountAccess() {}
141 #endif
142 };
143
104 // 144 //
105 // A base class for reference counted classes. Otherwise, known as a cheap 145 // A base class for reference counted classes. Otherwise, known as a cheap
106 // knock-off of WebKit's RefCounted<T> class. To use this, just extend your 146 // knock-off of WebKit's RefCounted<T> class. To use this, just extend your
107 // class from it like so: 147 // class from it like so:
108 // 148 //
109 // class MyFoo : public base::RefCounted<MyFoo> { 149 // class MyFoo : public base::RefCounted<MyFoo> {
110 // ... 150 // ...
111 // private: 151 // private:
112 // friend class base::RefCounted<MyFoo>; 152 // friend class base::RefCounted<MyFoo>;
113 // ~MyFoo(); 153 // ~MyFoo();
114 // }; 154 // };
115 // 155 //
116 // You should always make your destructor non-public, to avoid any code deleting 156 // You should always make your destructor non-public, to avoid any code deleting
117 // the object accidently while there are references to it. 157 // the object accidently while there are references to it.
158 //
159 // The ref count manipulation to RefCounted is NOT thread safe and has DCHECKs
160 // to trap unsafe cross thread usage. A subclass instance of RefCounted can be
161 // passed to another execution sequence only when its ref count is 1. If the ref
162 // count is more than 1, the RefCounted class verifies the ref updates are made
163 // on the same execution sequence as the previous ones.
118 template <class T> 164 template <class T>
119 class RefCounted : public subtle::RefCountedBase { 165 class RefCounted : public subtle::RefCountedBase {
120 public: 166 public:
121 RefCounted() = default; 167 RefCounted() = default;
122 168
123 void AddRef() const { 169 void AddRef() const {
124 subtle::RefCountedBase::AddRef(); 170 subtle::RefCountedBase::AddRef();
125 } 171 }
126 172
127 void Release() const { 173 void Release() const {
(...skipping 317 matching lines...) Expand 10 before | Expand all | Expand 10 after
445 bool operator!=(std::nullptr_t null, const scoped_refptr<T>& rhs) { 491 bool operator!=(std::nullptr_t null, const scoped_refptr<T>& rhs) {
446 return !operator==(null, rhs); 492 return !operator==(null, rhs);
447 } 493 }
448 494
449 template <typename T> 495 template <typename T>
450 std::ostream& operator<<(std::ostream& out, const scoped_refptr<T>& p) { 496 std::ostream& operator<<(std::ostream& out, const scoped_refptr<T>& p) {
451 return out << p.get(); 497 return out << p.get();
452 } 498 }
453 499
454 #endif // BASE_MEMORY_REF_COUNTED_H_ 500 #endif // BASE_MEMORY_REF_COUNTED_H_
OLDNEW
« no previous file with comments | « base/at_exit.cc ('k') | base/memory/ref_counted.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698