summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSteven Moreland <smoreland@google.com>2021-07-21 23:30:29 +0000
committerSteven Moreland <smoreland@google.com>2021-09-27 15:53:57 -0700
commitfd1e8a0328169ee2c485055a4f976fa76d690af0 (patch)
tree9524875099cfc736b4f4502a4086ac11551e65f2
parent10717131812eaf84724f3dcc50880e38d9e03f59 (diff)
downloadnative-fd1e8a0328169ee2c485055a4f976fa76d690af0.tar.gz
libbinder: dec refs include count
This is in preparation for changing how/when we decrement refcounts. The difference in this CL is that the wire format includes a count with an easy mechanism to set the count we want. The difference in behavior from this CL is that sometimes, multiple decStrong refcounts will be combined into one. In the future, we'll want to combine multiple of these. Bug: 182940634 Test: binderRpcTest (on host and also on Pixel 3) Change-Id: I2339a8e0fff3c35ce3c5f562d590da7a12b6670a
-rw-r--r--libs/binder/RpcSession.cpp8
-rw-r--r--libs/binder/RpcState.cpp59
-rw-r--r--libs/binder/RpcState.h25
-rw-r--r--libs/binder/RpcWireFormat.h9
-rw-r--r--libs/binder/include/binder/RpcSession.h3
5 files changed, 78 insertions, 26 deletions
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index 38958c93cb..dafb33942a 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -217,15 +217,17 @@ status_t RpcSession::transact(const sp<IBinder>& binder, uint32_t code, const Pa
}
status_t RpcSession::sendDecStrong(const BpBinder* binder) {
- return sendDecStrong(binder->getPrivateAccessor().rpcAddress());
+ // target is 0 because this is used to free BpBinder objects
+ return sendDecStrongToTarget(binder->getPrivateAccessor().rpcAddress(), 0 /*target*/);
}
-status_t RpcSession::sendDecStrong(uint64_t address) {
+status_t RpcSession::sendDecStrongToTarget(uint64_t address, size_t target) {
ExclusiveConnection connection;
status_t status = ExclusiveConnection::find(sp<RpcSession>::fromExisting(this),
ConnectionUse::CLIENT_REFCOUNT, &connection);
if (status != OK) return status;
- return state()->sendDecStrong(connection.get(), sp<RpcSession>::fromExisting(this), address);
+ return state()->sendDecStrongToTarget(connection.get(), sp<RpcSession>::fromExisting(this),
+ address, target);
}
status_t RpcSession::readId() {
diff --git a/libs/binder/RpcState.cpp b/libs/binder/RpcState.cpp
index df935fe9e3..1838e81dd6 100644
--- a/libs/binder/RpcState.cpp
+++ b/libs/binder/RpcState.cpp
@@ -163,10 +163,17 @@ status_t RpcState::onBinderEntering(const sp<RpcSession>& session, uint64_t addr
_l.unlock();
+ // if this is a local binder, then we want to get rid of all refcounts
+ // (tell the other process it can drop the binder when it wants to - we
+ // have a local sp<>, so we will drop it when we want to as well). if
+ // this is a remote binder, then we need to hold onto one refcount until
+ // it is dropped in BpBinder::onLastStrongRef
+ size_t targetRecd = (*out)->localBinder() ? 0 : 1;
+
// We have timesRecd RPC refcounts, but we only need to hold on to one
// when we keep the object. All additional dec strongs are sent
// immediately, we wait to send the last one in BpBinder::onLastDecStrong.
- return session->sendDecStrong(address);
+ return session->sendDecStrongToTarget(address, targetRecd);
}
// we don't know about this binder, so the other side of the connection
@@ -567,32 +574,43 @@ status_t RpcState::waitForReply(const sp<RpcSession::RpcConnection>& connection,
return OK;
}
-status_t RpcState::sendDecStrong(const sp<RpcSession::RpcConnection>& connection,
- const sp<RpcSession>& session, uint64_t addr) {
+status_t RpcState::sendDecStrongToTarget(const sp<RpcSession::RpcConnection>& connection,
+ const sp<RpcSession>& session, uint64_t addr,
+ size_t target) {
+ RpcDecStrong body = {
+ .address = RpcWireAddress::fromRaw(addr),
+ };
+
{
std::lock_guard<std::mutex> _l(mNodeMutex);
if (mTerminated) return DEAD_OBJECT; // avoid fatal only, otherwise races
auto it = mNodeForAddress.find(addr);
LOG_ALWAYS_FATAL_IF(it == mNodeForAddress.end(),
"Sending dec strong on unknown address %" PRIu64, addr);
- LOG_ALWAYS_FATAL_IF(it->second.timesRecd <= 0, "Bad dec strong %" PRIu64, addr);
- it->second.timesRecd--;
+ LOG_ALWAYS_FATAL_IF(it->second.timesRecd < target, "Can't dec count of %zu to %zu.",
+ it->second.timesRecd, target);
+
+ // typically this happens when multiple threads send dec refs at the
+ // same time - the transactions will get combined automatically
+ if (it->second.timesRecd == target) return OK;
+
+ body.amount = it->second.timesRecd - target;
+ it->second.timesRecd = target;
+
LOG_ALWAYS_FATAL_IF(nullptr != tryEraseNode(it),
"Bad state. RpcState shouldn't own received binder");
}
RpcWireHeader cmd = {
.command = RPC_COMMAND_DEC_STRONG,
- .bodySize = sizeof(RpcWireAddress),
+ .bodySize = sizeof(RpcDecStrong),
};
if (status_t status = rpcSend(connection, session, "dec ref header", &cmd, sizeof(cmd));
status != OK)
return status;
- if (status_t status = rpcSend(connection, session, "dec ref body", &addr, sizeof(addr));
- status != OK)
- return status;
- return OK;
+
+ return rpcSend(connection, session, "dec ref body", &body, sizeof(body));
}
status_t RpcState::getAndExecuteCommand(const sp<RpcSession::RpcConnection>& connection,
@@ -912,16 +930,15 @@ status_t RpcState::processDecStrong(const sp<RpcSession::RpcConnection>& connect
status != OK)
return status;
- if (command.bodySize != sizeof(RpcWireAddress)) {
- ALOGE("Expecting %zu but got %" PRId32 " bytes for RpcWireAddress. Terminating!",
- sizeof(RpcWireAddress), command.bodySize);
+ if (command.bodySize != sizeof(RpcDecStrong)) {
+ ALOGE("Expecting %zu but got %" PRId32 " bytes for RpcDecStrong. Terminating!",
+ sizeof(RpcDecStrong), command.bodySize);
(void)session->shutdownAndWait(false);
return BAD_VALUE;
}
- RpcWireAddress* address = reinterpret_cast<RpcWireAddress*>(commandData.data());
-
- uint64_t addr = RpcWireAddress::toRaw(*address);
+ RpcDecStrong* body = reinterpret_cast<RpcDecStrong*>(commandData.data());
+ uint64_t addr = RpcWireAddress::toRaw(body->address);
std::unique_lock<std::mutex> _l(mNodeMutex);
auto it = mNodeForAddress.find(addr);
if (it == mNodeForAddress.end()) {
@@ -939,15 +956,19 @@ status_t RpcState::processDecStrong(const sp<RpcSession::RpcConnection>& connect
return BAD_VALUE;
}
- if (it->second.timesSent == 0) {
- ALOGE("No record of sending binder, but requested decStrong: %" PRIu64, addr);
+ if (it->second.timesSent < body->amount) {
+ ALOGE("Record of sending binder %zu times, but requested decStrong for %" PRIu64 " of %u",
+ it->second.timesSent, addr, body->amount);
return OK;
}
LOG_ALWAYS_FATAL_IF(it->second.sentRef == nullptr, "Inconsistent state, lost ref for %" PRIu64,
addr);
- it->second.timesSent--;
+ LOG_RPC_DETAIL("Processing dec strong of %" PRIu64 " by %u from %zu", addr, body->amount,
+ it->second.timesSent);
+
+ it->second.timesSent -= body->amount;
sp<IBinder> tempHold = tryEraseNode(it);
_l.unlock();
tempHold = nullptr; // destructor may make binder calls on this session
diff --git a/libs/binder/RpcState.h b/libs/binder/RpcState.h
index dcfb5699e8..b21677084c 100644
--- a/libs/binder/RpcState.h
+++ b/libs/binder/RpcState.h
@@ -82,8 +82,29 @@ public:
uint64_t address, uint32_t code, const Parcel& data,
const sp<RpcSession>& session, Parcel* reply,
uint32_t flags);
- [[nodiscard]] status_t sendDecStrong(const sp<RpcSession::RpcConnection>& connection,
- const sp<RpcSession>& session, uint64_t address);
+
+ /**
+ * The ownership model here carries an implicit strong refcount whenever a
+ * binder is sent across processes. Since we have a local strong count in
+ * sp<> over these objects, we only ever need to keep one of these. So,
+ * typically we tell the remote process that we drop all the implicit dec
+ * strongs, and we hold onto the last one. 'target' here is the target
+ * timesRecd (the number of remaining reference counts) we wish to keep.
+ * Typically this should be '0' or '1'. The target is used instead of an
+ * explicit decrement count in order to allow multiple threads to lower the
+ * number of counts simultaneously. Since we only lower the count to 0 when
+ * a binder is deleted, targets of '1' should only be sent when the caller
+ * owns a local strong reference to the binder. Larger targets may be used
+ * for testing, and to make the function generic, but generally this should
+ * be avoided because it would be hard to guarantee another thread doesn't
+ * lower the number of held refcounts to '1'. Note also, these refcounts
+ * must be sent actively. If they are sent when binders are deleted, this
+ * can cause leaks, since even remote binders carry an implicit strong ref
+ * when they are sent to another process.
+ */
+ [[nodiscard]] status_t sendDecStrongToTarget(const sp<RpcSession::RpcConnection>& connection,
+ const sp<RpcSession>& session, uint64_t address,
+ size_t target);
enum class CommandType {
ANY,
diff --git a/libs/binder/RpcWireFormat.h b/libs/binder/RpcWireFormat.h
index a87aa074a9..171550e620 100644
--- a/libs/binder/RpcWireFormat.h
+++ b/libs/binder/RpcWireFormat.h
@@ -85,7 +85,7 @@ enum : uint32_t {
*/
RPC_COMMAND_REPLY,
/**
- * follows is RpcWireAddress
+ * follows is RpcDecStrong
*
* note - this in the protocol directly instead of as a 'special
* transaction' in order to keep it as lightweight as possible (we don't
@@ -117,6 +117,13 @@ struct RpcWireHeader {
};
static_assert(sizeof(RpcWireHeader) == 16);
+struct RpcDecStrong {
+ RpcWireAddress address;
+ uint32_t amount;
+ uint32_t reserved;
+};
+static_assert(sizeof(RpcDecStrong) == 16);
+
struct RpcWireTransaction {
RpcWireAddress address;
uint32_t code;
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index 6a29c05e36..12d448d1e4 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -176,7 +176,8 @@ private:
friend RpcState;
explicit RpcSession(std::unique_ptr<RpcTransportCtx> ctx);
- [[nodiscard]] status_t sendDecStrong(uint64_t address);
+ // for 'target', see RpcState::sendDecStrongToTarget
+ [[nodiscard]] status_t sendDecStrongToTarget(uint64_t address, size_t target);
class EventListener : public virtual RefBase {
public: