summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorHans Boehm2016-05-18 12:09:24 -0500
committerHans Boehm2016-05-23 12:28:52 -0500
commit3e4c076ef204c4b572d02bd1c8dbf8c599e0014d (patch)
tree028d5c25852c14eda61c5309621411806a4dd5c0 /libutils/SharedBuffer.h
parent62212954efc9cd4ddfa91f100ec4ecec27315e42 (diff)
downloadplatform-system-core-3e4c076ef204c4b572d02bd1c8dbf8c599e0014d.tar.gz
platform-system-core-3e4c076ef204c4b572d02bd1c8dbf8c599e0014d.tar.xz
platform-system-core-3e4c076ef204c4b572d02bd1c8dbf8c599e0014d.zip
Fix SharedBuffer. Remove aref.
Add comment that SharedBuffer is deprecated. Both aref and SharedBuffer had memory ordering bugs. Aref has no clients. SharedBuffer had several bugs, which are fixed here: mRefs was declared neither volatile, not atomic, allowing the compiler to, for example, reuse a stale previously loaded value. It used the default android_atomic release memory ordering, which is insufficient for reference count decrements. It used an ordinary memory read in onlyOwner() to check whether an object is safe to deallocate, without any attempt to ensure memory ordering. Comments claimed that SharedBuffer was exactly 16 bytes, but this was neither checked, nor correct on 64-bit platforms. This turns mRef into a std::atomic and removes the android_atomic dependency. Bug: 28826227 Change-Id: I39fa0b4f70ac0471b14ad274806fc4e0c0802e78
Diffstat (limited to 'libutils/SharedBuffer.h')
-rw-r--r--libutils/SharedBuffer.h21
1 files changed, 15 insertions, 6 deletions
diff --git a/libutils/SharedBuffer.h b/libutils/SharedBuffer.h
index b6709537e..48358cddc 100644
--- a/libutils/SharedBuffer.h
+++ b/libutils/SharedBuffer.h
@@ -14,9 +14,14 @@
14 * limitations under the License. 14 * limitations under the License.
15 */ 15 */
16 16
17/*
18 * DEPRECATED. DO NOT USE FOR NEW CODE.
19 */
20
17#ifndef ANDROID_SHARED_BUFFER_H 21#ifndef ANDROID_SHARED_BUFFER_H
18#define ANDROID_SHARED_BUFFER_H 22#define ANDROID_SHARED_BUFFER_H
19 23
24#include <atomic>
20#include <stdint.h> 25#include <stdint.h>
21#include <sys/types.h> 26#include <sys/types.h>
22 27
@@ -43,7 +48,7 @@ public:
43 * In other words, the buffer must have been release by all its 48 * In other words, the buffer must have been release by all its
44 * users. 49 * users.
45 */ 50 */
46 static ssize_t dealloc(const SharedBuffer* released); 51 static void dealloc(const SharedBuffer* released);
47 52
48 //! access the data for read 53 //! access the data for read
49 inline const void* data() const; 54 inline const void* data() const;
@@ -94,12 +99,16 @@ private:
94 SharedBuffer(const SharedBuffer&); 99 SharedBuffer(const SharedBuffer&);
95 SharedBuffer& operator = (const SharedBuffer&); 100 SharedBuffer& operator = (const SharedBuffer&);
96 101
97 // 16 bytes. must be sized to preserve correct alignment. 102 // Must be sized to preserve correct alignment.
98 mutable int32_t mRefs; 103 mutable std::atomic<int32_t> mRefs;
99 size_t mSize; 104 size_t mSize;
100 uint32_t mReserved[2]; 105 uint32_t mReserved[2];
101}; 106};
102 107
108static_assert(sizeof(SharedBuffer) % 8 == 0
109 && (sizeof(size_t) > 4 || sizeof(SharedBuffer) == 16),
110 "SharedBuffer has unexpected size");
111
103// --------------------------------------------------------------------------- 112// ---------------------------------------------------------------------------
104 113
105const void* SharedBuffer::data() const { 114const void* SharedBuffer::data() const {
@@ -127,7 +136,7 @@ size_t SharedBuffer::sizeFromData(const void* data) {
127} 136}
128 137
129bool SharedBuffer::onlyOwner() const { 138bool SharedBuffer::onlyOwner() const {
130 return (mRefs == 1); 139 return (mRefs.load(std::memory_order_acquire) == 1);
131} 140}
132 141
133}; // namespace android 142}; // namespace android