diff options
| -rw-r--r-- | libs/binder/Parcel.cpp | 108 | ||||
| -rw-r--r-- | libs/binder/ProcessState.cpp | 39 | ||||
| -rw-r--r-- | libs/binder/include/binder/Parcel.h | 2 | ||||
| -rw-r--r-- | libs/binder/include/binder/ProcessState.h | 1 | ||||
| -rw-r--r-- | libs/binder/tests/binderLibTest.cpp | 51 |
5 files changed, 9 insertions, 192 deletions
diff --git a/libs/binder/Parcel.cpp b/libs/binder/Parcel.cpp index a1ddec8920..aa9d1882a8 100644 --- a/libs/binder/Parcel.cpp +++ b/libs/binder/Parcel.cpp @@ -102,10 +102,6 @@ static void acquire_object(const sp<ProcessState>& proc, reinterpret_cast<IBinder*>(obj.cookie)->incStrong(who); } return; - case BINDER_TYPE_WEAK_BINDER: - if (obj.binder) - reinterpret_cast<RefBase::weakref_type*>(obj.binder)->incWeak(who); - return; case BINDER_TYPE_HANDLE: { const sp<IBinder> b = proc->getStrongProxyForHandle(obj.handle); if (b != nullptr) { @@ -114,11 +110,6 @@ static void acquire_object(const sp<ProcessState>& proc, } return; } - case BINDER_TYPE_WEAK_HANDLE: { - const wp<IBinder> b = proc->getWeakProxyForHandle(obj.handle); - if (b != nullptr) b.get_refs()->incWeak(who); - return; - } case BINDER_TYPE_FD: { if ((obj.cookie != 0) && (outAshmemSize != nullptr) && ashmem_valid(obj.handle)) { // If we own an ashmem fd, keep track of how much memory it refers to. @@ -144,10 +135,6 @@ static void release_object(const sp<ProcessState>& proc, reinterpret_cast<IBinder*>(obj.cookie)->decStrong(who); } return; - case BINDER_TYPE_WEAK_BINDER: - if (obj.binder) - reinterpret_cast<RefBase::weakref_type*>(obj.binder)->decWeak(who); - return; case BINDER_TYPE_HANDLE: { const sp<IBinder> b = proc->getStrongProxyForHandle(obj.handle); if (b != nullptr) { @@ -156,11 +143,6 @@ static void release_object(const sp<ProcessState>& proc, } return; } - case BINDER_TYPE_WEAK_HANDLE: { - const wp<IBinder> b = proc->getWeakProxyForHandle(obj.handle); - if (b != nullptr) b.get_refs()->decWeak(who); - return; - } case BINDER_TYPE_FD: { if (obj.cookie != 0) { // owned if ((outAshmemSize != nullptr) && ashmem_valid(obj.handle)) { @@ -230,55 +212,6 @@ static status_t flatten_binder(const sp<ProcessState>& /*proc*/, return finish_flatten_binder(binder, obj, out); } -static status_t flatten_binder(const sp<ProcessState>& /*proc*/, - const wp<IBinder>& binder, Parcel* out) -{ - flat_binder_object obj; - - obj.flags = 0x7f | FLAT_BINDER_FLAG_ACCEPTS_FDS; - if (binder != nullptr) { - sp<IBinder> real = binder.promote(); - if (real != nullptr) { - IBinder *local = real->localBinder(); - if (!local) { - BpBinder *proxy = real->remoteBinder(); - if (proxy == nullptr) { - ALOGE("null proxy"); - } - const int32_t handle = proxy ? proxy->handle() : 0; - obj.hdr.type = BINDER_TYPE_WEAK_HANDLE; - obj.binder = 0; /* Don't pass uninitialized stack data to a remote process */ - obj.handle = handle; - obj.cookie = 0; - } else { - obj.hdr.type = BINDER_TYPE_WEAK_BINDER; - obj.binder = reinterpret_cast<uintptr_t>(binder.get_refs()); - obj.cookie = reinterpret_cast<uintptr_t>(binder.unsafe_get()); - } - return finish_flatten_binder(real, obj, out); - } - - // XXX How to deal? In order to flatten the given binder, - // we need to probe it for information, which requires a primary - // reference... but we don't have one. - // - // The OpenBinder implementation uses a dynamic_cast<> here, - // but we can't do that with the different reference counting - // implementation we are using. - ALOGE("Unable to unflatten Binder weak reference!"); - obj.hdr.type = BINDER_TYPE_BINDER; - obj.binder = 0; - obj.cookie = 0; - return finish_flatten_binder(nullptr, obj, out); - - } else { - obj.hdr.type = BINDER_TYPE_BINDER; - obj.binder = 0; - obj.cookie = 0; - return finish_flatten_binder(nullptr, obj, out); - } -} - inline static status_t finish_unflatten_binder( BpBinder* /*proxy*/, const flat_binder_object& /*flat*/, const Parcel& /*in*/) @@ -305,35 +238,6 @@ static status_t unflatten_binder(const sp<ProcessState>& proc, return BAD_TYPE; } -static status_t unflatten_binder(const sp<ProcessState>& proc, - const Parcel& in, wp<IBinder>* out) -{ - const flat_binder_object* flat = in.readObject(false); - - if (flat) { - switch (flat->hdr.type) { - case BINDER_TYPE_BINDER: - *out = reinterpret_cast<IBinder*>(flat->cookie); - return finish_unflatten_binder(nullptr, *flat, in); - case BINDER_TYPE_WEAK_BINDER: - if (flat->binder != 0) { - out->set_object_and_refs( - reinterpret_cast<IBinder*>(flat->cookie), - reinterpret_cast<RefBase::weakref_type*>(flat->binder)); - } else { - *out = nullptr; - } - return finish_unflatten_binder(nullptr, *flat, in); - case BINDER_TYPE_HANDLE: - case BINDER_TYPE_WEAK_HANDLE: - *out = proc->getWeakProxyForHandle(flat->handle); - return finish_unflatten_binder( - static_cast<BpBinder*>(out->unsafe_get()), *flat, in); - } - } - return BAD_TYPE; -} - // --------------------------------------------------------------------------- Parcel::Parcel() @@ -1082,11 +986,6 @@ status_t Parcel::readStrongBinderVector(std::vector<sp<IBinder>>* val) const { return readTypedVector(val, &Parcel::readStrongBinder); } -status_t Parcel::writeWeakBinder(const wp<IBinder>& val) -{ - return flatten_binder(ProcessState::self(), val, this); -} - status_t Parcel::writeRawNullableParcelable(const Parcelable* parcelable) { if (!parcelable) { return writeInt32(0); @@ -2017,13 +1916,6 @@ sp<IBinder> Parcel::readStrongBinder() const return val; } -wp<IBinder> Parcel::readWeakBinder() const -{ - wp<IBinder> val; - unflatten_binder(ProcessState::self(), *this, &val); - return val; -} - status_t Parcel::readParcelable(Parcelable* parcelable) const { int32_t have_parcelable = 0; status_t status = readInt32(&have_parcelable); diff --git a/libs/binder/ProcessState.cpp b/libs/binder/ProcessState.cpp index a07b3a043b..1e1bc3a50e 100644 --- a/libs/binder/ProcessState.cpp +++ b/libs/binder/ProcessState.cpp @@ -210,8 +210,12 @@ sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle) if (e != nullptr) { // We need to create a new BpBinder if there isn't currently one, OR we - // are unable to acquire a weak reference on this current one. See comment - // in getWeakProxyForHandle() for more info about this. + // are unable to acquire a weak reference on this current one. The + // attemptIncWeak() is safe because we know the BpBinder destructor will always + // call expungeHandle(), which acquires the same lock we are holding now. + // We need to do this because there is a race condition between someone + // releasing a reference on this BpBinder, and a new reference on its handle + // arriving from the driver. IBinder* b = e->binder; if (b == nullptr || !e->refs->attemptIncWeak(this)) { if (handle == 0) { @@ -257,37 +261,6 @@ sp<IBinder> ProcessState::getStrongProxyForHandle(int32_t handle) return result; } -wp<IBinder> ProcessState::getWeakProxyForHandle(int32_t handle) -{ - wp<IBinder> result; - - AutoMutex _l(mLock); - - handle_entry* e = lookupHandleLocked(handle); - - if (e != nullptr) { - // We need to create a new BpBinder if there isn't currently one, OR we - // are unable to acquire a weak reference on this current one. The - // attemptIncWeak() is safe because we know the BpBinder destructor will always - // call expungeHandle(), which acquires the same lock we are holding now. - // We need to do this because there is a race condition between someone - // releasing a reference on this BpBinder, and a new reference on its handle - // arriving from the driver. - IBinder* b = e->binder; - if (b == nullptr || !e->refs->attemptIncWeak(this)) { - b = BpBinder::create(handle); - result = b; - e->binder = b; - if (b) e->refs = b->getWeakRefs(); - } else { - result = b; - e->refs->decWeak(this); - } - } - - return result; -} - void ProcessState::expungeHandle(int32_t handle, IBinder* binder) { AutoMutex _l(mLock); diff --git a/libs/binder/include/binder/Parcel.h b/libs/binder/include/binder/Parcel.h index 117b90a03d..3c56fcbfaf 100644 --- a/libs/binder/include/binder/Parcel.h +++ b/libs/binder/include/binder/Parcel.h @@ -117,7 +117,6 @@ public: status_t writeString16(const std::unique_ptr<String16>& str); status_t writeString16(const char16_t* str, size_t len); status_t writeStrongBinder(const sp<IBinder>& val); - status_t writeWeakBinder(const wp<IBinder>& val); status_t writeInt32Array(size_t len, const int32_t *val); status_t writeByteArray(size_t len, const uint8_t *val); status_t writeBool(bool val); @@ -271,7 +270,6 @@ public: sp<IBinder> readStrongBinder() const; status_t readStrongBinder(sp<IBinder>* val) const; status_t readNullableStrongBinder(sp<IBinder>* val) const; - wp<IBinder> readWeakBinder() const; template<typename T> status_t readParcelableVector( diff --git a/libs/binder/include/binder/ProcessState.h b/libs/binder/include/binder/ProcessState.h index 3af9eed9c6..8339976567 100644 --- a/libs/binder/include/binder/ProcessState.h +++ b/libs/binder/include/binder/ProcessState.h @@ -58,7 +58,6 @@ public: void* userData); sp<IBinder> getStrongProxyForHandle(int32_t handle); - wp<IBinder> getWeakProxyForHandle(int32_t handle); void expungeHandle(int32_t handle, IBinder* binder); void spawnPooledThread(bool isMain); diff --git a/libs/binder/tests/binderLibTest.cpp b/libs/binder/tests/binderLibTest.cpp index fb51f98457..4f0c969e91 100644 --- a/libs/binder/tests/binderLibTest.cpp +++ b/libs/binder/tests/binderLibTest.cpp @@ -66,7 +66,6 @@ enum BinderLibTestTranscationCode { BINDER_LIB_TEST_LINK_DEATH_TRANSACTION, BINDER_LIB_TEST_WRITE_FILE_TRANSACTION, BINDER_LIB_TEST_WRITE_PARCEL_FILE_DESCRIPTOR_TRANSACTION, - BINDER_LIB_TEST_PROMOTE_WEAK_REF_TRANSACTION, BINDER_LIB_TEST_EXIT_TRANSACTION, BINDER_LIB_TEST_DELAYED_EXIT_TRANSACTION, BINDER_LIB_TEST_GET_PTR_SIZE_TRANSACTION, @@ -767,22 +766,6 @@ TEST_F(BinderLibTest, PromoteLocal) { EXPECT_TRUE(strong_from_weak == nullptr); } -TEST_F(BinderLibTest, PromoteRemote) { - int ret; - Parcel data, reply; - sp<IBinder> strong = new BBinder(); - sp<IBinder> server = addServer(); - - ASSERT_TRUE(server != nullptr); - ASSERT_TRUE(strong != nullptr); - - ret = data.writeWeakBinder(strong); - EXPECT_EQ(NO_ERROR, ret); - - ret = server->transact(BINDER_LIB_TEST_PROMOTE_WEAK_REF_TRANSACTION, data, &reply); - EXPECT_GE(ret, 0); -} - TEST_F(BinderLibTest, CheckHandleZeroBinderHighBitsZeroCookie) { status_t ret; Parcel data, reply; @@ -808,7 +791,6 @@ TEST_F(BinderLibTest, FreedBinder) { wp<IBinder> keepFreedBinder; { Parcel data, reply; - data.writeBool(false); /* request weak reference */ ret = server->transact(BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION, data, &reply); ASSERT_EQ(NO_ERROR, ret); struct flat_binder_object *freed = (struct flat_binder_object *)(reply.data()); @@ -817,8 +799,9 @@ TEST_F(BinderLibTest, FreedBinder) { * delete its reference to it - otherwise the transaction * fails regardless of whether the driver is fixed. */ - keepFreedBinder = reply.readWeakBinder(); + keepFreedBinder = reply.readStrongBinder(); } + IPCThreadState::self()->flushCommands(); { Parcel data, reply; data.writeStrongBinder(server); @@ -1151,29 +1134,6 @@ class BinderLibTestService : public BBinder if (ret != size) return UNKNOWN_ERROR; return NO_ERROR; } - case BINDER_LIB_TEST_PROMOTE_WEAK_REF_TRANSACTION: { - int ret; - wp<IBinder> weak; - sp<IBinder> strong; - Parcel data2, reply2; - sp<IServiceManager> sm = defaultServiceManager(); - sp<IBinder> server = sm->getService(binderLibTestServiceName); - - weak = data.readWeakBinder(); - if (weak == nullptr) { - return BAD_VALUE; - } - strong = weak.promote(); - - ret = server->transact(BINDER_LIB_TEST_NOP_TRANSACTION, data2, &reply2); - if (ret != NO_ERROR) - exit(EXIT_FAILURE); - - if (strong == nullptr) { - reply->setError(1); - } - return NO_ERROR; - } case BINDER_LIB_TEST_DELAYED_EXIT_TRANSACTION: alarm(10); return NO_ERROR; @@ -1182,13 +1142,8 @@ class BinderLibTestService : public BBinder ; exit(EXIT_SUCCESS); case BINDER_LIB_TEST_CREATE_BINDER_TRANSACTION: { - bool strongRef = data.readBool(); sp<IBinder> binder = new BBinder(); - if (strongRef) { - reply->writeStrongBinder(binder); - } else { - reply->writeWeakBinder(binder); - } + reply->writeStrongBinder(binder); return NO_ERROR; } default: |