summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
author Steven Moreland <smoreland@google.com> 2023-03-01 01:25:33 +0000
committer Steven Moreland <smoreland@google.com> 2023-03-01 21:25:17 +0000
commitfeb13e8bfe5d9e23563761f042fcf19b7b008d4d (patch)
tree509ab35d45a55dea510e433a8e663ad37129b204
parent11a9b4824010d4ef39f9f04eb10d98adcd91d242 (diff)
RPC Binder: setMaxOutgoing{Threads,Connections}
This API is updated to emphasize that it is placing a limit on the number of connections that will be started corresponding to threads in a remote pool. These connections won't actually correspond to threads in the process that this API is called. Documentation has also been updated to clarify a few questions from an internal discussion forum. Fixes: 270374393 Test: binderRpcTest Change-Id: Ia0d9f0d0f42f58a2c63bf506476b33985f091a34
-rw-r--r--cmds/service/service.cpp2
-rw-r--r--libs/binder/RpcSession.cpp5
-rw-r--r--libs/binder/ServiceManagerHost.cpp4
-rw-r--r--libs/binder/include/binder/IServiceManager.h10
-rw-r--r--libs/binder/include/binder/RpcServer.h5
-rw-r--r--libs/binder/include/binder/RpcSession.h18
-rw-r--r--libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp6
-rw-r--r--libs/binder/libbinder_rpc_unstable.cpp4
-rw-r--r--libs/binder/rust/rpcbinder/src/session.rs9
-rw-r--r--libs/binder/tests/binderHostDeviceTest.cpp2
-rw-r--r--libs/binder/tests/binderRpcTest.cpp2
-rw-r--r--libs/binder/tests/binderRpcTestTrusty.cpp2
12 files changed, 40 insertions, 29 deletions
diff --git a/cmds/service/service.cpp b/cmds/service/service.cpp
index d5ca725eb9..5e8ef5d7d8 100644
--- a/cmds/service/service.cpp
+++ b/cmds/service/service.cpp
@@ -75,7 +75,7 @@ int main(int argc, char* const argv[])
ProcessState::initWithDriver("/dev/vndbinder");
#endif
#ifndef __ANDROID__
- setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingThreads = 1}));
+ setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingConnections = 1}));
#endif
sp<IServiceManager> sm = defaultServiceManager();
fflush(stdout);
diff --git a/libs/binder/RpcSession.cpp b/libs/binder/RpcSession.cpp
index ce6ef2becf..1a821f14c9 100644
--- a/libs/binder/RpcSession.cpp
+++ b/libs/binder/RpcSession.cpp
@@ -90,7 +90,7 @@ size_t RpcSession::getMaxIncomingThreads() {
return mMaxIncomingThreads;
}
-void RpcSession::setMaxOutgoingThreads(size_t threads) {
+void RpcSession::setMaxOutgoingConnections(size_t threads) {
RpcMutexLockGuard _l(mMutex);
LOG_ALWAYS_FATAL_IF(mStartedSetup,
"Must set max outgoing threads before setting up connections");
@@ -932,7 +932,8 @@ status_t RpcSession::ExclusiveConnection::find(const sp<RpcSession>& session, Co
(session->server()
? "This is a server session, so see RpcSession::setMaxIncomingThreads "
"for the corresponding client"
- : "This is a client session, so see RpcSession::setMaxOutgoingThreads "
+ : "This is a client session, so see "
+ "RpcSession::setMaxOutgoingConnections "
"for this client or RpcServer::setMaxThreads for the corresponding "
"server"));
return WOULD_BLOCK;
diff --git a/libs/binder/ServiceManagerHost.cpp b/libs/binder/ServiceManagerHost.cpp
index 194254ac69..2b67f030e0 100644
--- a/libs/binder/ServiceManagerHost.cpp
+++ b/libs/binder/ServiceManagerHost.cpp
@@ -159,8 +159,8 @@ sp<IBinder> getDeviceService(std::vector<std::string>&& serviceDispatcherArgs,
LOG_ALWAYS_FATAL_IF(!forwardResult->hostPort().has_value());
auto rpcSession = RpcSession::make();
- if (options.maxOutgoingThreads.has_value()) {
- rpcSession->setMaxOutgoingThreads(*options.maxOutgoingThreads);
+ if (options.maxOutgoingConnections.has_value()) {
+ rpcSession->setMaxOutgoingConnections(*options.maxOutgoingConnections);
}
if (status_t status = rpcSession->setupInetClient("127.0.0.1", *forwardResult->hostPort());
diff --git a/libs/binder/include/binder/IServiceManager.h b/libs/binder/include/binder/IServiceManager.h
index c78f870153..55167a7db0 100644
--- a/libs/binder/include/binder/IServiceManager.h
+++ b/libs/binder/include/binder/IServiceManager.h
@@ -224,12 +224,12 @@ bool checkPermission(const String16& permission, pid_t pid, uid_t uid,
// }
// Resources are cleaned up when the object is destroyed.
//
-// For each returned binder object, at most |maxOutgoingThreads| outgoing threads are instantiated.
-// Hence, only |maxOutgoingThreads| calls can be made simultaneously. Additional calls are blocked
-// if there are |maxOutgoingThreads| ongoing calls. See RpcSession::setMaxOutgoingThreads.
-// If |maxOutgoingThreads| is not set, default is |RpcSession::kDefaultMaxOutgoingThreads|.
+// For each returned binder object, at most |maxOutgoingConnections| outgoing connections are
+// instantiated, depending on how many the service on the device is configured with.
+// Hence, only |maxOutgoingConnections| calls can be made simultaneously.
+// See also RpcSession::setMaxOutgoingConnections.
struct RpcDelegateServiceManagerOptions {
- std::optional<size_t> maxOutgoingThreads;
+ std::optional<size_t> maxOutgoingConnections;
};
sp<IServiceManager> createRpcDelegateServiceManager(
const RpcDelegateServiceManagerOptions& options);
diff --git a/libs/binder/include/binder/RpcServer.h b/libs/binder/include/binder/RpcServer.h
index 25193a3428..1001b64ede 100644
--- a/libs/binder/include/binder/RpcServer.h
+++ b/libs/binder/include/binder/RpcServer.h
@@ -119,7 +119,10 @@ public:
[[nodiscard]] status_t setupExternalServer(base::unique_fd serverFd);
/**
- * This must be called before adding a client session.
+ * This must be called before adding a client session. This corresponds
+ * to the number of incoming connections to RpcSession objects in the
+ * server, which will correspond to the number of outgoing connections
+ * in client RpcSession objects.
*
* If this is not specified, this will be a single-threaded server.
*
diff --git a/libs/binder/include/binder/RpcSession.h b/libs/binder/include/binder/RpcSession.h
index 40faf2c3b1..e301569acc 100644
--- a/libs/binder/include/binder/RpcSession.h
+++ b/libs/binder/include/binder/RpcSession.h
@@ -67,26 +67,30 @@ public:
/**
* Set the maximum number of incoming threads allowed to be made (for things like callbacks).
* By default, this is 0. This must be called before setting up this connection as a client.
- * Server sessions will inherits this value from RpcServer.
+ * Server sessions will inherits this value from RpcServer. Each thread will serve a
+ * connection to the remote RpcSession.
*
* If this is called, 'shutdown' on this session must also be called.
* Otherwise, a threadpool will leak.
*
- * TODO(b/189955605): start these dynamically
+ * TODO(b/189955605): start these lazily - currently all are started
*/
void setMaxIncomingThreads(size_t threads);
size_t getMaxIncomingThreads();
/**
- * Set the maximum number of outgoing threads allowed to be made.
+ * Set the maximum number of outgoing connections allowed to be made.
* By default, this is |kDefaultMaxOutgoingThreads|. This must be called before setting up this
* connection as a client.
*
- * This limits the number of outgoing threads on top of the remote peer setting. This RpcSession
- * will only instantiate |min(maxOutgoingThreads, remoteMaxThreads)| outgoing threads, where
- * |remoteMaxThreads| can be retrieved from the remote peer via |getRemoteMaxThreads()|.
+ * For an RpcSession client, if you are connecting to a server which starts N threads,
+ * then this must be set to >= N. If you set the maximum number of outgoing connections
+ * to 1, but the server requests 10, then it would be considered an error. If you set a
+ * maximum number of connections to 10, and the server requests 1, then only 1 will be
+ * created. This API is used to limit the amount of resources a server can request you
+ * create.
*/
- void setMaxOutgoingThreads(size_t threads);
+ void setMaxOutgoingConnections(size_t threads);
size_t getMaxOutgoingThreads();
/**
diff --git a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp
index 42d226b805..e273dff5f6 100644
--- a/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp
+++ b/libs/binder/include_rpc_unstable/binder_rpc_unstable.hpp
@@ -126,11 +126,11 @@ AIBinder* ARpcSession_setupPreconnectedClient(ARpcSession* session,
void ARpcSession_setFileDescriptorTransportMode(ARpcSession* session,
ARpcSession_FileDescriptorTransportMode mode);
-// Sets the maximum number of incoming threads.
+// Sets the maximum number of incoming threads, to service connections.
void ARpcSession_setMaxIncomingThreads(ARpcSession* session, size_t threads);
-// Sets the maximum number of outgoing threads.
-void ARpcSession_setMaxOutgoingThreads(ARpcSession* session, size_t threads);
+// Sets the maximum number of outgoing connections.
+void ARpcSession_setMaxOutgoingConnections(ARpcSession* session, size_t threads);
// Decrements the refcount of the underlying RpcSession object.
void ARpcSession_free(ARpcSession* session);
diff --git a/libs/binder/libbinder_rpc_unstable.cpp b/libs/binder/libbinder_rpc_unstable.cpp
index daff8c1e16..971b3a056a 100644
--- a/libs/binder/libbinder_rpc_unstable.cpp
+++ b/libs/binder/libbinder_rpc_unstable.cpp
@@ -265,8 +265,8 @@ void ARpcSession_setMaxIncomingThreads(ARpcSession* handle, size_t threads) {
session->setMaxIncomingThreads(threads);
}
-void ARpcSession_setMaxOutgoingThreads(ARpcSession* handle, size_t threads) {
+void ARpcSession_setMaxOutgoingConnections(ARpcSession* handle, size_t threads) {
auto session = handleToStrongPointer<RpcSession>(handle);
- session->setMaxOutgoingThreads(threads);
+ session->setMaxOutgoingConnections(threads);
}
}
diff --git a/libs/binder/rust/rpcbinder/src/session.rs b/libs/binder/rust/rpcbinder/src/session.rs
index 0b517cf613..28c5390665 100644
--- a/libs/binder/rust/rpcbinder/src/session.rs
+++ b/libs/binder/rust/rpcbinder/src/session.rs
@@ -75,11 +75,14 @@ impl RpcSessionRef {
};
}
- /// Sets the maximum number of outgoing threads.
- pub fn set_max_outgoing_threads(&self, threads: usize) {
+ /// Sets the maximum number of outgoing connections.
+ pub fn set_max_outgoing_connections(&self, connections: usize) {
// SAFETY - Only passes the 'self' pointer as an opaque handle.
unsafe {
- binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingThreads(self.as_ptr(), threads)
+ binder_rpc_unstable_bindgen::ARpcSession_setMaxOutgoingConnections(
+ self.as_ptr(),
+ connections,
+ )
};
}
diff --git a/libs/binder/tests/binderHostDeviceTest.cpp b/libs/binder/tests/binderHostDeviceTest.cpp
index 464da60dde..77a5fa8d65 100644
--- a/libs/binder/tests/binderHostDeviceTest.cpp
+++ b/libs/binder/tests/binderHostDeviceTest.cpp
@@ -66,7 +66,7 @@ MATCHER_P(StatusEq, expected, (negation ? "not " : "") + statusToString(expected
void initHostRpcServiceManagerOnce() {
static std::once_flag gSmOnce;
std::call_once(gSmOnce, [] {
- setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingThreads = 1}));
+ setDefaultServiceManager(createRpcDelegateServiceManager({.maxOutgoingConnections = 1}));
});
}
diff --git a/libs/binder/tests/binderRpcTest.cpp b/libs/binder/tests/binderRpcTest.cpp
index 84c93ddc30..846d945704 100644
--- a/libs/binder/tests/binderRpcTest.cpp
+++ b/libs/binder/tests/binderRpcTest.cpp
@@ -350,7 +350,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc(
for (const auto& session : sessions) {
CHECK(session->setProtocolVersion(clientVersion));
session->setMaxIncomingThreads(options.numIncomingConnections);
- session->setMaxOutgoingThreads(options.numOutgoingConnections);
+ session->setMaxOutgoingConnections(options.numOutgoingConnections);
session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode);
switch (socketType) {
diff --git a/libs/binder/tests/binderRpcTestTrusty.cpp b/libs/binder/tests/binderRpcTestTrusty.cpp
index b3bb5ebda7..84abbac6c6 100644
--- a/libs/binder/tests/binderRpcTestTrusty.cpp
+++ b/libs/binder/tests/binderRpcTestTrusty.cpp
@@ -71,7 +71,7 @@ std::unique_ptr<ProcessSession> BinderRpc::createRpcTestSocketServerProcessEtc(
auto session = android::RpcSession::make(std::move(factory));
EXPECT_TRUE(session->setProtocolVersion(clientVersion));
- session->setMaxOutgoingThreads(options.numOutgoingConnections);
+ session->setMaxOutgoingConnections(options.numOutgoingConnections);
session->setFileDescriptorTransportMode(options.clientFileDescriptorTransportMode);
status = session->setupPreconnectedClient({}, [&]() {