Revert "Revert "Revert "Revert "Remove incorrect android_atomic_...64 use.""""

After fixing b/16874785.

This reverts commit f010a05c7e9a23b6083294aff4a8183ab01f686e.

Original comment, which actually describes the effect of this:

Change the mExtras field in Binder.h to be a stdatomic.h atomic
value, and replace references to it with proper stdatomic.h calls.
This removes one of a small number of remaining 64 bit
android_atomic references.  It also replaces the erroneously
non-atomic read accesses to mExtras.

It would be better if this used the C++11 <atomic> facility,
but we don't quite have that yet.

Fixes

Bug:16513433

Change-Id: I1645ca5d6f60595bf5d388913665ce4b8780b26d
(cherry picked from commit 3effababf2980d029339522fdc914bdeb913d99b)

diff --git a/include/binder/Binder.h b/include/binder/Binder.h
index ba3ac4b..86628a0 100644 (file)
--- a/include/binder/Binder.h
+++ b/include/binder/Binder.h
@@ -17,6 +17,8 @@
 #ifndef ANDROID_BINDER_H
 #define ANDROID_BINDER_H
 
+#include <stdatomic.h>
+#include <stdint.h>
 #include <binder/IBinder.h>
 
 // ---------------------------------------------------------------------------
@@ -69,7 +71,7 @@ private:
 
     class Extras;
 
-            Extras*     mExtras;
+    atomic_uintptr_t    mExtras;  // should be atomic<Extras *>
             void*       mReserved0;
 };
 

diff --git a/libs/binder/Binder.cpp b/libs/binder/Binder.cpp
index 71e62ab..d9d4971 100644 (file)
--- a/libs/binder/Binder.cpp
+++ b/libs/binder/Binder.cpp
@@ -16,7 +16,7 @@
 
 #include <binder/Binder.h>
 
-#include <utils/Atomic.h>
+#include <stdatomic.h>
 #include <utils/misc.h>
 #include <binder/BpBinder.h>
 #include <binder/IInterface.h>
@@ -71,8 +71,8 @@ public:
 // ---------------------------------------------------------------------------
 
 BBinder::BBinder()
-    : mExtras(NULL)
 {
+    atomic_init(&mExtras, 0);
 }
 
 bool BBinder::isBinderAlive() const
@@ -139,19 +139,19 @@ void BBinder::attachObject(
     const void* objectID, void* object, void* cleanupCookie,
     object_cleanup_func func)
 {
-    Extras* e = mExtras;
+    Extras* e = reinterpret_cast<Extras*>(
+                    atomic_load_explicit(&mExtras, memory_order_acquire));
 
     if (!e) {
         e = new Extras;
-#ifdef __LP64__
-        if (android_atomic_release_cas64(0, reinterpret_cast<int64_t>(e),
-                reinterpret_cast<volatile int64_t*>(&mExtras)) != 0) {
-#else
-        if (android_atomic_cmpxchg(0, reinterpret_cast<int32_t>(e),
-                reinterpret_cast<volatile int32_t*>(&mExtras)) != 0) {
-#endif
+        uintptr_t* expected = 0;
+        if (!atomic_compare_exchange_strong_explicit(
+                                        &mExtras, &expected,
+                                        reinterpret_cast<uintptr_t>(e),
+                                        memory_order_release,
+                                        memory_order_acquire)) {
             delete e;
-            e = mExtras;
+            e = reinterpret_cast<Extras*>(expected);  // Filled in by CAS
         }
         if (e == 0) return; // out of memory
     }
@@ -162,7 +162,8 @@ void BBinder::attachObject(
 
 void* BBinder::findObject(const void* objectID) const
 {
-    Extras* e = mExtras;
+    Extras* e = reinterpret_cast<Extras*>(
+                    atomic_load_explicit(&mExtras, memory_order_acquire));
     if (!e) return NULL;
 
     AutoMutex _l(e->mLock);
@@ -171,7 +172,8 @@ void* BBinder::findObject(const void* objectID) const
 
 void BBinder::detachObject(const void* objectID)
 {
-    Extras* e = mExtras;
+    Extras* e = reinterpret_cast<Extras*>(
+                    atomic_load_explicit(&mExtras, memory_order_acquire));
     if (!e) return;
 
     AutoMutex _l(e->mLock);
@@ -185,7 +187,9 @@ BBinder* BBinder::localBinder()
 
 BBinder::~BBinder()
 {
-    if (mExtras) delete mExtras;
+    Extras* e = reinterpret_cast<Extras*>(
+                    atomic_load_explicit(&mExtras, memory_order_relaxed));
+    if (e) delete e;
 }