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

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

Issue 188603002: Fix base::internal::Singleton to use acquire loads instead of nobarrier ones. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Created 6 years, 9 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 | Annotate | Revision Log
« no previous file with comments | « no previous file | base/memory/singleton.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) 2011 The Chromium Authors. All rights reserved. 1 // Copyright (c) 2011 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 // PLEASE READ: Do you really need a singleton? 5 // PLEASE READ: Do you really need a singleton?
6 // 6 //
7 // Singletons make it hard to determine the lifetime of an object, which can 7 // Singletons make it hard to determine the lifetime of an object, which can
8 // lead to buggy code and spurious crashes. 8 // lead to buggy code and spurious crashes.
9 // 9 //
10 // Instead of adding another singleton into the mix, try to identify either: 10 // Instead of adding another singleton into the mix, try to identify either:
(...skipping 215 matching lines...) Expand 10 before | Expand all | Expand 10 after
226 // member. 226 // member.
227 227
228 // Return a pointer to the one true instance of the class. 228 // Return a pointer to the one true instance of the class.
229 static Type* get() { 229 static Type* get() {
230 #ifndef NDEBUG 230 #ifndef NDEBUG
231 // Avoid making TLS lookup on release builds. 231 // Avoid making TLS lookup on release builds.
232 if (!Traits::kAllowedToAccessOnNonjoinableThread) 232 if (!Traits::kAllowedToAccessOnNonjoinableThread)
233 base::ThreadRestrictions::AssertSingletonAllowed(); 233 base::ThreadRestrictions::AssertSingletonAllowed();
234 #endif 234 #endif
235 235
236 base::subtle::AtomicWord value = base::subtle::NoBarrier_Load(&instance_); 236 // The load has acquire memory ordering as the thread which reads the
237 // instance_ pointer must acquire visibility over the singleton data.
238 base::subtle::AtomicWord value = base::subtle::Acquire_Load(&instance_);
237 if (value != 0 && value != base::internal::kBeingCreatedMarker) { 239 if (value != 0 && value != base::internal::kBeingCreatedMarker) {
238 // See the corresponding HAPPENS_BEFORE below. 240 // See the corresponding HAPPENS_BEFORE below.
239 ANNOTATE_HAPPENS_AFTER(&instance_); 241 ANNOTATE_HAPPENS_AFTER(&instance_);
240 return reinterpret_cast<Type*>(value); 242 return reinterpret_cast<Type*>(value);
241 } 243 }
242 244
243 // Object isn't created yet, maybe we will get to create it, let's try... 245 // Object isn't created yet, maybe we will get to create it, let's try...
244 if (base::subtle::Acquire_CompareAndSwap( 246 if (base::subtle::Acquire_CompareAndSwap(
245 &instance_, 0, base::internal::kBeingCreatedMarker) == 0) { 247 &instance_, 0, base::internal::kBeingCreatedMarker) == 0) {
246 // instance_ was NULL and is now kBeingCreatedMarker. Only one thread 248 // instance_ was NULL and is now kBeingCreatedMarker. Only one thread
247 // will ever get here. Threads might be spinning on us, and they will 249 // will ever get here. Threads might be spinning on us, and they will
248 // stop right after we do this store. 250 // stop right after we do this store.
249 Type* newval = Traits::New(); 251 Type* newval = Traits::New();
250 252
251 // This annotation helps race detectors recognize correct lock-less 253 // This annotation helps race detectors recognize correct lock-less
252 // synchronization between different threads calling get(). 254 // synchronization between different threads calling get().
253 // See the corresponding HAPPENS_AFTER below and above. 255 // See the corresponding HAPPENS_AFTER below and above.
254 ANNOTATE_HAPPENS_BEFORE(&instance_); 256 ANNOTATE_HAPPENS_BEFORE(&instance_);
257 // Releases the visibility over instance_ to the readers.
255 base::subtle::Release_Store( 258 base::subtle::Release_Store(
256 &instance_, reinterpret_cast<base::subtle::AtomicWord>(newval)); 259 &instance_, reinterpret_cast<base::subtle::AtomicWord>(newval));
257 260
258 if (newval != NULL && Traits::kRegisterAtExit) 261 if (newval != NULL && Traits::kRegisterAtExit)
259 base::AtExitManager::RegisterCallback(OnExit, NULL); 262 base::AtExitManager::RegisterCallback(OnExit, NULL);
260 263
261 return newval; 264 return newval;
262 } 265 }
263 266
264 // We hit a race. Wait for the other thread to complete it. 267 // We hit a race. Wait for the other thread to complete it.
(...skipping 15 matching lines...) Expand all
280 instance_ = 0; 283 instance_ = 0;
281 } 284 }
282 static base::subtle::AtomicWord instance_; 285 static base::subtle::AtomicWord instance_;
283 }; 286 };
284 287
285 template <typename Type, typename Traits, typename DifferentiatingType> 288 template <typename Type, typename Traits, typename DifferentiatingType>
286 base::subtle::AtomicWord Singleton<Type, Traits, DifferentiatingType>:: 289 base::subtle::AtomicWord Singleton<Type, Traits, DifferentiatingType>::
287 instance_ = 0; 290 instance_ = 0;
288 291
289 #endif // BASE_MEMORY_SINGLETON_H_ 292 #endif // BASE_MEMORY_SINGLETON_H_
OLDNEW
« no previous file with comments | « no previous file | base/memory/singleton.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698