path: root/include/utils
diff options
authorHans Boehm <>2016-05-11 18:15:12 -0700
committerHans Boehm <>2016-05-19 16:18:26 -0700
commit70a46d674a04e95da633a5914abd7a55a46e2b3e (patch)
tree6d3a7e244b6ec0c8cc16536e1954c81140b1a6a1 /include/utils
parentd4af0d64de042b0425bbae1f91d1720eebcf95a3 (diff)
Fix memory order and race bugs in Refbase.h & RefBase.cpp
Convert to use std::atomic directly. Consistently use relaxed ordering for increments, release ordering for decrements, and an added acquire fence when the count goes to zero. Fix what looks like another race in attemptIncStrong: It seems entirely possible that the final adjustment for INITIAL_STRONG_VALUE would see e.g. INITIAL_STRONG_VALUE + 1, since we could be running in the middle of another initial increment. Attempt to somewhat document what this actually does, and what's expected from the client. Hide the documentation in the .cpp file for now. Remove a confusing redundant test in decWeak. OBJECT_LIFETIME_STRONG and OBJECT_LIFETIME_WEAK are the only options, in spite of some of the original comments. It's conceivable that either of these issues has resulted in actual crashes, though I would guess the probability is small. It's hard enough to reason about this code without the bugs. Bug: 28705989 Change-Id: I4107a56c3fc0fdb7ee17fc8a8f0dd7fb128af9d8 (cherry picked from commit e263e6c6337a24d56dc803792206e54981ad53a5)
Diffstat (limited to 'include/utils')
1 files changed, 6 insertions, 5 deletions
diff --git a/include/utils/RefBase.h b/include/utils/RefBase.h
index eac6a7840..14d9cb12e 100644
--- a/include/utils/RefBase.h
+++ b/include/utils/RefBase.h
@@ -17,7 +17,7 @@
-#include <cutils/atomic.h>
+#include <atomic>
#include <stdint.h>
#include <sys/types.h>
@@ -176,16 +176,17 @@ class LightRefBase
inline LightRefBase() : mCount(0) { }
inline void incStrong(__attribute__((unused)) const void* id) const {
- android_atomic_inc(&mCount);
+ mCount.fetch_add(1, std::memory_order_relaxed);
inline void decStrong(__attribute__((unused)) const void* id) const {
- if (android_atomic_dec(&mCount) == 1) {
+ if (mCount.fetch_sub(1, std::memory_order_release) == 1) {
+ std::atomic_thread_fence(std::memory_order_acquire);
delete static_cast<const T*>(this);
//! DEBUGGING ONLY: Get current strong ref count.
inline int32_t getStrongCount() const {
- return mCount;
+ return mCount.load(std::memory_order_relaxed);
typedef LightRefBase<T> basetype;
@@ -200,7 +201,7 @@ private:
const void* old_id, const void* new_id) { }
- mutable volatile int32_t mCount;
+ mutable std::atomic<int32_t> mCount;
// This is a wrapper around LightRefBase that simply enforces a virtual